Skip to content

fix(web-shell): scope mock sessions by workspace #10273

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.

Sign up for GitHub

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

Open
AaronZ345 wants to merge 10 commits into
base: main
Choose a base branch
from
+337 −30
Open

fix(web-shell): scope mock sessions by workspace #10273

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3498951
fix(web-shell): scope mock sessions by workspace
Aug 27, 2026
77a0a2a
fix(web-shell): harden workspace-scoped session mocks
Aug 27, 2026
8ef6f9d
test(serve): compare retry port exactly
Aug 27, 2026
8dcc622
fix(web-shell): align mock session route matching
Aug 27, 2026
c682cf2
fix(web-shell): share mock workspace session route parsing
Aug 27, 2026
766e904
fix(web-shell): harden mocked workspace session routes
Aug 27, 2026
63df202
test(web-shell): align pinned session catalog fetch
Aug 28, 2026
da020a9
Merge remote-tracking branch 'origin/main' into fix/10250-visuals-sid…
Aug 28, 2026
0c8a656
test(web-shell): pin workspace session scoping
Aug 28, 2026
fa9786d
Merge remote-tracking branch 'origin/main' into fix/10250-visuals-sid…
wenshao Aug 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 24 additions & 3 deletions packages/web-shell/client/e2e/utils/mockDaemon.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,21 +632,37 @@ function readRequestBody(raw: string | null): unknown {
function filterScenarioSessions(
scenario: WebShellDaemonScenario,
searchParams: URLSearchParams,
workspaceCwd?: string,
): DaemonSessionSummary[] {
const group = searchParams.get('group');
const sourceType = searchParams.get('sourceType');
const sourceSessions = sourceType
const workspaceSessions = workspaceCwd
? scenario.sessions.filter(
(session) => session.workspaceCwd === workspaceCwd,
)
: scenario.sessions;
const sourceSessions = sourceType
? workspaceSessions.filter(
(session) =>
session.sourceType === sourceType ||
(sourceType === 'default' && session.sourceType === undefined),
)
: scenario.sessions;
: workspaceSessions;
return group === 'pinned'
? sourceSessions.filter((session) => Boolean(session.isPinned))
: sourceSessions;
}

function workspaceCwdFromSessionsPath(path: string): string | undefined {

@qwen-code-ci-bot qwen-code-ci-bot Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Workspace scoping fails open on guard/extractor drift. workspaceCwdFromSessionsPath returns undefined on a non-matching path, and both consumers — the live-state handler's !workspaceCwd || fallback and filterScenarioSessions' workspaceCwd ? … : scenario.sessions branch — then silently serve the unscoped cross-workspace catalog, the exact behavior this PR exists to remove. Both fallbacks are dead today (every route guard regex is a subset of the helper's patterns), but this file already carries three parallel copies of the route shapes (isDaemonPath, isDaemonRoute, handleDaemonRoute), so guard/extractor drift is its known failure mode: if a new sessions path shape is added to a route guard without extending this helper, the handler silently returns sessions from every workspace again, and the symptom surfaces as Playwright strict-mode duplicate-name failures in visual specs — a recurrence of the bug this PR fixes — many files away from the cause. Making the helper total turns that silent re-leak into a loud failure at the cause:

function workspaceCwdFromSessionsPath(path: string): string {
  const workspaceMatch = path.match(
    /^\/workspaces\/([^/]+)\/sessions(?:\/live-state)?\/?$/,
  );
  if (workspaceMatch) return decodeURIComponent(workspaceMatch[1]);

  const legacyMatch = path.match(/^\/workspace\/(.+)\/sessions\/?$/);
  if (legacyMatch) return decodeURIComponent(legacyMatch[1]);
  throw new Error(`Unrecognized sessions path: ${path}`);
}

…plus deleting the !workspaceCwd || fallback in the live-state handler and the unscoped branch in filterScenarioSessions. Every call site sits behind a guard that guarantees a cwd segment, so the throw fires precisely when guard and extractor disagree.

— qwen3.8-max via Qwen Code /review (v0.22.2)

const workspaceMatch = path.match(
/^\/workspaces\/([^/]+)\/sessions(?:\/live-state)?\/?$/,
);
if (workspaceMatch) return decodeURIComponent(workspaceMatch[1]);

const legacyMatch = path.match(/^\/workspace\/(.+)\/sessions\/?$/);
return legacyMatch ? decodeURIComponent(legacyMatch[1]) : undefined;
}

function isDaemonPath(path: string): boolean {
return (
path === '/health' ||
Expand Down Expand Up @@ -1009,10 +1025,14 @@ async function handleDaemonRoute(
method === 'GET' &&
/^\/workspaces\/[^/]+\/sessions\/live-state\/?$/.test(path)
) {
const workspaceCwd = workspaceCwdFromSessionsPath(path);
await json(route, {
v: 1,
catalogVersion: scenario.sessionCatalogVersion,
sessions: scenario.sessions
.filter(
(session) => !workspaceCwd || session.workspaceCwd === workspaceCwd,
)

@qwen-code-ci-bot qwen-code-ci-bot Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The newly added workspace scoping on the live-state route has no test that can tell scoped from unscoped or empty responses. The only spec enabling workspace_session_live_state (web-shell.session-live-state.spec.ts) is single-workspace, where this filter is a no-op, and it asserts only request counters plus a tab click — deleting or inverting the filter leaves every assertion green. The multi-workspace workspace sidebar visual scenario does not enable the live-state capability, so its new toHaveCount(1) assertions pin only the sessions-listing route, not this one. A future regression in workspaceCwdFromSessionsPath on this path would therefore ship green: the mock silently diverges from the real daemon's per-workspace live-state contract, and recordLiveSessions (session-catalog-store.ts) stores the response per workspace without re-filtering, so live-state rows get attributed to the wrong workspace with no E2E failing. Consider adding multi-workspace live-state coverage — either extend web-shell.session-live-state.spec.ts to two workspaces with one live session (clientCount: 1) each and assert scoping on content, or enable workspace_session_live_state in the workspace sidebar visual scenario so its existing toHaveCount(1) assertions pin this path too. If you add that coverage, please confirm it bites: removing or inverting this .filter(...) must make the new assertion fail.

— qwen3.8-max via Qwen Code /review (v0.22.2)

.filter(
(session) =>
(session.clientCount ?? 0) > 0 ||
Expand All @@ -1035,8 +1055,9 @@ async function handleDaemonRoute(
(/^\/workspace\/.+\/sessions\/?$/.test(path) ||
/^\/workspaces\/[^/]+\/sessions\/?$/.test(path))
) {
const workspaceCwd = workspaceCwdFromSessionsPath(path);
await json(route, {
sessions: filterScenarioSessions(scenario, searchParams),
sessions: filterScenarioSessions(scenario, searchParams, workspaceCwd),
});
return;
}
Expand Down
30 changes: 27 additions & 3 deletions packages/web-shell/client/e2e/visuals/screenshots.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { expect, test } from '@playwright/test';
import type { DaemonEvent } from '@qwen-code/sdk/daemon';
import type { DaemonEvent, DaemonSessionSummary } from '@qwen-code/sdk/daemon';
import {
assistantTextEvent,
createWebShellDaemonScenario,
Expand Down Expand Up @@ -785,9 +785,32 @@ for (const theme of THEMES) {
// turn this into a cryptic "not visible" failure.
const primaryCwd = '/tmp/qwen-web-shell-e2e';
const primarySessionName = 'Run auth migration';
const secondaryCwd = '/tmp/qwen-api-service';
const secondarySessionName = 'Audit API retries';
const sessions = [
{
sessionId: 'workspace-primary-session',
workspaceCwd: primaryCwd,
createdAt: '2026-07-03T00:00:00.000Z',
updatedAt: '2026-07-03T00:00:00.000Z',
displayName: primarySessionName,
clientCount: 1,
hasActivePrompt: false,
},
{
sessionId: 'workspace-secondary-session',
workspaceCwd: secondaryCwd,
createdAt: '2026-07-03T00:00:00.000Z',
updatedAt: '2026-07-03T00:00:00.000Z',
displayName: secondarySessionName,
clientCount: 0,
hasActivePrompt: false,
},
] satisfies DaemonSessionSummary[];
const scenario = createWebShellDaemonScenario({
workspaceCwd: primaryCwd,
displayName: primarySessionName,
sessions,
Comment on lines 810 to +813

@qwen-code-ci-bot qwen-code-ci-bot Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The scenario overrides workspaceCwd, displayName and sessions but not sessionId, so the loaded session (builder default web-shell-e2e-session) is present in no seeded workspace listing — the row the test waits for (workspace-primary-session) merely shares its display name. App.tsx's loaded-session reconciliation therefore misses on page.sessions.find(...) and silently takes the summary.displayName fallback, so the capture exercises the fallback path instead of the normal listing-hit path and depicts a state a real daemon cannot produce (a client connected to a session no workspace lists). The sidebar also marks the current row by session identity (isCurrentSession compares the connection's sessionId), so no row in this capture is ever flagged as current, and any regression in the current-session treatment (highlight, aria-current, action gating) stays invisible in the only multi-workspace capture. Aligning the loaded session with the seeded primary session fixes both:

Suggested change
const scenario = createWebShellDaemonScenario({
workspaceCwd: primaryCwd,
displayName: primarySessionName,
sessions,
const scenario = createWebShellDaemonScenario({
workspaceCwd: primaryCwd,
displayName: primarySessionName,
sessions,
sessionId: 'workspace-primary-session',

goto/SSE keying all derive from scenario.sessionId, so this stays self-consistent — and the adjacent "Wait for the loaded session's row" comment then matches reality too.

— qwen3.8-max via Qwen Code /review (v0.22.2)

capabilities: {
workspaces: [
{
Expand All @@ -798,7 +821,7 @@ for (const theme of THEMES) {
},
{
id: 'ws-api',
cwd: '/tmp/qwen-api-service',
cwd: secondaryCwd,
primary: false,
trusted: true,
},
Expand All @@ -823,7 +846,8 @@ for (const theme of THEMES) {
// per-workspace fetch. Wait for the loaded session's row before capturing
// so the async load has settled — otherwise the row list races the
// screenshot and the capture differs between runs.
await expect(sidebar.getByText(primarySessionName)).toBeVisible();
await expect(sidebar.getByText(primarySessionName)).toHaveCount(1);
await expect(sidebar.getByText(secondarySessionName)).toHaveCount(1);
await captureScreenshot(page, `workspace-sidebar-${theme}`);
});

Expand Down
Loading