Skip to content

fix(web-shell): initialize workspace selector from ID - #7518

Merged
wenshao merged 3 commits into
QwenLM:mainfrom
patrick-andstar:codex/fix-webshell-workspace-id
Jul 23, 2026
Merged

fix(web-shell): initialize workspace selector from ID#7518
wenshao merged 3 commits into
QwenLM:mainfrom
patrick-andstar:codex/fix-webshell-workspace-id

Conversation

@patrick-andstar

Copy link
Copy Markdown
Contributor

What this PR does

Initializes the visible, unlocked Web Shell workspace selector from the workspace resolved for a supplied workspace ID. A new chat therefore begins in the selected secondary workspace while users can still change that selection.

Why it's needed

When an embedding supplies only a non-primary workspace ID, the session is already scoped to that workspace but the native selector initially displays the primary workspace. The selector can then overwrite the intended target before the first prompt, causing a new session to be created in the primary workspace.

Reviewer Test Plan

How to verify

  1. Register a primary workspace and a trusted secondary workspace with the daemon.
  2. Render an unlocked Web Shell with the secondary workspace ID, without supplying a workspace path or lock.
  3. Confirm the selector initially shows the secondary workspace and remains available for changes.
  4. Send the first prompt. Confirm that the created session belongs to the secondary workspace.

Evidence (Before & After)

Before: an unlocked embedding that supplied only a secondary workspace ID showed the primary workspace in the native selector, so the first new session could be created in the primary workspace.

After: the selector starts on the resolved secondary workspace, remains unlocked, and the first new session uses that workspace. Automated regression coverage verifies both the resolved-ID selector initialization and the first-session creation path.

Tested on

OS Status
🍏 macOS ⚠️ not tested
🪟 Windows ✅ tested
🐧 Linux ⚠️ not tested

Environment (optional)

Windows 11, Node.js v24.11.1, npm 11.6.2, and the package-level Vitest configuration. The focused Web Shell regression suite passed 182 tests; formatting and ESLint checks passed for the four changed files.

Risk & Scope

  • Main risk or tradeoff: The change only determines the initial in-memory selection for unlocked, ID-based embeddings. Active sessions, explicit path selection, locked workspaces, and later user changes retain their existing behavior.
  • Not validated / out of scope: macOS and Linux were not tested locally, and no visual capture was produced. Full repository preflight and typecheck cannot run in this sparse checkout: preflight stops because scripts/clean.js is unavailable, while typecheck lacks the SDK and Web UI declarations required by this checkout. The focused Vitest run required a temporary local module-resolution workaround because this checkout does not contain the SDK package metadata; it was removed and is not part of this PR.
  • Breaking changes / migration notes: None expected. Duplicate-work check: open pull requests were searched using workspaceId and 7430; no open pull request fixes this selector initialization.

Linked Issues

Fixes #7430

中文说明

本 PR 做了什么

本 PR 会根据传入的工作区 ID 所解析出的工作区,初始化可见且未锁定的 Web Shell 工作区选择器。因此,新聊天会在所选的次级工作区中开始,同时用户仍可修改该选择。

为什么需要它

当嵌入方只传入非主工作区 ID 时,会话本身已经限定在该工作区,但原生选择器初始仍显示主工作区。选择器随后可能在首次提示词之前覆盖预期目标,导致新会话被创建在主工作区。

Reviewer 测试计划

如何验证

  1. 在 daemon 中注册一个主工作区和一个受信任的次级工作区。
  2. 使用次级工作区 ID 渲染未锁定的 Web Shell,不传入工作区路径或锁定配置。
  3. 确认选择器初始显示次级工作区,并且仍可以修改选择。
  4. 发送第一条提示词。确认创建出的会话属于次级工作区。

证据(Before & After)

Before:只传入次级工作区 ID 的未锁定嵌入会在原生选择器中显示主工作区,因此第一个新会话可能被创建在主工作区。

After:选择器会从解析出的次级工作区开始,保持未锁定状态,第一个新会话也会使用该工作区。自动化回归覆盖同时验证了解析出的 ID 对选择器的初始化,以及第一个会话的创建路径。

测试平台

OS 状态
🍏 macOS ⚠️ 未测试
🪟 Windows ✅ 已测试
🐧 Linux ⚠️ 未测试

环境(可选)

Windows 11、Node.js v24.11.1、npm 11.6.2,以及 package 级 Vitest 配置。聚焦的 Web Shell 回归套件通过了 182 个测试;4 个改动文件的格式化和 ESLint 检查均已通过。

风险与范围

  • 主要风险或权衡:本改动只决定未锁定、基于 ID 的嵌入场景中的内存初始选择。活跃会话、显式路径选择、锁定工作区和用户后续的修改都保持既有行为。
  • 未验证或范围外:未在本地测试 macOS 和 Linux,也没有生成可视化截图或录屏。完整仓库 preflight 和 typecheck 无法在此稀疏检出中运行:preflight 因 scripts/clean.js 不可用而停止,typecheck 则缺少此检出所需的 SDK 与 Web UI 声明。聚焦的 Vitest 运行因该检出不包含 SDK 包元数据而需要临时本地模块解析 workaround;该临时内容已移除,且不属于本 PR。
  • Breaking changes / migration notes:预计不存在。重复工作检查:已使用 workspaceId7430 搜索开放 Pull Request,未发现修复这一选择器初始化问题的开放 PR。

关联 Issue

Fixes #7430

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: observed bug with clear evidence — #7430 was filed by a maintainer with step-by-step reproduction. When an embedding supplies only a secondary workspaceId, the session is correctly scoped but the native selector still shows the primary workspace, so the first new session can land in the wrong workspace. This is a real, reproducible issue.

Direction: aligned. The web-shell workspace selector should reflect the workspace resolved from the supplied ID. CHANGELOG has no direct reference, but the area is clearly relevant — workspace initialization is core to the embedding contract.

Size: not applicable — no core paths touched. 56 additions / 1 deletion across 4 files (2 source, 2 test).

Approach: the scope feels right. The fix threads a single initialSelectedWorkspaceCwd prop from WorkspaceSessionProvider into App's useState initializer — exactly the minimal change needed. No unrelated edits, no drive-by refactors. The condition effectiveWorkspaceId ? targetWorkspace?.cwd : undefined correctly limits this to the ID-based (unlocked) path, leaving the locked and cwd-based paths untouched.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:已观测到的 bug,有明确证据——#7430 由维护者提交,附带逐步复现步骤。当嵌入方只传入次级 workspaceId 时,会话已正确限定在该工作区,但原生选择器仍显示主工作区,导致第一个新会话可能被创建在错误的工作区。这是一个真实、可复现的问题。

方向:对齐。Web Shell 工作区选择器应反映从传入 ID 解析出的工作区。CHANGELOG 无直接引用,但该领域显然相关——工作区初始化是嵌入契约的核心部分。

规模:不适用——未触及核心路径。4 个文件(2 个源码、2 个测试)共 56 行新增 / 1 行删除。

方案:范围合理。修复方案将单个 initialSelectedWorkspaceCwd prop 从 WorkspaceSessionProvider 传入 AppuseState 初始值——恰好是所需的最小改动。无无关编辑,无顺手重构。条件 effectiveWorkspaceId ? targetWorkspace?.cwd : undefined 正确地将此限制在基于 ID(未锁定)的路径,锁定和基于 cwd 的路径不受影响。

进入代码审查 🔍

Qwen Code · qwen3.8-max-preview

Reviewed at 4b583d45c46774803fb3358c37fdb6ffeaa85b10 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal: given the issue description, I would (1) pass the resolved targetWorkspace?.cwd from WorkspaceSessionProvider to App as an initial workspace prop, gated on the ID-based (unlocked) path only, (2) use it as the useState initializer for selectedWorkspaceCwd in App, and (3) add regression tests at both the provider and App levels.

Comparison: the PR does exactly this. No simpler path was missed.

No critical blockers or convention violations found. Specifics:

  • WorkspaceSessionProvider.tsx: the new initialSelectedWorkspaceCwd prop is correctly gated on effectiveWorkspaceId — it only fires when the workspace was resolved by ID (not by cwd or lock). The locked path continues to use lockedWorkspaceCwd separately.
  • App.tsx: the useState initializer change from undefined to initialSelectedWorkspaceCwd is the minimal fix. The existing trust-check useEffect (lines 1387–1395) still guards against untrusted workspaces by resetting to undefined, so no security gap is introduced.
  • Tests: both new tests target the exact bug scenario — index.test.tsx verifies the provider passes the correct prop without locking, and App.test.tsx verifies the selector shows the secondary workspace and the first session is created there.

ESLint and Prettier pass on all four changed files.

Testing

This is a web-shell React component change (browser-based UI), so CLI tmux testing does not apply. Verification was done via the package-level Vitest suite, which exercises the component tree through React Testing Library — the same rendering path a browser would use.

 ✓ App.test.tsx (165 tests) 5895ms
 ✓ index.test.tsx (17 tests)

 Test Files  2 passed (2)
      Tests  182 passed (182)
   Duration  7.37s

The two new tests specifically cover the fix:

  • initializes an unlocked workspace selector from workspace id — confirms WebShellWithProviders with workspaceId="secondary" passes initialSelectedWorkspaceCwd: '/work/secondary' to App without setting lockedWorkspaceCwd.
  • creates first sessions in the initial unlocked workspace — confirms the selector shows the secondary workspace and createSession is called with workspaceCwd: '/work/secondary'.
中文说明

代码审查

独立方案: 根据 issue 描述,我会 (1) 将解析出的 targetWorkspace?.cwdWorkspaceSessionProvider 作为初始工作区 prop 传给 App,仅在基于 ID(未锁定)的路径下生效,(2) 在 App 中将其用作 selectedWorkspaceCwduseState 初始值,(3) 在 provider 和 App 两个层级添加回归测试。

对比: PR 的实现与此完全一致。没有遗漏更简单的路径。

未发现关键阻塞项或规范违反。具体:

  • WorkspaceSessionProvider.tsx:新的 initialSelectedWorkspaceCwd prop 正确地以 effectiveWorkspaceId 为条件——仅在通过 ID 解析工作区时生效(非 cwd 或锁定路径)。锁定路径继续使用独立的 lockedWorkspaceCwd
  • App.tsxuseState 初始值从 undefined 改为 initialSelectedWorkspaceCwd 是最小修复。现有的信任检查 useEffect(1387–1395 行)仍然会在不受信任的工作区时重置为 undefined,因此不会引入安全漏洞。
  • 测试:两个新测试都针对确切的 bug 场景——index.test.tsx 验证 provider 传递正确的 prop 且不锁定,App.test.tsx 验证选择器显示次级工作区且第一个会话在该工作区创建。

ESLint 和 Prettier 在所有四个改动文件上通过。

测试

这是 Web Shell React 组件变更(浏览器 UI),CLI tmux 测试不适用。验证通过 package 级 Vitest 套件完成,该套件通过 React Testing Library 执行组件树——与浏览器使用的渲染路径相同。

两个新测试具体覆盖了修复:

  • initializes an unlocked workspace selector from workspace id——确认 WebShellWithProviders 传入 workspaceId="secondary" 时向 App 传递 initialSelectedWorkspaceCwd: '/work/secondary' 且不设置 lockedWorkspaceCwd
  • creates first sessions in the initial unlocked workspace——确认选择器显示次级工作区且 createSessionworkspaceCwd: '/work/secondary' 调用。

Qwen Code · qwen3.8-max-preview

Reviewed at 4b583d45c46774803fb3358c37fdb6ffeaa85b10 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 5/5 — clean, minimal fix for a maintainer-filed bug; matches my independent proposal exactly.

This is a textbook small fix: one prop threaded through, one useState initializer changed, two targeted regression tests. The bug is real (filed by a maintainer with clear repro), the fix is the minimum that solves it, and the existing trust-check effect still guards the boundary. Nothing to maintain, nothing to curse in six months.

The PR also does the small things right — the condition effectiveWorkspaceId ? targetWorkspace?.cwd : undefined avoids touching the locked or cwd-based paths, and the tests verify both the prop threading and the end-to-end session creation.

中文说明

置信度:5/5 — 干净、最小的修复,针对维护者提交的 bug;与我的独立方案完全一致。

这是一个教科书式的小修复:穿透一个 prop,更改一个 useState 初始值,添加两个精准的回归测试。Bug 是真实的(由维护者提交,附清晰复现),修复是解决问题所需的最小改动,现有的信任检查 effect 仍然守护着边界。六个月后不需要维护,不会让人抓狂。

PR 在细节上也做对了——条件 effectiveWorkspaceId ? targetWorkspace?.cwd : undefined 避免触及锁定或基于 cwd 的路径,测试同时验证了 prop 传递和端到端会话创建。

Qwen Code · qwen3.8-max-preview

Reviewed at 4b583d45c46774803fb3358c37fdb6ffeaa85b10 · re-run with @qwen-code /triage

@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. ✅

@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.

No issues found. LGTM! ✅

— qwen3.7-max via Qwen Code /review

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

🖼️ web-shell visual preview

Rendered against a mock daemon (no real backend): the PR base vs this PR head 4b583d4. 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 2 render-shaping files:

  • packages/web-shell/client/App.tsx
  • packages/web-shell/client/components/WorkspaceSessionProvider.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

@wenshao

wenshao commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

✅ Local build & test verification — merge-ready

I built and ran this PR locally in an isolated worktree at head 4b583d45, with a controlled A/B against the merge-base d064bd7d. Verdict: correct, minimal, load-bearing, and clean — good to merge.

Environment: macOS (Darwin 24.6), Node 22, packages/web-shell with vitest@3.2.4; node_modules linked from a fully-built checkout so @qwen-code/{sdk,webui} resolve for both vitest and tsc.

Summary

Check Result
New test — App.test.tsx › creates first sessions in the initial unlocked workspace ✅ pass
New test — index.test.tsx › initializes an unlocked workspace selector from workspace id ✅ pass
Controlled revert A (drop the App.tsx useState initializer) ✅ App test turns red — fix is load-bearing
Controlled revert B (drop the WorkspaceSessionProvider pass-through) ✅ index test turns red — wiring is load-bearing
Full packages/web-shell suite — regression parity vs base +2 new tests, 0 regressions
prettier --check + eslint (4 changed files) ✅ clean
tsc -p tsconfig.json --noEmit 0 errors
Live browser E2E of the real ?workspace=<id> embedding path ✅ fixed selector renders unlocked on the secondary workspace

Controlled A/B (the proof it is load-bearing)

The two new tests independently cover the two production edits. Reverting each edit — while keeping its test — turns exactly that test red, then restoring it turns it green again:

  • Revert AApp.tsx: useState(initialSelectedWorkspaceCwd)useState(undefined)App.test.tsx fails with expected undefined to be '/work/secondary'. This is the bug: without the initializer the composer's selectedWorkspaceCwd is undefined, so the first new session is created in the primary workspace.
  • Revert B — remove initialSelectedWorkspaceCwd={effectiveWorkspaceId ? targetWorkspace?.cwd : undefined}index.test.tsx fails: App never receives the resolved secondary cwd.

Controlled A/B verification

Regression parity (same env, base vs head)

Full packages/web-shell suite: base = 2027 passed / 9 failed (2036), head = 2029 passed / 9 failed (2038). The delta is exactly the 2 new tests. The 9 failures are identical on both trees — all in build-artifact.test.ts, which reads ../dist/index.js (a vite build output this focused vitest run does not produce). They are untouched by the PR and are environmental, not a regression. (Heads-up for anyone running the package suite in isolation: run npm run build first, or scope to the client tests.)

Live browser end-to-end

I also drove the real unlocked embedding path (?workspace=secondary, no workspaceCwd/lock) against a mock daemon registering a primary + a trusted secondary workspace, on both base and head. On head, the composer's workspace selector initializes to the resolved Secondary workspace and stays unlocked/changeable — exactly the expected behavior:

Fixed: unlocked selector initialized to Secondary

On base the same selector initializes to Primary at first paint (the bug), then a post-mount effect settles it back to Secondary — i.e. the first-render/first-prompt race the issue describes. That window is inherently transient in a live browser, which is why the deterministic contract (initial selectedWorkspaceCwd and the first createSession target) is best pinned by the unit tests above — and it is.

Notes

  • Condition is correctly scoped: effectiveWorkspaceId ? targetWorkspace?.cwd : undefined only touches the unlocked ID-based path; locked / explicit-path / active-session flows keep their existing behavior (the trust-reset effect at App.tsx still guards untrusted targets).
  • No prior maintainer verification comment or pr-assets branch existed for this PR; this is the first.

LGTM. ✅

Screenshots are hosted on branch pr-assets/pr-7518-verify. Verification used an isolated worktree; no changes were pushed to the PR branch.

中文版本(点击展开)

✅ 本地构建与测试验证 —— 可以合并

我在隔离 worktree 中检出 PR 头 4b583d45 做了本地构建与测试,并与合并基 d064bd7d 做了受控 A/B 对照。结论:修复正确、最小、可承载(load-bearing)、干净 —— 建议合并。

环境: macOS(Darwin 24.6)、Node 22、packages/web-shell + vitest@3.2.4node_modules 链接自一个完整构建过的检出,因此 @qwen-code/{sdk,webui}vitesttsc 都能正常解析。

汇总

检查项 结果
新增测试 —— App.test.tsx › creates first sessions in the initial unlocked workspace ✅ 通过
新增测试 —— index.test.tsx › initializes an unlocked workspace selector from workspace id ✅ 通过
受控回退 A(去掉 App.tsxuseState 初始值) ✅ App 测试变 —— 修复是必需的
受控回退 B(去掉 WorkspaceSessionProvider 的透传) ✅ index 测试变 —— 接线是必需的
packages/web-shell 全量套件 —— 与 base 的回归对照 +2 新测试,0 回归
prettier --check + eslint(4 个改动文件) ✅ 干净
tsc -p tsconfig.json --noEmit 0 错误
真实 ?workspace=<id> 嵌入路径的浏览器端到端验证 ✅ 修复后的选择器在次级工作区上未锁定渲染

受控 A/B(证明修复是必需的)

两个新增测试分别独立覆盖两处生产代码改动。回退任一处改动(同时保留其对应测试),恰好让该测试变红,恢复后又变绿:

  • 回退 A —— App.tsxuseState(initialSelectedWorkspaceCwd)useState(undefined),导致 App.test.tsx 失败:expected undefined to be '/work/secondary'。这正是该 bug:没有初始值,composer 的 selectedWorkspaceCwdundefined第一个新会话会被创建在主工作区
  • 回退 B —— 删除 initialSelectedWorkspaceCwd={effectiveWorkspaceId ? targetWorkspace?.cwd : undefined},导致 index.test.tsx 失败:App 拿不到解析出的次级 cwd

受控 A/B 验证

回归对照(同一环境,base vs head)

packages/web-shell 全量套件:base = 2027 通过 / 9 失败(2036)head = 2029 通过 / 9 失败(2038)。差值正好是那 2 个新测试。那 9 个失败在两棵树上完全一致 —— 全部位于 build-artifact.test.ts,它读取 ../dist/index.js(这次聚焦的 vitest 运行不会产出该 vite build 产物)。它们与本 PR 无关,属于环境因素,并非回归。(提醒:单独跑该 package 套件时,请先 npm run build,或只跑 client 测试。)

浏览器端到端

我还在 base 与 head 上分别驱动了真实的未锁定嵌入路径(?workspace=secondary,不传 workspaceCwd/锁定),mock daemon 注册了一个主工作区 + 一个受信任的次级工作区。在 head 上,composer 的工作区选择器初始化为解析出的 Secondary,并保持未锁定/可切换 —— 完全符合预期:

修复后:未锁定选择器初始化到 Secondary

base 上,同一选择器在首帧初始化为 Primary(bug),随后一个 mount 后的 effect 又把它纠正回 Secondary —— 即 issue 所描述的首次渲染/首次提示的竞态窗口。该窗口在真实浏览器中本质上是瞬态的,因此其确定性契约(初始 selectedWorkspaceCwd 以及首个 createSession 的目标)最好由上面的单元测试来锁定 —— 而它确实做到了。

备注

  • 条件的作用域正确:effectiveWorkspaceId ? targetWorkspace?.cwd : undefined 只影响未锁定的基于 ID 的路径;锁定 / 显式路径 / 活跃会话等流程保持既有行为(App.tsx 中针对未受信任目标的 trust-reset effect 仍然生效)。
  • 该 PR 此前没有维护者的验证评论,也没有 pr-assets 分支;这是第一次。

LGTM。✅

截图托管于分支 pr-assets/pr-7518-verify。验证在隔离 worktree 中进行,未向 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.

Reviewed — no blockers. Suggestions are inline.

— qwen3.7-max via Qwen Code /review

Comment thread packages/web-shell/client/components/WorkspaceSessionProvider.tsx
fix: adjust initial workspace directory resolution logic to support embeddings passing only unlocked workspaceCwd
Replace condition check from effectiveWorkspaceId to !lockWorkspaceCwd && targetWorkspace, prevent sessions from incorrectly initializing under primary workspace for secondary workspace path inputs

Co-authored-by: qwen-code-ci-bot <qwen-code-ci@service.alibaba.com>
@patrick-andstar

Copy link
Copy Markdown
Contributor Author

@wenshao Thanks again for the detailed verification. The branch has been updated, but the current CI run is awaiting maintainer approval. When convenient, could you approve the pending workflows and give the PR final approval if it still looks good? Thank you!

@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.

No issues found. LGTM! ✅

— qwen3.7-max via Qwen Code /review

@wenshao
wenshao added this pull request to the merge queue Jul 23, 2026
Merged via the queue into QwenLM:main with commit aa11e8d Jul 23, 2026
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WebShell workspaceId does not initialize the native workspace selector

3 participants