fix(web-shell): pass session workspace cwd when opening from overview panel - #9730
Merged
Merged
Conversation
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
Collaborator
|
Thanks for the PR!
Moving on to code review. 🔍 中文说明感谢贡献！
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewReviewed
Test evidence (PR's own CI, fetched via API — PR code not executed locally)At the time of writing, the Linux unit suite ( Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Not verified here: the end-to-end daemon round-trip (panel click → load request carrying the session's own cwd → 200 instead of 404/409). The author's Playwright capture covers it on macOS, but CI's unit tests only pin the callback argument, and this review does not re-run PR code. Sandboxed verification would settle this: 中文说明代码审查审查了
测试证据（来自 PR 自身 CI，经 API 获取——未在本地执行 PR 代码）截至本评论发出，Linux 单测（ 此处未验证：端到端 daemon 往返（面板点击 → 加载请求携带会话自身 cwd → 200 而非 404/409）。作者的 Playwright 抓包在 macOS 上覆盖了该场景，但 CI 单测只钉住回调参数，本审查不重新执行 PR 代码。沙箱验证可以落定此事： — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 5/5 — a minimal, correctly-wired fix for an observed bug, with a regression test that pins the diff. This is exactly the change I would have made: the session cards already know their workspace, the switch path already accepts a workspace directory (the sidebar has always passed it), so the only thing missing was the panel forwarding what it already had. Three production lines, no downstream changes needed — I traced the full chain ( The Linux unit suite and the web-shell visual capture are still running on the reviewed commit, so approval is deferred until CI lands green on 中文说明置信度：5/5 —— 针对已观测 bug 的最小、接线正确的修复，且回归测试钉住了该改动。 这正是我会做的改法：会话卡片本就知道自己所属的 workspace，切换链路本就接受 workspace 目录（侧栏一直在传），唯一缺的只是面板把已有信息传下去。生产代码 3 行，下游无需任何改动——我完整追踪了链路（ Linux 单测与 web-shell 视觉采集仍在所审查的提交上运行，因此批准推迟到 CI 在该提交上全部变绿后，由 finalize 任务代为执行。 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
approved these changes
Aug 22, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
approved these changes
Aug 22, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM！✅
— qwen3.8-max via Qwen Code /review (v0.21.15)
wenshao
enabled auto-merge
August 22, 2026 15:03
ytahdn
reviewed
Aug 22, 2026
ytahdn
left a comment
ytahdn
left a comment
Collaborator
There was a problem hiding this comment.
Review — fix(web-shell): pass session workspace cwd when opening from overview panel
Static review of the single commit 5601d08 (SessionOverviewPanel.tsx + its test, +26/−4). No tests/builds were run; every claim below was re-checked against the head tree and its parent.
对单提交 5601d08 的纯静态审查（SessionOverviewPanel.tsx 及其测试，+26/−4）。未运行任何测试/构建，以下结论均已在 head tree 及其父提交上复核。
End-to-end wiring verified / 端到端链路已验证
The panel-side change is only half of a fix like this, so I traced the whole path:
这类修复的关键在于链路是否贯通，因此我追踪了完整路径：
-
SessionCard.workspaceCwdis required and populated fromsession.workspaceCwd(SessionOverviewPanel.tsx:112); the SDK typeDaemonSessionSummary.workspaceCwdis a requiredstring(sdk-typescript daemon types, the session-summary interface), so every card — primary or other workspace — carries its owning directory. No undefined gap. -
The click handler now passes
card.workspaceCwd(line ~420), and the widened(sessionId, workspaceCwd?) => voidsignature appears at exactly the two prop declarations (inner + exported component) — no third signature site was missed. -
The sole
<SessionOverviewPanel>render site in App.tsx (~12062) wireshandleOpenSessionFromOverview, which already carried the two-arg signature on the parent commit (verified there) and forwards vialoadSidebarSession(sessionId, workspaceCwd)tosessionActions.loadSession(sessionId, { workspaceCwd }). So the switch path was already cwd-capable (that's why the sidebar always worked); this PR supplies the one argument the panel had been dropping. -
The single-arg inline handlers elsewhere in App.tsx (~12139, ~12249) belong to the scheduled-tasks and goals pages — different components, not this panel — matching the PR's stated out-of-scope note; TypeScript-wise a narrower handler stays assignable to the widened prop type.
-
SessionCard.workspaceCwd为必填字段，取自session.workspaceCwd（SessionOverviewPanel.tsx:112）；SDK 类型DaemonSessionSummary.workspaceCwd是必填string（sdk-typescript daemon types 中的会话摘要接口），因此无论 primary 还是其他 workspace 的卡片都携带归属目录，不存在 undefined 缺口。 -
点击处理现在传出
card.workspaceCwd（约 420 行），扩展后的(sessionId, workspaceCwd?) => void签名恰好只出现在两处 prop 声明（内层 + 导出组件），没有遗漏第三个签名点。 -
App.tsx 中唯一的
<SessionOverviewPanel>渲染点（约 12062 行）接的是handleOpenSessionFromOverview，该回调在父提交上就已是双参签名（已在父提交核实），并经loadSidebarSession(sessionId, workspaceCwd)传到sessionActions.loadSession(sessionId, { workspaceCwd })。也就是说切换链路本就支持 cwd（这正是侧栏一直正常的原因）；本 PR 只是补上面板一直丢掉的那个参数。 -
App.tsx 其他单参内联回调（约 12139、12249 行）属于定时任务与 Goals 页面——是别的组件，不是本面板——与 PR 声明的范围外事项一致；TypeScript 层面更窄的回调对扩展后的 prop 类型依然可赋值。
Behavioral delta is exactly the intended one / 行为变化恰为预期
Because the switch path previously fell back to the primary workspace when the argument was missing, primary-workspace cards now pass a value equal to the old fallback — their behavior is unchanged by construction. Only cross-workspace cards change behavior: the load request targets the owning workspace instead of being rejected with the observed 404/409 codes. The diagnosis matches the mechanism (daemon resolving the session under the wrong workspace runtime).
由于切换链路在参数缺省时原本就回退到 primary workspace，primary 卡片现在传的值等于原回退值——其行为按构造不变。只有跨 workspace 卡片的行为发生变化：加载请求指向会话归属的 workspace，不再以观察到的 404/409 被拒。诊断与机制吻合（daemon 在错误的 workspace runtime 下解析会话）。
Tests and CI / 测试与 CI
The updated existing test pins the primary case with the new second argument, and the new cross-workspace regression test renders a two-workspace daemon, clicks the workspace-B card, and asserts onOpenSession receives ('b1', '/wsB'). Read-only confirmation: both are consistent with the implementation. CI on the head is green (Test ubuntu, web-shell E2E Smoke, visuals).
更新后的既有用例以新的第二参数钉住 primary 场景；新增的跨 workspace 回归测试渲染双 workspace daemon、点击 workspace-B 卡片并断言 onOpenSession 收到 ('b1', '/wsB')。静态确认两者与实现一致。head 上 CI 全绿（Test ubuntu、web-shell E2E Smoke、visuals）。
Praise / 值得肯定
-
Textbook minimal fix: the value was already accepted downstream, so the change is just plumbing the panel had been dropping — which also makes the primary-path no-op guarantee trivially true.
-
Strong evidence discipline: before/after Playwright captures with the concrete 404/409 error codes and request bodies, not just a description.
-
Honest scoping — the sidebar Archived gap and the goal/scheduled-task dialogs are called out as separate pre-existing issues rather than silently bundled.
-
教科书式的最小修复：下游本就接受该参数，改动只是补上面板丢掉的管道——这也使 primary 路径"行为不变"的保证天然成立。
-
证据扎实：给出修复前后的 Playwright 抓包、具体 404/409 错误码与请求体，而非仅文字描述。
-
范围诚实：侧栏 Archived 缺口与 Goals/定时任务弹窗被明确标为独立的既有问题，没有悄悄打包进本 PR。
Conclusion / 结论
💬 COMMENT — no blockers. A correct, minimal, well-evidenced fix with green CI.
💬 COMMENT——无阻塞项。修复正确、最小化、证据充分，CI 全绿。
ytahdn
approved these changes
Aug 22, 2026
ytahdn
left a comment
ytahdn
left a comment
Collaborator
There was a problem hiding this comment.
LGTM — approving.
Verified end-to-end on head 5601d08: the owning-workspace cwd is carried by every card (SDK session summaries require it), flows through the already two-arg switch path into sessionActions.loadSession, and primary-workspace behavior is unchanged by construction since the passed value equals the old fallback. Cross-workspace regression test pins the new argument; Test ubuntu, E2E Smoke, and visuals are all green.
已核实：每张卡片都携带归属 workspace 的 cwd（SDK 会话摘要中该字段必填），并经本就支持双参的切换链路传入 sessionActions.loadSession；primary workspace 行为按构造不变（传值等于原回退值）。跨 workspace 回归测试已钉住新参数，Test ubuntu / E2E Smoke / visuals 全绿。
This was referenced Aug 22, 2026
This was referenced Aug 24, 2026
Collaborator
|
Released in v0.22.2. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Clicking a session card in the Session Overview panel now hands the session's owning workspace directory to the session-switch path, so the load request targets the workspace where the session actually lives. The callback contract between the panel and the app is widened to carry that directory; nothing else changes.
Why it's needed
On a daemon registered with multiple workspaces, the overview panel lists sessions from every workspace. Clicking a session that belongs to a non-primary workspace used to drop the session's workspace association, so the switch fell back to the primary workspace. The daemon then looked for the session under the wrong workspace runtime and rejected the load —
404 session_not_foundfor a cold session, or409 session_workspace_conflictwhen the session was already live under its own runtime. The URL still navigated to/session/<id>(without the workspace selector), but the main view was left on the empty "New session" state and the user could not enter the session from the overview panel at all. The same session opened fine from the sidebar list, which already passed the workspace directory.Reviewer Test Plan
How to verify
node dist/cli.js serve --port 4173 --workspace <dir-a> --workspace <dir-b>.POST /session+POST /session/:id/promptwithcwdset to<dir-b>).POST /session/:id/loadrequest body carriescwd: <dir-b>(resolved, e.g./private/tmp/...on macOS), the daemon returns 200, the URL gains a?workspace=…selector, and the transcript loads. Before this fix the request carried the primary workspace'scwdand failed with 404/409, leaving the view on "New session".cd packages/web-shell && npx vitest run client/components/SessionOverviewPanel.test.tsx— 28/28 pass, including a new cross-workspace regression test.Evidence (Before & After)
Before (Playwright traffic capture, panel click on a workspace-B session): request body
{"cwd":"/private/tmp/qwen-webshell-repro",...}(primary workspace, wrong) →404 session_not_found, URL at/session/<id>without?workspace=, main view stuck on "New session", console loggedDaemonHttpError: POST /session/:id/load.After (same scenario, rebuilt daemon): request body
{"cwd":"/private/tmp/qwen-webshell-verify-b",...}(session's own workspace) →200, URL/session/<id>?workspace=aac870abadba792e, transcript loaded, no console errors or error toasts.Tested on
Environment (optional)
Built bundle (
npm run build), real daemon with two workspaces, headless Playwright chromium.Risk & Scope
Linked Issues
N/A (no open issue; reproduction notes recorded locally).
中文说明
本 PR 做了什么
从 Session Overview（会话管理）面板点击会话卡片时，现在会把该会话所属的 workspace 目录传给会话切换链路，使加载请求指向会话真实所在的 workspace。面板与 App 之间的回调签名相应扩展以携带该目录；其他逻辑不变。
为什么需要它
在多 workspace 注册的 daemon 上，会话管理面板会列出所有 workspace 的会话。此前点击属于非 primary workspace 的会话时，会话的 workspace 归属被丢弃，切换链路退回 primary workspace，daemon 便在错误的 workspace runtime 下找会话并拒绝加载：冷会话返回
404 session_not_found，会话已在其自身 runtime 下运行时返回409 session_workspace_conflict。此时 URL 虽然跳到了/session/<id>（但不带 workspace 选择参数），主区域却停留在 "New session" 空态，用户从会话管理面板完全进不去该会话。而同一个会话从侧栏会话列表点击是正常的，因为侧栏一直带着 workspace 目录。评审者验证计划
如何验证
node dist/cli.js serve --port 4173 --workspace <dir-a> --workspace <dir-b>。cwd指向<dir-b>的POST /session+POST /session/:id/prompt）。POST /session/:id/load请求体携带cwd: <dir-b>（解析后的路径，macOS 上形如/private/tmp/...），daemon 返回 200，URL 带上?workspace=…选择参数，transcript 正常加载。修复前请求体携带的是 primary workspace 的cwd，返回 404/409，页面停留在 "New session"。cd packages/web-shell && npx vitest run client/components/SessionOverviewPanel.test.tsx— 28/28 通过，其中包含新增的跨 workspace 回归测试。证据（修复前/修复后）
修复前（Playwright 抓包，面板点击 workspace-B 的会话）：请求体
{"cwd":"/private/tmp/qwen-webshell-repro",...}（primary workspace，错误）→404 session_not_found，URL 为不带?workspace=的/session/<id>，主区域卡在 "New session"，console 记录DaemonHttpError: POST /session/:id/load。修复后（同一场景，重新构建的 daemon）：请求体
{"cwd":"/private/tmp/qwen-webshell-verify-b",...}（会话自己的 workspace）→200，URL 为/session/<id>?workspace=aac870abadba792e，transcript 正常加载，无 console 报错、无错误 toast。测试环境
运行环境（可选）
构建产物（
npm run build），注册了两个 workspace 的真实 daemon，headless Playwright chromium。风险与范围
关联 Issue
N/A（暂无 issue；复现记录保存在本地）。