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 3 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
25 changes: 22 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 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;
}

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]);

@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] R1-1: (fix-induced) The round-2 fix closed the round-1 input — the extractor now throws instead of returning undefined, and both consumers filter unconditionally — but it opened a new mechanism at the same site. The throw couples the two route guards to the two patterns inside workspaceCwdFromSessionsPath: four independent regexes, ~370 lines apart, that must silently stay in agreement, and the route callback in installMockDaemon has no try/catch around handleDaemonRoute. The hardcoded legacy capture also diverges from the real daemon's route semantics: the legacy guard admits .+ (spanning /), while the real daemon's Express :id param matches a single segment.

If a future edit widens or adds a sessions route guard without touching this helper, workspaceCwdFromSessionsPath throws Unrecognized sessions path inside the Playwright route callback; nothing catches it, so the catalog/live-state fetch never resolves and unrelated e2e tests fail with a hang or an error pointing at the mock rather than the route. And a request like /workspace/a/b/sessions gets a 200 with sessions: [] from the mock where the real daemon would 404, so an empty-state assertion can pass against behavior the real daemon never serves. Verified by running the mock's literal regexes and the repo's own express 5.2.1 side by side:

MOCK path /workspace/a/b/sessions | legacy capture: "a/b" (→ 200, sessions filtered by cwd "a/b" → [])
EXPRESS 404 /workspace/a/b/sessions -> {"matched":false}
EXPRESS 200 /workspace/%2Ftmp%2Fa/sessions -> {"matched":"GET /workspace/:id/sessions","id":"/tmp/a"}

Match-and-capture in one place: use the capture groups of the guard regexes themselves (hoist the match into the handler and drop the separate helper), and align the legacy pattern to [^/]+ to mirror Express :id single-segment semantics. That leaves exactly two patterns, both living where the route is dispatched, and the guard itself guarantees the capture succeeded.

中文说明

[Suggestion] R1-1：（由上轮修复引入）第 2 轮的修复关闭了第 1 轮报告的输入——提取函数现在抛出异常而不是返回 undefined，两个消费方也都无条件过滤——但它在同一位置引入了新的机制。这个 throw 把两个路由守卫和 workspaceCwdFromSessionsPath 内部的两个正则耦合在了一起：四个独立的正则相距约 370 行，必须悄悄保持一致，而 installMockDaemon 的路由回调在 handleDaemonRoute 外没有 try/catch。硬编码的 legacy 捕获还与真实 daemon 的路由语义存在偏差：legacy 守卫接受 .+（可跨 /），而真实 daemon 的 Express :id 参数只匹配单段。

如果未来某个改动拓宽或新增了 sessions 路由守卫却没有同步修改这个辅助函数，workspaceCwdFromSessionsPath 会在 Playwright 路由回调里抛出 Unrecognized sessions path；由于没有捕获，catalog/live-state 请求永远不会 resolve，不相关的 e2e 测试会以挂起失败，或报出指向 mock 而非路由的错误。另外，像 /workspace/a/b/sessions 这样的请求会从 mock 拿到 200 和空 sessions: []，而真实 daemon 会返回 404，于是空状态断言可能在真实 daemon 根本不会提供的行为上通过。上面的 probe 输出是用 mock 的原始正则和仓库自带的 express 5.2.1 实际运行对比得出的。

建议在一处完成匹配和捕获：使用守卫正则自身的捕获组（把匹配提升到 handler 里，删掉独立的辅助函数），并把 legacy 模式对齐为 [^/]+，与 Express :id 的单段语义一致。这样只留下两个模式，且都在路由分发处，守卫本身即可保证捕获成功。

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


throw new Error(`Unrecognized sessions path: ${path}`);
}

function isDaemonPath(path: string): boolean {
return (
path === '/health' ||
Expand Down Expand Up @@ -1009,10 +1025,12 @@ 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) => session.workspaceCwd === workspaceCwd)
.filter(
(session) =>
(session.clientCount ?? 0) > 0 ||
Expand All @@ -1035,8 +1053,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
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
85 changes: 85 additions & 0 deletions packages/web-shell/client/e2e/web-shell.session-live-state.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { expect, test } from '@playwright/test';
import type { DaemonSessionSummary } from '@qwen-code/sdk/daemon';
import {
createWebShellDaemonScenario,
installMockDaemon,
Expand Down Expand Up @@ -55,3 +56,87 @@ test('uses live-state instead of polling the full session catalog @smoke', async
.toBeGreaterThan(liveRequestsAfterSourceChange);
expect(fullCatalogRequests()).toBe(requestsAfterSourceChange);
});

test('scopes live-state sessions to the requested workspace', async ({
page,
}, testInfo) => {
const primaryCwd = '/tmp/qwen-live-primary';
const secondaryCwd = '/tmp/qwen-live-secondary';
const sessions = [
{
sessionId: 'primary-live',
workspaceCwd: primaryCwd,
createdAt: '2026-07-03T00:00:00.000Z',
updatedAt: '2026-07-03T00:00:00.000Z',
displayName: 'Primary live',
clientCount: 1,
hasActivePrompt: false,
},
{
sessionId: 'secondary-live',
workspaceCwd: secondaryCwd,
createdAt: '2026-07-03T00:00:00.000Z',
updatedAt: '2026-07-03T00:00:00.000Z',
displayName: 'Secondary live',
clientCount: 1,
hasActivePrompt: false,
},
] satisfies DaemonSessionSummary[];
const scenario = createWebShellDaemonScenario({
workspaceCwd: primaryCwd,
sessionId: 'primary-live',
displayName: 'Primary live',
sessions,
capabilities: {
features: [
'session_events',
'session_source_metadata',
'workspace_session_live_state',
],
workspaces: [
{
id: 'primary',
cwd: primaryCwd,
primary: true,
trusted: true,
},
{
id: 'secondary',
cwd: secondaryCwd,
primary: false,
trusted: true,
},
],
},
});
await installMockDaemon(page, scenario, {
baseURL: String(testInfo.project.use.baseURL),
});
const baseURL = String(testInfo.project.use.baseURL);

const primaryState = await page.evaluate(
async ({ baseURL, cwd }) => {
const response = await fetch(
`${baseURL}/workspaces/${encodeURIComponent(cwd)}/sessions/live-state`,
);
return response.json();
},
{ baseURL, cwd: primaryCwd },
);
const secondaryState = await page.evaluate(
async ({ baseURL, cwd }) => {
const response = await fetch(
`${baseURL}/workspaces/${encodeURIComponent(cwd)}/sessions/live-state`,
);
return response.json();
},
{ baseURL, cwd: secondaryCwd },
);

expect(primaryState.sessions.map((session) => session.sessionId)).toEqual([
'primary-live',
]);
expect(secondaryState.sessions.map((session) => session.sessionId)).toEqual([
'secondary-live',
]);
});
Loading