Skip to content

fix(web-shell): report intended workspace to host when starting a new chat - #7910

Merged
wenshao merged 3 commits into
QwenLM:mainfrom
wenshao:fix/web-shell-new-session-workspace
Jul 28, 2026
Merged

fix(web-shell): report intended workspace to host when starting a new chat#7910
wenshao merged 3 commits into
QwenLM:mainfrom
wenshao:fix/web-shell-new-session-workspace

Conversation

@wenshao

@wenshao wenshao commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

When you start a new chat in a multi-workspace Web Shell, the workspace reported to the host (and therefore shown in the composer's workspace chip/selector) is now the workspace you actually picked for the new chat. Previously, if another workspace had the active session — for example one with a running task — the new chat could be reported against that other workspace instead, so the composer showed the wrong workspace right after the new session appeared.

Why it's needed

Clearing a session to start a fresh chat drops the session id but keeps the connection's last workspace around as a leftover. The notification that tells the host "the view changed" read that leftover workspace, so a "new chat in workspace A" was routed back to the previous workspace B. The host then re-mounted the shell for workspace B, and the composer displayed B even though the user asked for A. The leftover workspace only matters when there is no active session, which is exactly the state a cleared/new chat is in — so with no active session we now report the workspace picked for the next session rather than the stale connection workspace. Behavior with an active session is unchanged.

Reviewer Test Plan

How to verify

  • Unit: a regression test covers the exact state — no active session, a stale connection workspace pointing at a secondary workspace, and the next-session workspace set to the primary. It asserts the host is notified with the primary, not the secondary. This test fails on main (reports the stale secondary) and passes with this change.
  • Manual (multi-workspace deployment): with workspaces A and B registered, start a long-running task in B so B holds the active session, then click "new task" under workspace A in the sidebar. Expected: the composer's workspace indicator shows A. Observed before the fix: it showed B.

Evidence (Before & After)

Verified via the regression test in client/App.test.tsx:

  • Before (stashed fix): reports the selected workspace... fails — onSessionIdChange was called with (undefined, 'secondary', '/work/secondary').
  • After: the same test passes — onSessionIdChange is called with (undefined, undefined, '/workspace') and never with the stale secondary.
  • Full client/App.test.tsx suite: 234 passed.

Manual TUI capture in a real multi-workspace daemon was not recorded; the routing behavior is exercised by the unit test above.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

Unit tests only (npx vitest run client/App.test.tsx in packages/web-shell), plus package typecheck and ESLint.

Risk & Scope

  • Main risk or tradeoff: with no active session, changing the next-session workspace now also notifies the host (previously it could stay silent and leave the host route stale). The existing last-notified guard prevents duplicate notifications, and the full App suite passes.
  • Not validated / out of scope: no manual multi-workspace TUI recording; behavior with an active session is intentionally unchanged.
  • Breaking changes / migration notes: none.

Linked Issues

中文说明

这个 PR 做了什么

在多 workspace 的 Web Shell 中新建会话时,上报给 host(也就是 composer 里 workspace 标签/选择器显示的)workspace,现在会是你真正为本次新会话选择的 workspace。之前,如果另一个 workspace 持有当前活跃会话(例如它有正在运行的任务),新会话可能被上报成那个 workspace,导致新会话出现后 composer 立刻显示错误的 workspace。

为什么需要

清空会话以开启新聊天时,会清掉 session id,但 connection 上一次的 workspace 会作为残留保留下来。那个告诉 host「视图变了」的通知读取了这个残留 workspace,于是「在 A 新建会话」被路由回了上一个 workspace B。host 随后按 workspace B 重新挂载 shell,composer 就显示 B,即使用户要的是 A。残留 workspace 只在没有活跃会话时才有影响,而清空/新建会话恰恰就处于这种状态——所以在没有活跃会话时,现在上报「为下一个会话选择的 workspace」,而不是残留的 connection workspace。有活跃会话时的行为保持不变。

评审测试计划

如何验证

  • 单测:一个回归测试覆盖了精确状态——无活跃会话、残留的 connection workspace 指向某个 secondary workspace、下一会话 workspace 设为 primary。它断言 host 收到的是 primary,而不是 secondary。该测试在 main 上失败(上报残留的 secondary),在本改动后通过。
  • 手动(多 workspace 部署):注册 workspace A 和 B,在 B 中启动一个长时间运行的任务使 B 持有活跃会话,然后点击侧栏 A 下的「new task」。预期:composer 的 workspace 指示显示 A。修复前观察到:显示 B。

证据(Before & After)

通过 client/App.test.tsx 中的回归测试验证:

  • 修复前(stash 掉修复):reports the selected workspace... 失败——onSessionIdChange 被以 (undefined, 'secondary', '/work/secondary') 调用。
  • 修复后:同一测试通过——onSessionIdChange 被以 (undefined, undefined, '/workspace') 调用,且从不以残留的 secondary 调用。
  • 完整 client/App.test.tsx 套件:234 通过。

未在真实多 workspace daemon 中录制手动 TUI;上述路由行为由单测覆盖。

测试环境

OS 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

环境(可选)

仅单测(在 packages/web-shellnpx vitest run client/App.test.tsx),外加 package typecheck 与 ESLint。

风险与范围

  • 主要风险/权衡:在没有活跃会话时,更改下一会话 workspace 现在也会通知 host(之前可能保持沉默,使 host 路由保持过期)。已有的 last-notified 守卫避免了重复通知,完整 App 套件通过。
  • 未验证/超出范围:未做手动多 workspace TUI 录制;有活跃会话时的行为有意保持不变。
  • 破坏性变更/迁移说明:无。

关联 Issue

… chat

Clearing a session leaves connection.workspaceCwd pointing at the previous session's workspace. The onSessionIdChange notification read that stale value, so starting a new chat in workspace A routed the host back to the old workspace (e.g. one with a running task) and the composer showed the wrong workspace. With no active session, report the workspace picked for the next session instead.
@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: this is an observed bug, not theoretical hardening. The PR carries a regression test that fails on main (host notified with the stale secondary workspace) and passes with the fix — that is concrete before/after evidence. The leftover connection.workspaceCwd after a session clear is a real state the new-chat path lands in.

Direction: aligned. Multi-workspace routing in the Web Shell is squarely in scope, and the fix makes the host notification agree with what the composer chip already reports. No auth/sandbox/model-selection/telemetry surface is touched. CHANGELOG has no direct reference, but the area is clearly relevant.

Size: not applicable — packages/web-shell/client/ is not a core module path. 11 production lines added / 4 removed in App.tsx, plus 155 test lines.

Approach: the scope feels right and genuinely minimal. Rather than adding new state, it reuses the existing activeWorkspaceCwd memo (already the source of truth for the composer chip and the git-status effect) as the reported workspace, falling back to connection.workspaceCwd. That keeps the host and the chip in agreement by construction. I don't see a simpler path, and there are no drive-by changes to split out.

Risk: no elevated risk signals — none of the changed files match the high-risk revert-correlated paths.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:这是一个已观测到的 bug,而非理论性加固。PR 附带了一个回归测试,在 main 上失败(host 被通知为残留的 secondary workspace),在修复后通过——这是具体的 before/after 证据。会话清空后残留的 connection.workspaceCwd 是新建会话路径确实会落入的状态。

方向:对齐。Web Shell 的多 workspace 路由完全在范围内,且修复让 host 通知与 composer 标签已上报的内容保持一致。未触及 auth/sandbox/模型选择/telemetry 等敏感面。CHANGELOG 无直接引用,但该领域明显相关。

规模:不适用——packages/web-shell/client/ 不是核心模块路径。App.tsx 中 11 行生产代码新增 / 4 行删除,外加 155 行测试。

方案:范围合理且真正最小化。它没有新增状态,而是复用已有的 activeWorkspaceCwd memo(它已是 composer 标签和 git-status effect 的事实来源)作为上报的 workspace,并回退到 connection.workspaceCwd。这从构造上就让 host 与标签保持一致。我没看到更简路径,也没有需要拆出去的顺手改动。

风险:无升级风险信号——改动文件均未命中与 revert 相关的高风险路径。

进入代码审查 🔍

Qwen Code · qwen3.8-max-preview

Reviewed at 26b4fd7b031c6e73edd6499b9bff9b7b11f1d9bc · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Code review

My independent take before reading the diff: when no session is active, the host notification should report the same workspace the composer chip shows (the next-session selection), not the stale connection.workspaceCwd left over from the cleared session. The PR does exactly this — and via the cleanest possible mechanism: it reuses the existing activeWorkspaceCwd memo rather than introducing new state.

A few things I verified in the code:

  • When a session is active, activeWorkspaceCwd === connection.workspaceCwd, so reportedWorkspaceCwd collapses to the old value — the active-session path is provably unchanged, which test 3 (keeps reporting the active session workspace…) nails down.
  • When no session is active, activeWorkspaceCwd resolves locked ?? selected ?? primary, i.e. the workspace picked for the next session — the value the chip and the git-status effect already use. The host and the chip now agree by construction.
  • workspaceId is derived from the same reportedWorkspaceCwd, so the reported id and cwd stay consistent rather than drifting.
  • The ?? connection.workspaceCwd fallback only bites when activeWorkspaceCwd is undefined (no locked/selected/primary workspace at all) — a defensive edge that preserves prior behavior, not a behavior change.
  • The last-notified guard now compares against reportedWorkspaceCwd, and activeWorkspaceCwd is correctly added to the effect's dependency array, so the notification re-fires on each deferred workspace switch (test 4) without duplicating.

No correctness, security, or regression concerns. No convention violations — this is a surgical 11-line change that reuses existing infrastructure rather than adding a parallel helper. The four new tests target the exact states that matter (explicit selection, unset→primary fallback, active-session-unchanged, and re-notification on switch).

Test evidence

This is an unattended CI run, so I'm quoting the PR's own CI checks on the reviewed commit rather than running anything. The directly relevant lanes are green:

Check Conclusion
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success
Capture web-shell visuals (ubuntu-latest, Node 22.x) ✅ success
precheck-pr / precheck ✅ success
Test (macos-latest, Node 22.x) ⏭️ skipped
Test (windows-latest, Node 22.x) ⏭️ skipped
Integration Tests (CLI, No Sandbox) ⏭️ skipped

The web-shell E2E Smoke lane passing is the meaningful signal here — it exercises the Web Shell package this change lives in. The unit suite (Test, ubuntu) is green, which includes the new App.test.tsx regression tests. macOS/Windows test lanes and the CLI integration lane are skipped (fork PR), not failed — no red checks anywhere on this commit.

Not verified: a manual multi-workspace TUI reproduction. The routing behavior is covered by the unit regression tests and the web-shell E2E smoke; a maintainer can trigger the isolated @qwen-code /tmux job if a live multi-workspace capture is wanted before merge.

中文说明

代码审查

读 diff 前我的独立判断:当没有活跃会话时,host 通知应上报 composer 标签所显示的同一 workspace(下一会话的选择),而非清空会话后残留的 connection.workspaceCwd。PR 正是这么做的——而且用了最干净的机制:复用已有的 activeWorkspaceCwd memo,而非引入新状态。

我在代码中核实的几点:

  • 有活跃会话时,activeWorkspaceCwd === connection.workspaceCwd,所以 reportedWorkspaceCwd 退化为旧值——活跃会话路径可证明地保持不变,测试 3 正好锁定这一点。
  • 无活跃会话时,activeWorkspaceCwd 解析为 locked ?? selected ?? primary,即为下一会话选择的 workspace——这正是标签和 git-status effect 已在使用的值。host 与标签从构造上保持一致。
  • workspaceId 由同一 reportedWorkspaceCwd 派生,id 与 cwd 不会漂移。
  • ?? connection.workspaceCwd 回退仅在 activeWorkspaceCwd 为 undefined(完全没有 locked/selected/primary workspace)时生效——是保留旧行为的防御性边界,而非行为变化。
  • last-notified 守卫现在与 reportedWorkspaceCwd 比较,且 activeWorkspaceCwd 被正确加入 effect 依赖数组,因此通知会在每次延迟的 workspace 切换时重新触发(测试 4),同时不重复。

无正确性、安全性或回归问题。无规范违规——这是一个 11 行的外科手术式改动,复用既有基础设施而非新增并行 helper。四个新测试精确覆盖了关键状态(显式选择、未设置→primary 回退、活跃会话不变、切换时重新通知)。

测试证据

这是无人值守的 CI 运行,因此我引用 PR 自身在被审 commit 上的 CI 检查,而非自行运行任何东西。直接相关的流水线为绿:单元套件(ubuntu)通过(含新增的 App.test.tsx 回归测试),web-shell E2E Smoke 通过(这是最有意义的信号,因为它演练了本改动所在的 Web Shell 包),web-shell 视觉捕获通过。macOS/Windows 测试与 CLI 集成流水线为 skipped(fork PR),非失败——该 commit 上无任何红色检查。

未验证:手动多 workspace TUI 复现。路由行为已由单元回归测试和 web-shell E2E smoke 覆盖;若合并前需要真实多 workspace 抓取,maintainer 可触发隔离的 @qwen-code /tmux 任务。

Qwen Code · qwen3.8-max-preview

Reviewed at 26b4fd7b031c6e73edd6499b9bff9b7b11f1d9bc · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Confidence: 5/5 — a surgical, well-tested fix that reuses existing infrastructure; I'd merge without hesitation.

Stepping back: the problem is real and demonstrated (the regression test fails on main), the fix is the minimal version of what I'd have proposed independently — reuse the activeWorkspaceCwd memo the chip already trusts, rather than bolt on new state — and the active-session path is provably unchanged. Every line in the diff earns its place; there's no scope creep, no drive-by refactor, no new abstraction to maintain. In six months this reads as an obvious, well-scoped bugfix, not something to curse.

CI is green on the reviewed commit, including the web-shell E2E smoke that exercises this exact package, and there are no pending checks. Approving now, pinned to the reviewed commit.

中文说明

置信度:5/5 —— 一个外科手术式、充分测试的修复,复用既有基础设施;我会毫不犹豫地合并。

退一步看:问题是真实且有演示的(回归测试在 main 上失败),修复正是我独立会提出的最小版本——复用标签已信任的 activeWorkspaceCwd memo,而非新增状态——且活跃会话路径可证明地保持不变。diff 中每一行都物有所值;没有范围蔓延、没有顺手重构、没有需要维护的新抽象。六个月后这读起来是一个显而易见、范围良好的 bugfix,而非令人懊恼的东西。

被审 commit 上 CI 为绿,包括演练本包正好所在包的 web-shell E2E smoke,且无待处理检查。现在批准,固定到被审 commit。

Qwen Code · qwen3.8-max-preview

Reviewed at 26b4fd7b031c6e73edd6499b9bff9b7b11f1d9bc · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

🖼️ web-shell visual preview

Rendered against a mock daemon (no real backend): the PR base vs this PR head 26b4fd7. Only screenshots that changed are shown (flows below, if any, are head-only) — refreshes on every push.

Screenshots · before / after

ℹ️ No screenshot changed against the PR base — but this PR edits 1 render-shaping file:

  • packages/web-shell/client/App.tsx

Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to packages/web-shell/client/e2e/visuals/screenshots.spec.ts that seeds whatever state the UI is gated on; it then appears here as a head-only (NEW) capture.

Full-resolution recordings (.webm) are attached to the workflow run.

Qwen Code · web-shell visuals

@qwen-code-ci-bot qwen-code-ci-bot left a comment

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.

LGTM, looks ready to ship — CI landed green after the review. ✅

@wenshao

wenshao commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /takeover

@qwen-code-dev-bot qwen-code-dev-bot added the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Jul 28, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the autofix/takeover label (or comment @qwen-code /takeover stop) to release.

中文说明

🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 autofix/takeover 标签(或评论 @qwen-code /takeover stop)即可释放。

@qwen-code-dev-bot

qwen-code-dev-bot commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

AutoFix round 2 finishedview run. See this round's report below.

中文说明

AutoFix 第 2 轮已完成 —— 查看运行。本轮报告见下方。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

Autofix review round — no action needed

No actionable review feedback was found for PR #7910 in this round:

  • Reviews: none
  • Inline comments: none
  • Issue-level comments: one informational bot message (web-shell visual preview) — no change requested
  • Failed checks: review-pr is listed as failed but produced no findings in the feedback

The PR diff (2 files, +51/−4) is unchanged. No code changes were made.

中文说明

Autofix 审查轮次 — 无需操作

本轮在 PR #7910 中未发现需要处理的审查反馈:

  • 审查意见:无
  • 行内评论:无
  • Issue 级别评论:一条来自机器人的信息性消息(web-shell 视觉预览)——未要求任何更改
  • 失败的检查review-pr 被列为失败,但在反馈中未产生任何具体发现

PR 差异(2 个文件,+51/−4)未发生变化。本轮未进行任何代码更改。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Reviewed. Suggestions are inline.

中文说明

已审查。 建议见行内评论。

— qwen3.8-max-preview via Qwen Code /review

Comment thread packages/web-shell/client/App.tsx Outdated
@wenshao

wenshao commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

Local verification report — real build, real browser

I verified this locally by rendering the real <App/> in headless Chromium against a stubbed daemon, driving the real sidebar buttons, and A/B-ing client/App.tsx between the merge base (b3873571a) and this head (f18c257e3). Full unit suites were run on both arms too.

Verdict: the mechanism is correct and the added regression test is non-vacuous — but the fix only covers half the entry points. The half it misses includes the PR's own manual repro. One line closes the rest; details and a verified patch below.


✅ What is verified working

Check Result
client/App.test.tsx on this head 234 / 234 pass
Whole @qwen-code/web-shell suite on this head 2626 pass; the only 10 failures are build-artifact.test.ts (ENOENT … /dist/index.js — needs npm run build, fails identically on base)
New regression test is non-vacuous On base it fails with received (undefined, 'secondary', '/work/secondary') — it really pins the leftover workspace
Active-session behaviour unchanged A deliberately conflicting next-session pick does not leak into an active session's report
Dead session (missingSession) Still silent, as before
Deferred workspace switches Base emitted only the initial report; this head emits one per switch — the behaviour change flagged in Risk & Scope, working as described

Real-browser proof of the fix, starting from the state the PR describes (session-1 live in non-primary wsA), clicking “New task” under another non-primary workspace wsB:

fixed

Left panel is a host that routes on onSessionIdChange and re-mounts the shell, mirroring WorkspaceSessionProvider. Before: host routed back to wsA, composer chip stuck on a. After: host routes to wsB, chip shows b. Exactly as intended.


⚠️ Finding 1 (main) — undefined means primary here, not “unknown”

selectedWorkspaceCwd === undefined is not “no selection” in this codebase; it is how “the primary workspace” is spelled:

  • client/components/WorkspaceSelector.tsx:130onSelectWorkspace(next.primary ? undefined : next.cwd)
  • client/components/sidebar/WebShellSidebar.tsx:4231const wsCwd = ws.primary ? undefined : ws.cwd; (passed to handleNewSession at :4274)
  • the sidebar's top-level New task (:3999) calls handleNewSession() with no argument at all, and so do /clear, /new, /reset, shellApi.createNewSession(), the missing-session recovery, the new-session suggestion, scheduled-tasks “create via chat”, and Goals — every one of them lands in createNewSession()setSelectedWorkspaceCwd(undefined).

So on all of those paths selectedWorkspaceCwd ?? connection.workspaceCwd (App.tsx:4627) falls straight back to the same stale value the PR is trying to avoid.

This includes the PR's own manual test plan. “Click new task under workspace A” routes through wsCwd = undefined whenever A is the primary project — the common case. Driving that exact click in a real browser, the BASE and PR screenshots came out byte-identical (md5 acc1afc2… for both):

gap-primary

Same for the sidebar's top-level New task button (BASE and PR shots again byte-identical, md5 4564707e…):

gap-toplevel

The divergence is not cosmetic. After /clear in a secondary-workspace session, I let the deferred session actually get created and recorded both sides:

daemon asked to create the session in = "/workspace"   (primary)
composer chip                          = "/workspace"
host was told                          = (undefined, "secondary", "/work/secondary")

ensureSessionForPrompt resolves the new session's workspace as lockedWorkspaceCwd ?? acceptedWorkspaceCwd ?? primaryWorkspaceCwd (App.tsx:3312) — connection.workspaceCwd plays no part. So the host is routed to a workspace the next session will not live in.

Suggested fix (one line, verified)

activeWorkspaceCwd (App.tsx:1570) already means exactly “the session's workspace, else the workspace picked for the next session (locked / selected / primary)”, and it is what the composer chip and getComposerWorkspaceCwd() (App.tsx:3359) use. Reusing it makes the host report agree with the chip by construction:

-    const reportedWorkspaceCwd = connection.sessionId
-      ? connection.workspaceCwd
-      : (selectedWorkspaceCwd ?? connection.workspaceCwd);
+    const reportedWorkspaceCwd = activeWorkspaceCwd ?? connection.workspaceCwd;
     onSessionIdChange,
-    selectedWorkspaceCwd,
+    activeWorkspaceCwd,
     workspace.capabilities,

I ran this: all three browser scenarios above turn correct (right panels of the last two screenshots), the created-vs-reported workspace agree, your new regression test still passes, App.test.tsx stays 234 / 234, and the whole package stays at 2626 passing. It also removes the third near-duplicate copy of the same intent.


⚠️ Finding 2 (Low) — an untrusted workspace is now reported for one tick

The sidebar's onSelectWorkspace={setSelectedWorkspaceCwd} writes the selection with no trust check; the trust-clearing effect (App.tsx:1509-1518) resets it one tick later. With no active session, base emitted one report (the stale workspace); this head emits two, the first being (undefined, 'untrusted', '/work/untrusted').

Nothing unsafe gets created — ensureSessionForPrompt still filters trusted === true — but a routing host will briefly mount the untrusted workspace's route. The suggested one-liner also improves the follow-up (falls back to primary instead of the stale workspace).


📝 Finding 3 (docs) — the shipped standalone shell cannot show this

Worth a note in the description, because a reviewer following “Manual (multi-workspace deployment)” on qwen serve will see nothing:

  • client/main.tsx:150handleSessionIdChange takes only (sessionId, workspaceId); the third argument is dropped.
  • client/main.tsx:96replaceStandaloneSessionUrl deletes the workspace param whenever sessionId is undefined, which is precisely the new-chat state. The reported workspace therefore cannot affect the URL there.
  • client/main.tsx:124-127sessionId / workspaceId are read once at mount (const [sessionId] = useState(...)), so the standalone app never re-routes or re-mounts at runtime.

My harness prints what the standalone host would compute; it is / on every arm and every scenario. The observable is real for embedders using WebShell / WebShellWithProviders from @qwen-code/web-shell that route on the callback — naming that host in the description would make the repro reproducible.

(The premise itself checks out: getConnectionAfterSessionClear in packages/webui/src/daemon/session/actions.ts:84 deletes sessionId, clientId, displayName, tokenUsage, tokenCount, supportedCommands, context — and deliberately keeps workspaceCwd.)


🧪 Test coverage — 4 of 9 mutants survive the 234-test suite

Mutation of the new code Suite
always selectedWorkspaceCwd ?? connection.workspaceCwd (drop the active-session branch) survives
connection.workspaceCwd (full revert) killed
drop the ?? connection.workspaceCwd fallback survives
invert the ternary condition killed
swap the ?? operands killed
look up by the new value but report connection.workspaceCwd killed
keep the dedupe guard keyed on connection.workspaceCwd survives
drop selectedWorkspaceCwd from the dependency array survives
look up activeWorkspace by the old value killed

Two of the four die with two small tests worth adding:

  1. active session + a conflicting next-session pick → must still report the session's own workspace (this is the “behaviour with an active session is unchanged” claim from the description; nothing currently pins it);
  2. deferred workspace switch A → B with no session → the host must be notified for each switch (this is the new notification path; today only a sessionId change makes the effect re-run, so dropping the dependency goes unnoticed).

中文完整版

本地验证报告 —— 真实构建、真实浏览器

我在本地做了真实验证:在无头 Chromium 中渲染真实的 <App/>(daemon 层打桩),点击真实的侧栏按钮,并在 merge base(b3873571a)与本 PR head(f18c257e3)之间对 client/App.tsx 做 A/B。两侧都跑了完整单测。

结论:机制正确,新增的回归测试也不是空测试 —— 但这个修复只覆盖了一半的入口,而漏掉的那一半正好包含 PR 自己写的手动复现步骤。 一行即可补齐,补丁在下面,我已验证过。


✅ 已验证有效的部分

检查项 结果
本 head 上的 client/App.test.tsx 234 / 234 通过
本 head 上整个 @qwen-code/web-shell 套件 2626 通过;仅有的 10 个失败是 build-artifact.test.tsENOENT … /dist/index.js,需要先 npm run build;在 base 上同样失败)
新回归测试非空测 在 base 上失败,实收 (undefined, 'secondary', '/work/secondary') —— 确实钉住了残留 workspace
有活跃会话时行为不变 故意冲突的「下一会话」选择不会泄漏进活跃会话的上报
死会话(missingSession 依然静默,与之前一致
无会话时切换 workspace base 只发出初始那一条上报;本 head 每次切换都上报 —— 即 Risk & Scope 中提到的行为变化,符合描述

修复生效的真实浏览器证据,起始状态即 PR 描述的场景(session-1 活跃在非 primary 的 wsA),点击另一个非 primary workspace wsB 下的「New task」

fixed

左栏是一个会根据 onSessionIdChange 路由并重挂 shell 的 host,行为对齐 WorkspaceSessionProvider。修复前:host 被路由回 wsA,composer 标签卡在 a。修复后:host 路由到 wsB,标签显示 b。完全符合预期。


⚠️ 发现 1(主要)—— 这里的 undefined 表示 primary,不是「未知」

在本代码库中,selectedWorkspaceCwd === undefined 不是「没有选择」,而正是「primary workspace」的写法:

  • client/components/WorkspaceSelector.tsx:130onSelectWorkspace(next.primary ? undefined : next.cwd)
  • client/components/sidebar/WebShellSidebar.tsx:4231const wsCwd = ws.primary ? undefined : ws.cwd;(在 :4274 传给 handleNewSession
  • 侧栏顶部的 New task:3999)直接 handleNewSession() 不带参数;/clear/new/resetshellApi.createNewSession()、缺失会话恢复、新会话建议、定时任务「create via chat」、Goals 也都一样 —— 全部走到 createNewSession()setSelectedWorkspaceCwd(undefined)

因此在这些路径上,selectedWorkspaceCwd ?? connection.workspaceCwdApp.tsx:4627)会直接回落到本 PR 想要避开的那个残留值。

这其中就包含 PR 自己的手动测试计划。 「点击 workspace A 下的 new task」在 A 是 primary 项目(最常见的情况)时,走的正是 wsCwd = undefined。我在真实浏览器里点了这个按钮,BASE 与 PR 的截图逐字节相同(两者 md5 acc1afc2…):

gap-primary

侧栏顶部的 New task 同理(BASE 与 PR 截图同样逐字节相同,md5 4564707e…):

gap-toplevel

这个分歧不只是显示问题。我在 secondary workspace 的会话里执行 /clear 后,让延迟会话真正被创建出来,并同时记录了两侧:

daemon 被要求创建会话的 workspace = "/workspace"(primary)
composer 标签                     = "/workspace"
上报给 host 的                    = (undefined, "secondary", "/work/secondary")

ensureSessionForPrompt 解析新会话 workspace 的表达式是 lockedWorkspaceCwd ?? acceptedWorkspaceCwd ?? primaryWorkspaceCwdApp.tsx:3312),connection.workspaceCwd 完全不参与。也就是说,host 被路由到了一个新会话根本不会落在其中的 workspace。

建议的修复(一行,已验证)

activeWorkspaceCwdApp.tsx:1570)的语义恰好就是**「会话所在 workspace,否则是为下一个会话选定的 workspace(locked / selected / primary)」**,而且 composer 标签与 getComposerWorkspaceCwd()App.tsx:3359)用的就是它。复用它可以从构造上保证 host 上报与标签一致:

-    const reportedWorkspaceCwd = connection.sessionId
-      ? connection.workspaceCwd
-      : (selectedWorkspaceCwd ?? connection.workspaceCwd);
+    const reportedWorkspaceCwd = activeWorkspaceCwd ?? connection.workspaceCwd;
     onSessionIdChange,
-    selectedWorkspaceCwd,
+    activeWorkspaceCwd,
     workspace.capabilities,

我实测过:上面三个浏览器场景全部变正确(后两张截图的右侧面板);「创建所在 workspace」与「上报 workspace」一致;你新增的回归测试仍然通过App.test.tsx 保持 234 / 234;整个包保持 2626 通过。同时也消除了同一意图的第三份近似重复实现。


⚠️ 发现 2(低)—— 未受信任的 workspace 会被上报一拍

侧栏的 onSelectWorkspace={setSelectedWorkspaceCwd} 写入选择时不做信任校验,随后由 trust-clearing effect(App.tsx:1509-1518)在下一拍重置。无活跃会话时,base 发出一条上报(残留 workspace);本 head 发出两条,第一条是 (undefined, 'untrusted', '/work/untrusted')

不会创建出不安全的东西 —— ensureSessionForPrompt 仍然按 trusted === true 过滤 —— 但会路由的 host 会短暂挂载到未受信任 workspace 的路由上。上面建议的一行改动也改善了随后的回落(回落到 primary,而不是残留 workspace)。


📝 发现 3(文档)—— 出厂的 standalone web shell 无法复现该现象

建议在描述里补一句,否则 reviewer 按 「Manual (multi-workspace deployment)」qwen serve 上什么都看不到:

  • client/main.tsx:150 —— handleSessionIdChange 只接收 (sessionId, workspaceId),第三个参数被丢弃。
  • client/main.tsx:96 —— 只要 sessionId 为 undefined(新建会话正是这个状态),replaceStandaloneSessionUrl 就会删除 workspace 参数。所以上报的 workspace 在这里根本影响不到 URL。
  • client/main.tsx:124-127 —— sessionId / workspaceId 在挂载时只读一次const [sessionId] = useState(...)),standalone 应用在运行期从不重新路由或重挂。

我的 harness 会打印 standalone host 会算出的 URL:在所有 arm、所有场景下都是 /。真正能观察到该现象的是使用 @qwen-code/web-shellWebShell / WebShellWithProviders 且基于该回调做路由的宿主 —— 在描述中点明这个宿主,复现步骤才可执行。

(前提本身是成立的:packages/webui/src/daemon/session/actions.ts:84getConnectionAfterSessionClear 删除了 sessionIdclientIddisplayNametokenUsagetokenCountsupportedCommandscontext,并且有意保留 workspaceCwd。)


🧪 测试强度 —— 9 个变异中有 4 个能在 234 个测试下存活

对新代码的变异 套件结果
恒取 selectedWorkspaceCwd ?? connection.workspaceCwd(去掉活跃会话分支) 存活
恒取 connection.workspaceCwd(完全回退) 被杀
去掉 ?? connection.workspaceCwd 兜底 存活
反转三元条件 被杀
交换 ?? 两侧 被杀
用新值查找但仍上报 connection.workspaceCwd 被杀
去重守卫仍以 connection.workspaceCwd 为键 存活
从依赖数组中删掉 selectedWorkspaceCwd 存活
activeWorkspace 仍按旧值查找 被杀

其中两个可以用两个小测试杀掉,建议补上:

  1. 有活跃会话 + 冲突的下一会话选择 → 仍必须上报会话自己的 workspace(这正是描述里「有活跃会话时行为不变」的断言,目前没有任何测试钉住它);
  2. 无会话时 workspace 从 A 切到 B → 每次切换 host 都必须收到通知(这是新增的通知路径;目前只有 sessionId 变化才会让 effect 重跑,所以删掉该依赖不会被发现)。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [review-pr] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下:

Review round summary — PR #7910

Addressed the inline suggestion and the matching main finding from the local
verification report by reusing the existing activeWorkspaceCwd resolver, and
added the recommended regression coverage. One commit:
fix(web-shell): reuse activeWorkspaceCwd for the no-session host report (#7910).

Feedback points and decisions

[rc:3664011121] [Suggestion] — no-session fallback reverts to the stale
connection.workspaceCwd → Implemented (resolved in code).

The report effect computed selectedWorkspaceCwd ?? connection.workspaceCwd
for the no-session case. Because selectedWorkspaceCwd === undefined is how
"primary / no explicit selection" is spelled across the sidebar "New task",
/clear, /new, /reset, createNewSession(), and the recovery paths, the
fallback dropped straight back to the stale connection.workspaceCwd — the
exact misrouting this PR sets out to remove, surviving on the most common
new-chat path. The two sibling resolvers (activeWorkspaceCwd and
getComposerWorkspaceCwd) already resolve this case as
locked ?? selected ?? primary.

Change:

  • reportedWorkspaceCwd is now activeWorkspaceCwd ?? connection.workspaceCwd.
    When a session is active, activeWorkspaceCwd === connection.workspaceCwd,
    so the active-session behaviour is unchanged; with no session it resolves
    locked ?? selected ?? primary, falling back to connection.workspaceCwd
    only when no workspace is known. This makes the host report agree with the
    composer chip by construction and removes a third near-duplicate copy of the
    same resolution logic (net simpler).
  • The effect dependency changed from selectedWorkspaceCwd to
    activeWorkspaceCwd (which itself tracks the selection, the lock, and the
    workspace list), so selection changes still re-run the effect.

Finding 1 (main, issue-level report) — same root cause → Implemented by the
same change above.

Test coverage suggestions (issue-level report) → Implemented. Added three
focused tests in App.test.tsx, each verified non-vacuous:

  1. selection unset → reports primary, not the stale workspace. Fails on the
    pre-fix code (which reported the stale secondary), so it pins exactly the
    gap fixed this round.
  2. active session + conflicting next-session selection → still reports the
    session's own workspace.
    Pins the "active-session behaviour unchanged"
    claim (reviewer's suggested test pre-release: fix ci #1).
  3. deferred workspace switch A → B with no session → host notified for each
    switch.
    Fails if activeWorkspaceCwd is dropped from the dependency
    array, so it pins the new notification path (reviewer's suggested test Where is the config saved? #2).

Finding 2 (Low, issue-level report) — untrusted workspace reported for one
tick → Partially addressed; extra hardening declined.
The same change already
improves the follow-up report: after the trust-clearing effect resets the
selection, the report now falls back to the primary workspace instead of the
stale one (as the report noted). The transient first-tick report of a
just-selected untrusted workspace remains. It is pre-existing (the base also
emitted a report on selection), is not unsafe (ensureSessionForPrompt still
filters trusted === true, so no session is ever created in it), and adding a
trust filter to the host-report path would grow the diff beyond the inline
finding without fixing a real defect. Declined as not worth the diff growth.

Finding 3 (docs, issue-level report) — standalone shell cannot show this →
Noted, no code change.
This is a PR-description clarification (the observable
applies to embedders routing on onSessionIdChange, not the standalone
qwen serve shell, whose main.tsx drops the third argument and deletes the
workspace URL param when sessionId is undefined). The PR body is owned by
the workflow rather than this review round, so it is recorded here for the
maintainer instead of being edited in code.

Conflict notes

--conflict false; no merge performed.

Verification

  • npm run build — passed
  • npm run typecheck — passed
  • npm run lint (full: eslint . --ext .ts,.tsx && eslint integration-tests) — passed
  • npx eslint packages/web-shell/client/App.tsx packages/web-shell/client/App.test.tsx — passed
  • npx prettier --check on both touched files — passed
  • vitest run client/App.test.tsx (touched package) — 237 passed (234 prior + 3 new)
  • Non-vacuity checks: test 1 fails on the pre-fix code; test 3 fails when
    activeWorkspaceCwd is removed from the dependency array (both restored
    afterwards)
  • Integration tests after npm run bundle — not run: the change is confined to
    the @qwen-code/web-shell React client and is exercised through the unit
    suite rendering the real <App/>; it is not behavior only reachable through
    the bundled CLI or the integration harness
中文说明

审查轮次总结 — PR #7910

通过复用已有的 activeWorkspaceCwd 解析器,处理了行内建议以及本地验证报告中对应的主要发现,并补充了建议的回归覆盖。单次提交:fix(web-shell): reuse activeWorkspaceCwd for the no-session host report (#7910)

反馈点与处理

[rc:3664011121] [建议] — 无会话时的回退退回到过期的 connection.workspaceCwd → 已实现(在代码中解决)。

上报 effect 在无会话时计算 selectedWorkspaceCwd ?? connection.workspaceCwd。由于 selectedWorkspaceCwd === undefined 正是整个侧栏「New task」、/clear/new/resetcreateNewSession() 以及恢复路径中「primary / 未显式选择」的写法,这个回退会直接落回过期的 connection.workspaceCwd——也就是本 PR 要消除的错误路由本身,并且在最常见的新建对话路径上依然存在。两个同级解析器(activeWorkspaceCwdgetComposerWorkspaceCwd)已经把这种情况解析为 locked ?? selected ?? primary

改动:

  • reportedWorkspaceCwd 现在为 activeWorkspaceCwd ?? connection.workspaceCwd。当有活跃会话时,activeWorkspaceCwd === connection.workspaceCwd,因此有活跃会话时的行为不变;无会话时解析为 locked ?? selected ?? primary,仅在没有任何已知 workspace 时才回退到 connection.workspaceCwd。这从构造上保证了 host 上报与 composer 标签一致,并消除了同一解析逻辑的第三份近似重复实现(整体更简洁)。
  • effect 的依赖从 selectedWorkspaceCwd 改为 activeWorkspaceCwd(它本身会跟踪选择、锁定以及 workspace 列表),因此切换选择仍会重新运行该 effect。

发现 1(主要,issue 级报告)— 同一根因 → 已实现,即上面这处改动。

测试覆盖建议(issue 级报告)→ 已实现。App.test.tsx 中新增三个聚焦测试,并逐一验证非空测:

  1. 选择未设置 → 上报 primary,而非过期 workspace。 在修复前的代码上会失败(当时上报的是过期的 secondary),因此恰好钉住了本轮修复的缺口。
  2. 有活跃会话 + 冲突的下一会话选择 → 仍上报会话自己的 workspace。 钉住「有活跃会话时行为不变」这一断言(评审建议的测试 pre-release: fix ci #1)。
  3. 无会话时 workspace 从 A 切到 B → 每次切换都通知 host。 当从依赖数组中删掉 activeWorkspaceCwd 时会失败,因此钉住了新增的通知路径(评审建议的测试 Where is the config saved? #2)。

发现 2(低,issue 级报告)— 未受信任的 workspace 会被上报一拍 → 部分处理;拒绝额外加固。 同一处改动已经改善了随后的上报:在 trust-clearing effect 重置选择之后,上报现在回退到 primary workspace,而不是过期的那个(正如报告所指出的)。刚选中的未受信任 workspace 在第一拍仍会被短暂上报。这属于既有行为(base 在选择时同样会发出一条上报),并不存在安全问题(ensureSessionForPrompt 仍按 trusted === true 过滤,因此绝不会在其中创建会话);在 host 上报路径上再加一层信任校验会让 diff 超出行内建议的范围,且并未修复真实缺陷。以「不值得增加 diff 体量」为由拒绝。

发现 3(文档,issue 级报告)— 出厂 standalone shell 无法复现 → 已记录,不改代码。 这是一条 PR 描述层面的澄清(该可观察现象适用于基于 onSessionIdChange 做路由的宿主,而非 standalone 的 qwen serve shell——后者的 main.tsx 会丢弃第三个参数,并在 sessionId 为 undefined 时删除 URL 中的 workspace 参数)。PR 描述由工作流维护,而非本审查轮次,因此这里记录给维护者,而不在代码中改动。

冲突说明

--conflict false;未执行合并。

验证

  • npm run build — 通过
  • npm run typecheck — 通过
  • npm run lint(完整:eslint . --ext .ts,.tsx && eslint integration-tests)— 通过
  • npx eslint packages/web-shell/client/App.tsx packages/web-shell/client/App.test.tsx — 通过
  • 对两个改动文件执行 npx prettier --check — 通过
  • vitest run client/App.test.tsx(受影响包)— 237 通过(原 234 + 新增 3)
  • 非空测检查:测试 1 在修复前代码上失败;测试 3 在从依赖数组移除 activeWorkspaceCwd 时失败(两者随后均已还原)
  • npm run bundle 之后的集成测试 — 未运行:改动仅限于 @qwen-code/web-shell 的 React 客户端,并通过渲染真实 <App/> 的单测套件覆盖;该行为并非只能通过打包后的 CLI 或集成测试框架触达

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@qwen-code-ci-bot qwen-code-ci-bot left a comment

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.

⚠️ Downgraded from Approve to Comment: CI still running. Reviewed.

中文说明

⚠️ 已从批准降级为评论:CI still running。 已审查。

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

No changes were made this round.

The only review feedback newer than the last evaluation is an automated COMMENTED notice from @qwen-code-ci-bot stating that a prior Approve was downgraded to Comment because CI was still running ("Reviewed."). It names no defect, requests no change, and carries no CHANGES_REQUESTED. There were no inline comments, no issue-level comments, no failed checks, and no still-red checks to address.

Since there is nothing actionable, the PR head is left unchanged.

中文说明

本轮未做任何改动。

自上次评估以来唯一新增的审查反馈是 @qwen-code-ci-bot 发出的一条自动 COMMENTED(评论)通知,说明此前的批准(Approve)因 CI 仍在运行而被降级为评论(“Reviewed.”)。该通知未指出任何缺陷,未请求任何改动,也不包含 CHANGES_REQUESTED(请求修改)。同时没有内联评论、没有 issue 级评论、没有失败的检查,也没有持续标红的检查需要处理。

由于没有任何可操作的内容,PR 的 head 分支保持不变。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@yiliang114 yiliang114 left a comment

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.

LGTM

@wenshao

wenshao commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

Local verification report — real browser, real client, before/after

I built a local harness and verified this PR end-to-end at 26b4fd7 (base 9461aa8), driving the real client in Chromium (vite dev build) against the repo's own mock daemon, inside an embedding host that does what the PR description says a host does: it keeps its route in sync with onSessionIdChange and re-mounts the shell for the reported workspace. Every A/B below reverts only client/App.tsx to base and re-runs the identical harness.

1. Unit A/B

Run Result
client/App.test.tsx @ PR head 237 passed
client/App.test.tsx @ base App.tsx (PR tests kept) 3 failed / 234 passed — the three new behavioural tests fail with (undefined, 'secondary', '/work/secondary')
Whole packages/web-shell suite @ head 2387 passed; the only failures are the 10 build-artifact.test.ts cases that need a built dist/index.js (I did not build the package)
npm run lint clean
npm run typecheck 3 errors, identical on base (pickWorkspaceDirectory, toolUseId) — stale sibling package types in my shared node_modules, unrelated to this PR

Worth noting: the fourth new test (keeps reporting the active session workspace…) passes on base too — it is a guard, not a regression detector. That is correct, just not evidence.

2. Real-browser A/B — the fix works, and it is not cosmetic

Two workspaces (primary-app primary, secondary-app secondary), a session with a running task in one of them, then "New task" under the other in the sidebar.

Scenario (routing host) base PR #7910
Session in secondary → New task under primary host + composer show secondary-app primary-app
Session in primary → New task under secondary host + composer show primary-app secondary-app
Active session still routes by its own workspace ✅ (unchanged)
POST /session cwd of the session the next prompt creates /work/secondary-app /work/primary-app

That last row is the important one: this is not just a wrong label — on base the next chat is really created in the wrong workspace, so the agent would run against the wrong repo. Stability: 9/9 over --repeat-each=3.

routing host before/after

mirror direction

I also measured the trade-off called out in "Risk & Scope" (a deferred workspace switch now notifies the host): an unsent composer draft is dropped by the workspace switch itself on both base and head, so the extra notification costs nothing there.

3. Findings the reviewer should weigh

F1 — the motivating symptom still reproduces in the standalone shell (pre-existing, not caused and not fixed by this PR).
client/main.tsx — what qwen serve ships — only rewrites the URL and never re-mounts, so the fix cannot reach it. In that host, after "New task" under primary-app from a session in secondary-app, the composer still reads secondary-app and POST /session still carries cwd=/work/secondary-app, identically on base and head.

The cause is a second copy of the same stale value: WebShellSidebar.tsx:1226 re-selects the connection's leftover workspace whenever there is no session and no explicit selection —

if (currentSessionId || selectedWorkspaceCwd !== undefined) return;

if (connectedWorkspace && !connectedWorkspace.primary) {
  onSelectWorkspace?.(connectedWorkspace.cwd);   // stale connection.workspaceCwd

Because "new chat in the primary workspace" is spelled selectedWorkspaceCwd === undefined — exactly as this PR's second test documents — the sidebar cannot tell the user's explicit "primary" from "unset", and overwrites it. Isolation, same host, only sidebar differs (both on PR head):

sidebar isolation

standalone shell after the fix

F2 — new with this PR: the report can stop converging for a host that routes asynchronously.
Because the notification now tracks activeWorkspaceCwd, the sidebar write-back above produces a second, contradictory report. With a host that applies the reported route synchronously this is invisible (the re-mount wins the same React batch). With a host that routes a tick later (router navigation, state round-trip — I simulated 300 ms) the two sides ping-pong indefinitely:

host applies route base PR #7910
synchronously 3 reports, settles (wrong workspace) 3 reports, settles (right workspace)
after 300 ms, sidebar on 3 reports, settles 19 reports @3s → 36 @8s, alternating /work/primary-app/work/secondary-app, never settles
after 300 ms, sidebar off 3 reports, settles 3 reports, settles (right workspace)

F3 — small contract wart (pre-existing). On every (re)mount the shell first reports (sessionId, undefined, undefined) before the connection resolves the session's workspace. A host that routes on that drops its workspace and re-mounts forever; my harness has to ignore it. Same on base and head — worth a line in the onSessionIdChange docs.

4. Recommendation

The change itself is correct, minimal, well-tested and does exactly what it claims for re-mounting embedder hosts, including the part that matters most (the workspace the next session is created in). I have no objection to merging it.

I would follow up on WebShellSidebar.tsx:1226 in a separate PR — distinguishing "no selection" from "primary" there fixes the symptom for the shipped standalone shell as well and removes the source of the F2 oscillation. Until then it is worth adjusting the PR description, which claims the composer indicator is fixed: that holds for hosts that re-mount on the notification, not for qwen serve itself.

Reproduction

Worktree at 26b4fd7, node_modules symlinked from a sibling checkout, no package build required.

# unit A/B
cd packages/web-shell && npx vitest run --config vitest.config.ts client/App.test.tsx
git checkout 9461aa8 -- packages/web-shell/client/App.tsx   # then re-run for the red side

# browser A/B (vite dev server is started by the playwright config)
npx playwright test --config playwright.config.ts pr7910

The harness is an extra page under client/e2e/ that mounts WebShellWithProviders and feeds onSessionIdChange back in as props, plus specs driving the sidebar "New task" / composer workspace picker. It lives outside the PR — happy to push it to a branch if you want it as a permanent regression test.

中文说明

本地验证报告 —— 真实浏览器、真实前端、before/after 对比

我在本地搭建了验证环境,对 26b4fd7(base 9461aa8)做了端到端验证:在 Chromium 中运行真实前端(vite dev 构建),后端用仓库自带的 mock daemon,外面套一个嵌入宿主(embedding host)——它就是 PR 描述里那种宿主:根据 onSessionIdChange 同步自己的路由,并按上报的 workspace 重新挂载 shell。下面所有 A/B 都client/App.tsx 回退到 base,其余(含 harness)完全不变。

1. 单测 A/B

运行 结果
client/App.test.tsx @ PR head 237 通过
client/App.test.tsx @ base 的 App.tsx(保留 PR 新增用例) 3 失败 / 234 通过 —— 三个新增行为用例都以 (undefined, 'secondary', '/work/secondary') 失败
packages/web-shell 全量套件 @ head 2387 通过;仅有的 10 个失败全在 build-artifact.test.ts,它需要已构建的 dist/index.js(我没有构建该包)
npm run lint 干净
npm run typecheck 3 个错误,base 上完全相同pickWorkspaceDirectorytoolUseId)——是我共享 node_modules 里兄弟包类型过期导致,与本 PR 无关

补充一点:第四个新增用例(keeps reporting the active session workspace…)在 base 上也通过——它是护栏,不是回归探测器。这没问题,只是它不构成证据。

2. 真实浏览器 A/B —— 修复有效,而且不只是「显示问题」

两个 workspace(primary-app 为 primary,secondary-app 为 secondary),其中一个持有正在跑任务的会话,然后在侧栏另一个 workspace 下点「New task」。

场景(会路由的宿主) base PR #7910
会话在 secondary → 在 primary 下新建 宿主和 composer 都显示 secondary-app primary-app
会话在 primary → 在 secondary 下新建 宿主和 composer 都显示 primary-app secondary-app
有活跃会话时仍按该会话自身 workspace 路由 ✅(行为不变)
下一条 prompt 真正创建会话时 POST /session 的 cwd /work/secondary-app /work/primary-app

最后一行最关键:这不只是标签显示错了——在 base 上,新会话确实被创建在错误的 workspace 里,agent 会跑在错误的仓库上。稳定性:--repeat-each=3 下 9/9 通过。

(截图见英文部分)

我也实测了「风险与范围」里提到的权衡(无会话时切换 workspace 现在也会通知宿主):composer 里未发送的草稿在 base 和 head 上都会被这次 workspace 切换本身清掉,所以这条新增通知在这一点上没有额外代价。

3. 需要评审一并权衡的发现

F1 —— 触发本 PR 的那个现象,在 standalone shell 里依然复现(既非本 PR 引入,也未被本 PR 修复)。
client/main.tsx(也就是 qwen serve 实际发布的那个宿主)只重写 URL、从不重新挂载,所以这个修复够不到它。在该宿主下,从 secondary-app 的会话里点 primary-app 的「New task」后,composer 仍然显示 secondary-app,并且 POST /session 仍然带 cwd=/work/secondary-app——base 和 head 完全一致。

根因是同一个残留值的第二处副本:WebShellSidebar.tsx:1226 在「没有会话且没有显式选择」时,会把 connection 残留的 workspace 重新选回去:

if (currentSessionId || selectedWorkspaceCwd !== undefined) return;

if (connectedWorkspace && !connectedWorkspace.primary) {
  onSelectWorkspace?.(connectedWorkspace.cwd);   // 残留的 connection.workspaceCwd

而「在 primary 里新建会话」的表示方式恰恰就是 selectedWorkspaceCwd === undefined——正如本 PR 第二个用例的注释所写——于是侧栏无法区分用户显式选择的「primary」和「尚未选择」,直接覆盖掉了用户的选择。同一宿主、仅 sidebar 不同的隔离实验(两侧都在 PR head 上)见英文部分截图。

F2 —— 本 PR 新引入:对于异步路由的宿主,上报可能不再收敛。
由于上报值现在跟随 activeWorkspaceCwd,上面那次侧栏回写会产生第二条互相矛盾的上报。如果宿主在同一个 React 批次里同步应用路由,这一点看不出来(重新挂载先发生);但如果宿主晚一拍才路由(router 跳转、状态往返,我模拟了 300ms),两边会无限来回:

宿主应用路由的方式 base PR #7910
同步 3 条上报后稳定(workspace 是错的) 3 条上报后稳定(workspace 正确)
延迟 300ms,开启侧栏 3 条上报后稳定 3s 时 19 条 → 8s 时 36 条,在 /work/primary-app/work/secondary-app 间反复横跳,不收敛
延迟 300ms,关闭侧栏 3 条上报后稳定 3 条上报后稳定(workspace 正确)

F3 —— 一个小的契约瑕疵(既有问题)。 每次(重新)挂载时,shell 会先上报一次 (sessionId, undefined, undefined),此时 connection 还没解析出该会话的 workspace。按这条上报去路由的宿主会丢掉 workspace 并无限重挂——我的 harness 必须忽略它。base 和 head 表现一致,建议在 onSessionIdChange 的文档里补一句。

4. 结论与建议

改动本身是正确的、最小的、测试也到位,对「会重新挂载的嵌入宿主」确实做到了它承诺的事,包括最要紧的那一点——下一个会话实际创建在哪个 workspace。我不反对合并。

建议后续单开一个 PR 处理 WebShellSidebar.tsx:1226:在那里把「未选择」和「选择了 primary」区分开,既能让 standalone shell 也真正修好这个现象,也能消掉 F2 振荡的来源。在那之前,PR 描述里「composer 的 workspace 指示会显示正确」的说法建议调整一下:它对「会按通知重新挂载的宿主」成立,对 qwen serve 自身并不成立。


Verified locally with Claude Code (Opus 5, 1M context) — real Chromium E2E against the repo's mock daemon; harness kept outside the PR.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

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.

LGTM, looks ready to ship. ✅

@wenshao
wenshao added this pull request to the merge queue Jul 28, 2026
Merged via the queue into QwenLM:main with commit a68c4cb Jul 28, 2026
56 of 57 checks passed
@yiliang114

yiliang114 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Done. Replied on PR #7910 acknowledging wenshao's verification report. No code changes needed — the fix is confirmed working for its intended scope (re-mounting embedder hosts). Agreed to:

  1. Adjust PR description to clarify standalone shell (qwen serve) is not covered
  2. Follow-up PR for WebShellSidebar.tsx:1226 to fix F1 (standalone symptom) and F2 (async oscillation) at the root
  3. Document the initial (sessionId, undefined, undefined) report behavior (F3)

✅ completed

@yiliang114

Copy link
Copy Markdown
Collaborator

Thanks for the incredibly thorough verification — the real-browser A/B with the routing host harness is exactly the kind of evidence that makes this review solid.

Agree on all three findings:

  • F1: Correct that the standalone host shipped by qwen serve (rewrite-only, no re-mount) is out of scope here. I will adjust the PR description to clarify the fix applies to hosts that re-mount on onSessionIdChange, not the shipped standalone shell.
  • F2: The async-host oscillation is a real regression vector introduced by tracking activeWorkspaceCwd. Since the root cause is the sidebar write-back at WebShellSidebar.tsx:1226 (pre-existing), a follow-up PR distinguishing "no selection" from "explicit primary" there is the right fix — it resolves both the F1 symptom and the F2 oscillation source in one place.
  • F3: Good catch on the initial (sessionId, undefined, undefined) report. I will add a note to the onSessionIdChange docs that hosts should ignore reports where workspace fields are undefined until the connection resolves.

No code changes needed in this PR — the fix does what it claims for re-mounting embedder hosts, and the follow-up belongs in the sidebar. Appreciate the harness offer; if you push it to a branch I would be happy to see it land as a regression test separately.

@doudouOUC doudouOUC left a comment

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.

No issues found. LGTM! ✅

中文说明

未发现问题。LGTM!✅

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

Local real-stack verification

I verified this locally against a real stack rather than only the mocked unit tests, so the routing behaviour is exercised end to end.

What "real" means here

  • A real qwen serve daemon, run from source (node scripts/dev.js serve), with two real registered workspacesws-primary (primary) and ws-secondary (registered via POST /workspaces) — and an isolated QWEN_HOME.
  • The web-shell from this PR's source, served by its own vite dev server (@qwen-code/webui and @qwen-code/sdk alias to source in serve mode), talking to that daemon through the dev proxy.
  • Real Chromium driven by Playwright. Only the model provider is faked (a local OpenAI-compatible SSE server), so real turns run without credentials.
  • A/B by reverse-applying only the client/App.tsx hunk (git apply -R) — same script, same daemon, same workspaces.

1. Unit RED/GREEN — confirmed, with one caveat about the baseline

Against the merge base, 3 of the 4 new tests fail and all 237 pass with the PR:

main baseline:  Tests  3 failed | 234 passed (237)
  ✗ reports the selected workspace, not the stale connection workspace, when no session is active
  ✗ reports the primary workspace, not the stale connection workspace, when the selection is unset
  ✗ notifies the host on each deferred workspace switch while no session is active
PR #7910:       Tests  237 passed (237)

Two notes for anyone re-running this:

  • git checkout <PR-head>^ -- client/App.tsx is not main here. The branch's first commit (f18c257e3) already carried a partial fix (selectedWorkspaceCwd ?? connection.workspaceCwd), so reverting only the final commit leaves 2 of the 4 tests green and understates the change. The reverse-applied full PR diff is the correct baseline — that is what the numbers above use.
  • The 4th new test (keeps reporting the active session workspace despite a conflicting next-session selection) passes on main as well. It is a no-regression guard, not RED/GREEN evidence — worth keeping, just not evidence of the fix.

2. Real browser A/B — the fix works ✅

Ran a real turn in ws-secondary, then clicked the sidebar “New task” under ws-primary, with a host that re-mounts the shell for whatever onSessionIdChange reports (the embedding contract in packages/web-shell/README.md).

real-stack A/B

host was told composer chip
main (undefined, "6d827c62…", <ws-secondary>) ws-secondary
PR #7910 (undefined, undefined, <ws-primary>) ws-primary

The reported bug reproduces on main in the real stack and is gone with this PR. That is the claim in the PR body, confirmed outside the unit mocks.

3. Residual gap — the shipped standalone Web Shell is not fixed by this 🟠

Same daemon and workspaces, but driving the shipped entry (client/main.tsx, i.e. what qwen serve opens) instead of a routing host:

standalone gap

client/App.tsx WebShellSidebar.tsx auto-select composer chip next prompt ran in
main as-is ws-secondary ws-secondary
PR #7910 as-is ws-secondary ws-secondary
main disabled (probe) ws-primary ws-primary
PR #7910 disabled (probe) ws-primary ws-primary

The "next prompt ran in" column is daemon ground truth (GET /workspace/<cwd>/sessions), not a UI reading — with the PR applied, the new chat started under ws-primary still creates its session in ws-secondary.

The determinant is not App.tsx at all. It is this effect in WebShellSidebar.tsx:

// no session + selection unset  ->  re-select whatever the connection last pointed at
const connectedWorkspace = workspaces.find((e) => e.cwd === connection.workspaceCwd);
if (connectedWorkspace && !connectedWorkspace.primary) {
  onSelectWorkspace?.(connectedWorkspace.cwd);

undefined is how "primary" is spelled for selectedWorkspaceCwd, so a new chat in the primary workspace lands in exactly the state this effect treats as "adopt the stale workspace". In the observer runs you can watch it happen: the shell reports <ws-primary> first (the fix firing) and then immediately reports <ws-secondary> again. With a routing host the intermediate report re-mounts the shell and resets connection.workspaceCwd, which is why the fix sticks there and not here.

This is not a regression — main behaves identically. But the PR body's manual test plan ("Expected: the composer's workspace indicator shows A") does not hold for qwen serve's own web UI. I'd either narrow that claim to routing hosts, or fold the sidebar effect into this PR / a follow-up.

One more thing seen on both arms, so also pre-existing: during a re-mount the shell briefly reports (sessionId, undefined, undefined). A host that routes on that tuple gets bounced to the primary workspace while the session actually lives elsewhere. Worth a sentence in the README contract that a missing workspaceCwd means "unknown", not "primary".

4. Regression checks

  • packages/web-shell full suite: 2387 / 2397. The 10 failures are all build-artifact.test.ts, which needs a prior vite build --config vite.lib.config.ts; after that build it is 10/10. Unrelated to this PR.
  • eslint --max-warnings 0 on both changed files: clean.
  • tsc -p tsconfig.json --noEmit: 3 errors, all from my local node_modules overlay resolving @qwen-code/webui / @qwen-code/sdk to an older built dist (pickWorkspaceDirectory, toolUseId), in files this PR does not touch. Not a PR issue.

Verdict

The change is correct and does fix the reported bug for hosts that route on onSessionIdChange. From my side this is good to merge on that basis. The two things I'd like resolved before or right after merge are cosmetic-to-the-diff but not to users: the manual-test claim in the description, and the sidebar auto-select that still sends a fresh primary-workspace chat into the stale workspace in the shipped UI.


🤖 Generated with Claude Code — Claude Opus 5 (1M context)

中文说明

本地真实环境验证

我没有只跑带 mock 的单测,而是在本地搭了一套真实链路做端到端验证。

这里的「真实」指

  • 真实的 qwen serve daemon,从源码直接运行(node scripts/dev.js serve),带两个真实注册的 workspace——ws-primary(primary)和 ws-secondary(通过 POST /workspaces 注册),并使用隔离的 QWEN_HOME
  • 本 PR 源码的 web-shell,由它自己的 vite dev server 提供(serve 模式下 @qwen-code/webui@qwen-code/sdk 都别名到源码),经 dev proxy 连到上面那个 daemon。
  • 真实 Chromium,由 Playwright 驱动。只有模型服务是假的(本地 OpenAI 兼容 SSE server),所以无需任何凭证也能跑真实对话轮次。
  • A/B 方式:只反向应用 client/App.tsx 那个 hunk(git apply -R)——脚本、daemon、workspace 完全一致。

1. 单测 RED/GREEN —— 确认成立,但基线有个坑

以 merge base 为基线,4 个新测试中有 3 个失败,打上本 PR 后 237 个全过:

main 基线:  Tests  3 failed | 234 passed (237)
  ✗ reports the selected workspace, not the stale connection workspace, when no session is active
  ✗ reports the primary workspace, not the stale connection workspace, when the selection is unset
  ✗ notifies the host on each deferred workspace switch while no session is active
PR #7910:   Tests  237 passed (237)

给后面复现的人两点提醒:

  • git checkout <PR-head>^ -- client/App.tsx 在这里不等于 main。分支的第一个提交(f18c257e3)已经带了部分修复(selectedWorkspaceCwd ?? connection.workspaceCwd),所以只回退最后一个提交会让 4 个测试里有 2 个变绿,从而低估本次改动。正确基线是把完整 PR diff 反向应用——上面的数据就是这么取的。
  • 第 4 个新测试(keeps reporting the active session workspace despite a conflicting next-session selection)在 main 上也通过。它是防回归的守卫测试,不是 RED/GREEN 证据——保留没问题,只是不能当成修复生效的证明。

2. 真实浏览器 A/B —— 修复有效 ✅

先在 ws-secondary 里跑一轮真实对话,再点击侧栏 ws-primary 下的 “New task”;宿主会按 onSessionIdChange 上报的内容重新挂载 shell(即 packages/web-shell/README.md 里记录的嵌入契约)。

real-stack A/B

上报给 host 的内容 composer 标签
main (undefined, "6d827c62…", <ws-secondary>) ws-secondary
PR #7910 (undefined, undefined, <ws-primary>) ws-primary

所报告的问题在真实链路的 main 上确实复现,打上本 PR 后消失。这正是 PR 描述里的主张,且是在单测 mock 之外得到确认的。

3. 遗留缺口 —— 出货形态的 standalone Web Shell 并没有被修好 🟠

同一个 daemon、同样两个 workspace,但驱动的是出货入口(client/main.tsx,也就是 qwen serve 打开的那个页面),而不是路由型宿主:

standalone gap

client/App.tsx WebShellSidebar.tsx 自动选中 composer 标签 下一条 prompt 实际运行于
main 原样 ws-secondary ws-secondary
PR #7910 原样 ws-secondary ws-secondary
main 禁用(探针) ws-primary ws-primary
PR #7910 禁用(探针) ws-primary ws-primary

「下一条 prompt 实际运行于」这一列是 daemon 侧的事实(GET /workspace/<cwd>/sessions),不是读 UI——即使打上本 PR,在 ws-primary 下新建的会话,创建出来的 session 仍然落在 ws-secondary

决定性因素根本不在 App.tsx,而是 WebShellSidebar.tsx 里的这个 effect:

// 无 session + 未选中 -> 重新选中 connection 上一次指向的 workspace
const connectedWorkspace = workspaces.find((e) => e.cwd === connection.workspaceCwd);
if (connectedWorkspace && !connectedWorkspace.primary) {
  onSelectWorkspace?.(connectedWorkspace.cwd);

selectedWorkspaceCwdundefined 来表示「primary」,所以在 primary workspace 新建聊天,恰好落进这个 effect 判定为「采用残留 workspace」的状态。在 observer 模式的记录里可以直接看到这个过程:shell 先上报 <ws-primary>(修复生效),紧接着又上报回 <ws-secondary>。而在路由型宿主下,中间那次上报会触发重新挂载并重置 connection.workspaceCwd,这就是修复在那边「站得住」、在这边站不住的原因。

不是回归——main 的表现完全一致。但 PR 描述里的手动测试计划(「预期:composer 的 workspace 指示显示 A」)对 qwen serve 自带的 web UI 并不成立。我建议要么把这条主张收窄到路由型宿主,要么把 sidebar 这个 effect 一并纳入本 PR 或后续 PR。

还有一点在两个分支上都能看到,所以同样是既有问题:重新挂载期间 shell 会短暂上报 (sessionId, undefined, undefined)。按这个三元组做路由的宿主会被弹回 primary workspace,而实际会话并不在那里。建议在 README 的契约里补一句:workspaceCwd 缺失表示「未知」,而不是「primary」。

4. 回归检查

  • packages/web-shell 全量套件:2387 / 2397。10 个失败全在 build-artifact.test.ts,它需要先跑 vite build --config vite.lib.config.ts;补上这步构建后是 10/10。与本 PR 无关。
  • 两个改动文件的 eslint --max-warnings 0:干净。
  • tsc -p tsconfig.json --noEmit:3 个报错,全都来自我本地 node_modules 覆盖层把 @qwen-code/webui / @qwen-code/sdk 解析到了较旧的构建产物(pickWorkspaceDirectorytoolUseId),且都在本 PR 未触及的文件里。不是 PR 的问题。

结论

改动是正确的,对于按 onSessionIdChange 做路由的宿主,确实修好了所报告的问题。就我个人而言,在这个基础上可以合并。合并前后我希望能处理的两点,对 diff 来说是小事,但对用户不是:一是描述里的手动测试主张,二是 sidebar 的自动选中——它在出货 UI 里仍会把 primary workspace 的新会话送进残留的 workspace。

@yiliang114 yiliang114 left a comment

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.

LGTM — clean fix. No active session → report locked ?? selected ?? primary workspace instead of stale connection cwd. Active-session path unchanged. Last-notified guard consistent. 4 regression tests cover the bug scenario, /clear, active-session isolation, and selection re-notification.

@wenshao
wenshao deleted the fix/web-shell-new-session-workspace branch July 28, 2026 13:55

@doudouOUC doudouOUC left a comment

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.

No issues found. LGTM! ✅

中文说明

未发现问题。LGTM!✅

— qwen3.7-max via Qwen Code /review

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.21.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants