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 6 commits
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
2 changes: 1 addition & 1 deletion packages/cli/src/serve/run-qwen-serve.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14420,10 +14420,10 @@ describe('runQwenServe channel worker supervisor', () => {
expect(portsAttempted).toEqual([4170, 4171]);
expect(handle.server.listening).toBe(true);
expect(handle.url).toMatch(/^http:\/\/127\.0\.0\.1:\d+$/);
expect(handle.url).not.toContain(':4170');
expect(new URL(handle.url).port).toBe(
String((handle.server.address() as AddressInfo).port),
);
expect(new URL(handle.url).port).not.toBe('4170');
expect(
stderrWrites.some((w) =>
w.includes('port 4170 is in use, trying 4171'),
Expand Down
85 changes: 59 additions & 26 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,68 @@ 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 workspaceSessions = scenario.sessions.filter(
(session) => session.workspaceCwd === workspaceCwd,
);
Comment on lines +640 to +642

@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] R4-1: The workspace scoping added here for the full-catalog sessions routes has no direct test, and its interaction with the group === 'pinned' branch is exercised by no multi-workspace scenario. Live-state scoping has a dedicated direct-fetch test (scopes live-state sessions to the requested workspace), but no spec fetches /workspaces/:cwd/sessions or .../sessions?group=pinned or asserts their response content: the only isPinned fixture (web-shell.channels.spec.ts) is single-workspace, and the two-workspace workspace sidebar visual test seeds no pinned sessions, so the pinned bucket is never fetched across workspaces. If a future edit of filterScenarioSessions bypassed the workspace filter on the pinned branch (filtering scenario.sessions instead of workspaceSessions when group === 'pinned'), nothing would go red — the mock would silently return other workspaces' pinned sessions for a workspace-scoped request, re-introducing exactly the cross-workspace duplication this PR exists to eliminate.

Consider adding a sibling to scopes live-state sessions to the requested workspace in web-shell.session-live-state.spec.ts: a two-workspace scenario where the non-requested workspace also has an isPinned session, then direct page.evaluate fetches of /workspaces/${encodeURIComponent(cwd)}/sessions and .../sessions?group=pinned asserting the exact returned sessionId lists contain only the requested workspace's sessions. The new test must go red if the workspace filter is skipped on the group === 'pinned' or sourceType branches — prove it with the mutant that filters unscoped scenario.sessions on the pinned branch.

中文说明

[建议] R4-1：这里为全量 catalog sessions 路由新增的 workspace 过滤没有直接测试，它与 group === 'pinned' 分支的组合也没有任何多 workspace 场景覆盖。live-state 过滤有专门的直接请求测试（scopes live-state sessions to the requested workspace），但没有任何 spec 直接请求 /workspaces/:cwd/sessions 或 .../sessions?group=pinned 并断言响应内容：唯一的 isPinned fixture（web-shell.channels.spec.ts）是单 workspace 的，双 workspace 的 workspace sidebar 可视化测试也没有准备 pinned session，因此 pinned bucket 从未在跨 workspace 场景下被请求。如果未来对 filterScenarioSessions 的修改绕过了 pinned 分支上的 workspace 过滤（比如在 group === 'pinned' 时过滤 scenario.sessions 而不是 workspaceSessions），不会有任何测试变红——mock 会悄悄为按 workspace 的请求返回其他 workspace 的 pinned sessions，重新引入这个 PR 要消除的跨 workspace 重复。

建议在 web-shell.session-live-state.spec.ts 中增加一个 scopes live-state sessions to the requested workspace 的同族测试：双 workspace 场景，其中未被请求的 workspace 还有一个 isPinned session，然后通过 page.evaluate 直接请求 /workspaces/${encodeURIComponent(cwd)}/sessions 和 .../sessions?group=pinned，断言返回的 sessionId 列表只包含被请求 workspace 的 session。如果在 group === 'pinned' 或 sourceType 分支上跳过 workspace 过滤，新测试必须变红——可以用「在 pinned 分支上过滤未过滤的 scenario.sessions」这个变异体来证明。

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

const sourceSessions = sourceType
? scenario.sessions.filter(
? workspaceSessions.filter(
(session) =>
session.sourceType === sourceType ||
(sourceType === 'default' && session.sourceType === undefined),
)
: scenario.sessions;
: workspaceSessions;
return group === 'pinned'
? sourceSessions.filter((session) => Boolean(session.isPinned))
: sourceSessions;
}

type WorkspaceSessionsRouteMatch = {
workspaceCwd: string;
liveState: boolean;
};

function decodeRouteSegment(segment: string): string | undefined {
try {
return decodeURIComponent(segment);
} catch {
return undefined;
}
}

function matchWorkspaceSessionsRoute(
path: string,
): WorkspaceSessionsRouteMatch | undefined {
const liveStateMatch = path.match(
/^\/workspaces\/([^/]+)\/sessions\/live-state\/?$/,
);
if (liveStateMatch) {
const workspaceCwd = decodeRouteSegment(liveStateMatch[1]);
if (workspaceCwd === undefined) return undefined;
return {
workspaceCwd,
liveState: true,
};
}

const sessionsMatch =
path.match(/^\/workspaces\/([^/]+)\/sessions\/?$/) ??
path.match(/^\/workspace\/([^/]+)\/sessions\/?$/);
if (!sessionsMatch) {
return undefined;
}
const workspaceCwd = decodeRouteSegment(sessionsMatch[1]);
if (workspaceCwd === undefined) return undefined;

return {
workspaceCwd,
liveState: false,
};
}

function isDaemonPath(path: string): boolean {
return (
path === '/health' ||
Expand All @@ -671,9 +718,7 @@ function isDaemonPath(path: string): boolean {
) ||
/^\/workspaces\/[^/]+\/channels\/[^/]+\/pairing-approvals\/?$/.test(path) ||
/^\/workspaces\/[^/]+\/channels\/[^/]+\/?$/.test(path) ||
/^\/workspace\/.+\/sessions\/?$/.test(path) ||
/^\/workspaces\/[^/]+\/sessions\/?$/.test(path) ||
/^\/workspaces\/[^/]+\/sessions\/live-state\/?$/.test(path) ||
Boolean(matchWorkspaceSessionsRoute(path)) ||
/^\/workspace\/.+\/session-groups\/?$/.test(path) ||
/^\/workspaces\/[^/]+\/session-groups\/?$/.test(path) ||
/^\/workspaces\/.+\/git\/?$/.test(path) ||
Expand Down Expand Up @@ -754,17 +799,8 @@ function isDaemonRoute(method: string, path: string): boolean {
) {
return true;
}
if (
method === 'GET' &&
/^\/workspaces\/[^/]+\/sessions\/live-state\/?$/.test(path)
) {
return true;
}
if (
method === 'GET' &&
(/^\/workspace\/.+\/sessions\/?$/.test(path) ||
/^\/workspaces\/[^/]+\/sessions\/?$/.test(path))
) {
const workspaceSessionsRoute = matchWorkspaceSessionsRoute(path);
if (method === 'GET' && workspaceSessionsRoute) {
return true;
}
if (
Expand Down Expand Up @@ -1005,14 +1041,14 @@ async function handleDaemonRoute(
await json(route, workspaceMcpResources(scenario, serverName));
return;
}
if (
method === 'GET' &&
/^\/workspaces\/[^/]+\/sessions\/live-state\/?$/.test(path)
) {
const workspaceSessionsRoute = matchWorkspaceSessionsRoute(path);
if (method === 'GET' && workspaceSessionsRoute?.liveState) {
const { workspaceCwd } = workspaceSessionsRoute;
await json(route, {
v: 1,
catalogVersion: scenario.sessionCatalogVersion,
sessions: scenario.sessions
.filter((session) => session.workspaceCwd === workspaceCwd)
.filter(
(session) =>
(session.clientCount ?? 0) > 0 ||
Expand All @@ -1030,13 +1066,10 @@ async function handleDaemonRoute(
});
return;
}
if (
method === 'GET' &&
(/^\/workspace\/.+\/sessions\/?$/.test(path) ||
/^\/workspaces\/[^/]+\/sessions\/?$/.test(path))
) {
if (method === 'GET' && workspaceSessionsRoute) {
const { workspaceCwd } = workspaceSessionsRoute;
await json(route, {
sessions: filterScenarioSessions(scenario, searchParams),
sessions: filterScenarioSessions(scenario, searchParams, workspaceCwd),
});
return;
}
Expand Down
31 changes: 28 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,33 @@ 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)

sessionId: 'workspace-primary-session',
capabilities: {
workspaces: [
{
Expand All @@ -798,7 +822,7 @@ for (const theme of THEMES) {
},
{
id: 'ws-api',
cwd: '/tmp/qwen-api-service',
cwd: secondaryCwd,
primary: false,
trusted: true,
},
Expand All @@ -823,7 +847,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
Loading