Skip to content

fix(web-shell): isolate slash command plugin pages - #7581

Merged
ytahdn merged 1 commit into
QwenLM:mainfrom
chiga0:codex/web-shell-shadow-dom-tools
Jul 23, 2026
Merged

fix(web-shell): isolate slash command plugin pages#7581
ytahdn merged 1 commit into
QwenLM:mainfrom
chiga0:codex/web-shell-shadow-dom-tools

Conversation

@ytahdn

@ytahdn ytahdn commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Extends the existing shadowDom.plugins option to isolate every plugin-management surface, including the unified Plugins page and the compatibility pages opened by /extensions, /mcp, and /skills. It also recognizes the agents panel introduced by #7572 so the agents manager and its nested create/edit page use the same boundary once that change is present.

Why it's needed

Consumers can opt into Shadow DOM isolation for the unified Plugins page, but opening the equivalent managers through slash commands currently renders them in the host page's Light DOM. That creates inconsistent style isolation depending on how the same management UI was opened.

Reviewer Test Plan

How to verify

Embed Web Shell with shadowDom={{ plugins: true, portals: false }}. Open the unified Plugins page, then run /extensions manage, /mcp, and /skills details; each manager body should render under the plugins ShadowRoot. Open Settings, Daemon Status, and Session Overview and confirm they remain in the Light DOM. Enabling shadowDom.portals should remain the only way to move dialogs, dropdowns, and other portal content into a ShadowRoot.

With #7572 present, open /agents and its create/edit flow; both the manager and nested form should remain in the same plugins ShadowRoot.

Evidence (Before & After)

Before: only the unified Plugins page body was isolated; compatibility pages opened by slash commands rendered in the Light DOM.

After: the unified page and all plugin-management compatibility pages share the same opt-in Shadow DOM behavior, while unrelated panels and portals retain their existing behavior.

Tested on

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

Environment (optional)

Local standalone Web Shell via npm run dev:daemon; 183 targeted Vitest tests, Web Shell typecheck, build, lint, and formatting checks passed.

Risk & Scope

  • Main risk or tradeoff: feat(web-shell): add workspace agent management #7572 changes the same panel-rendering area and may require a manual merge conflict resolution; its agents branch must remain inside the shared boundary.
  • Not validated / out of scope: Windows and Linux manual browser verification; portal isolation behavior is intentionally unchanged.
  • Breaking changes / migration notes: None. Shadow DOM remains opt-in.

Linked Issues

Related to #7572.

中文说明

本 PR 做了什么

扩展现有的 shadowDom.plugins 选项,使它隔离所有插件管理界面,包括统一 Plugins 页面以及通过 /extensions/mcp/skills 打开的兼容页面。同时预先识别 #7572 引入的 agents 面板,使该改动合入后 Agents 管理页及其内部的创建/编辑页面使用同一个 Shadow DOM 边界。

为什么需要

使用方可以为统一 Plugins 页面启用 Shadow DOM 隔离,但通过斜杠命令打开相同管理功能时,页面目前仍渲染在宿主页的 Light DOM 中。这会导致同一管理界面因打开方式不同而具有不一致的样式隔离行为。

Reviewer 测试计划

验证方式

使用 shadowDom={{ plugins: true, portals: false }} 嵌入 Web Shell。打开统一 Plugins 页面,然后分别执行 /extensions manage/mcp/skills details;每个管理页面主体都应渲染在 plugins ShadowRoot 中。打开 Settings、Daemon Status 和 Session Overview,确认它们仍在 Light DOM 中。只有启用 shadowDom.portals 时,弹窗、下拉菜单等 portal 内容才应进入 ShadowRoot。

在包含 #7572 的代码上打开 /agents 及其创建/编辑流程;管理页和内部表单应始终位于同一个 plugins ShadowRoot 中。

前后对比证据

修改前:只有统一 Plugins 页面主体被隔离,通过斜杠命令打开的兼容页面渲染在 Light DOM 中。

修改后:统一页面和所有插件管理兼容页面共享相同的可选 Shadow DOM 行为,无关面板和 portal 保持原有行为。

测试环境

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

环境(可选)

通过 npm run dev:daemon 启动本地 standalone Web Shell;183 个定向 Vitest 测试、Web Shell 类型检查、构建、lint 和格式检查均通过。

风险与范围

  • 主要风险或取舍:feat(web-shell): add workspace agent management #7572 修改了相同的面板渲染区域,合并时可能需要手工解决冲突;其 agents 分支必须保留在共享边界内。
  • 未验证或不在范围内:未在 Windows 和 Linux 浏览器中手工验证;portal 隔离行为有意保持不变。
  • 破坏性变更或迁移说明:无,Shadow DOM 仍为可选功能。

关联 Issue

关联 #7572

@ytahdn

ytahdn commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator Author

E2E verification report

  • Environment: macOS, local standalone Web Shell started with npm run dev:daemon.
  • Configuration: shadowDom={{ plugins: true }} was temporarily enabled in the standalone entry point for verification and removed before submission.
  • Verified flows: unified Plugins page, /extensions manage, /mcp, and /skills details.
  • Expected result: every manager body renders in the plugins ShadowRoot while portal content remains controlled independently.
  • Result: passed local user verification.
  • Automated regression coverage: 183 targeted Vitest tests passed, including explicit ShadowRoot assertions for the unified page and all three slash-command compatibility pages.
  • Forward compatibility: the agents panel from feat(web-shell): add workspace agent management #7572 is covered by the shared panel classifier; its manager and nested create/edit page should share the same boundary once feat(web-shell): add workspace agent management #7572 is present.

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: observed inconsistency — the unified Plugins page respects shadowDom.plugins, but the same management UI opened via /extensions, /mcp, or /skills renders in the Light DOM. Before/after is described clearly.

Direction: aligned. This is a straightforward consistency fix within the web-shell Shadow DOM boundary — no auth, sandbox, or model-selection surface is touched.

Size: not applicable (no core paths). 6 files, +142/−33 — production logic ~72 lines in App.tsx + shadowDom.ts, tests ~58 lines, docs ~30 lines.

Approach: the scope feels right. Hoisting the existing ShadowDomBoundary to wrap the whole panel body and gating it on a panel-ID set is the minimal change that solves the problem. The agents entry in the set is inert until #7572 lands — reasonable forward-compat. No unrelated changes or drive-by refactors.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:已观测到的不一致——统一 Plugins 页面遵循 shadowDom.plugins,但通过 /extensions/mcp/skills 打开的相同管理界面仍渲染在 Light DOM 中。Before/after 描述清晰。

方向:对齐。这是 web-shell Shadow DOM 边界内的一致性修复,未触及 auth、sandbox 或模型选择。

规模:不适用(未触及核心路径)。6 个文件,+142/−33——生产逻辑约 72 行(App.tsx + shadowDom.ts),测试约 58 行,文档约 30 行。

方案:范围合理。将现有 ShadowDomBoundary 提升到包裹整个面板主体,并通过面板 ID 集合控制启用,是解决问题的最小改动。agents 条目在 #7572 合入前处于惰性状态——合理的前向兼容。无无关改动或顺手重构。

进入代码审查 🔍

Qwen Code · qwen3.8-max-preview

Reviewed at 998a3cd3de25e11960432a33d4bcf1a77ad992bb · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

The approach is exactly what I'd propose independently: hoist the existing ShadowDomBoundary from wrapping only the plugins panel to wrapping the entire panelBody, and gate it on a panel-ID set. ShadowDomBoundary with enabled={false} returns children directly, so non-plugin panels (settings, status, sessions) are completely unaffected — no behavioral change for them.

isPluginShadowPanel is a clean Set lookup in shadowDom.ts, well-separated from the rendering logic. The agents entry is inert until #7572 introduces the agents inline panel — no dead code path today.

One minor note: for the extensions panel, initialFocusRef={panelHeadingRef} is set on both the boundary and the ExtensionsManagerPage prop. Both point to the same element, so the double-focus is a no-op — not a blocker.

No correctness bugs, security issues, or regressions found. No AGENTS.md violations.

Test Results

$ npx vitest run client/shadowDom.test.ts

 ✓ shadowDom.test.ts (13 tests) 13ms
   ✓ isPluginShadowPanel > includes the plugins management surface
   ✓ isPluginShadowPanel > includes the extensions management surface
   ✓ isPluginShadowPanel > includes the mcp management surface
   ✓ isPluginShadowPanel > includes the skills management surface
   ✓ isPluginShadowPanel > includes the agents management surface
   ✓ isPluginShadowPanel > excludes the null non-plugin surface
   ✓ isPluginShadowPanel > excludes the settings non-plugin surface
   ✓ isPluginShadowPanel > excludes the status non-plugin surface
   ✓ isPluginShadowPanel > excludes the sessions non-plugin surface

 Test Files  1 passed (1)
      Tests  13 passed (13)

$ npx vitest run client/App.test.tsx -t "shadow-isolat"

 ✓ App.test.tsx (170 tests | 166 skipped) 211ms
   ✓ shadow-isolates the unified plugin manager body when plugins is enabled
   ✓ shadow-isolates the /extensions manage compatibility page when plugins is enabled
   ✓ shadow-isolates the /mcp compatibility page when plugins is enabled
   ✓ shadow-isolates the /skills details compatibility page when plugins is enabled

 Test Files  1 passed (1)
      Tests  4 passed | 166 skipped (170)

$ npx vitest run   (full web-shell suite)

 Test Files  126 passed (126)
      Tests  2078 passed (2078)

$ npx tsc --noEmit   → clean
$ npm run build      → ✓ built in 10.84s
$ npx eslint <changed files> → clean

tmux is not available on this CI runner, so browser-level verification was not possible. The unit and integration tests above cover the shadow-isolation behavior for each panel type.

中文说明

代码审查

方案与我独立提出的一致:将现有 ShadowDomBoundary 从仅包裹 plugins 面板提升到包裹整个 panelBody,并通过面板 ID 集合控制启用。ShadowDomBoundaryenabled={false} 时直接返回 children,因此非插件面板(settings、status、sessions)完全不受影响。

isPluginShadowPanelshadowDom.ts 中简洁的 Set 查找,与渲染逻辑良好分离。agents 条目在 #7572 引入 agents 内联面板前处于惰性状态——当前无死代码路径。

一个小注意点:extensions 面板的 initialFocusRef={panelHeadingRef} 同时设置在 boundary 和 ExtensionsManagerPage prop 上,两者指向同一元素,双重聚焦为空操作——不构成阻塞。

未发现正确性 bug、安全问题或回归。无 AGENTS.md 违规。

测试结果

shadowDom 单元测试 13 项全部通过;App 集成测试中 4 项 shadow 隔离测试全部通过;完整 web-shell 测试套件 2078 项全部通过。类型检查、构建、lint 均干净。

此 CI 运行环境无 tmux,无法进行浏览器级验证。上述单元和集成测试覆盖了各面板类型的 shadow 隔离行为。

Qwen Code · qwen3.8-max-preview

Reviewed at 998a3cd3de25e11960432a33d4bcf1a77ad992bb · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 5/5

Small, focused fix that does exactly one thing: extends the existing shadowDom.plugins boundary to cover slash-command compatibility pages. The implementation reuses the existing ShadowDomBoundary component, adds a 5-line Set lookup, and moves the boundary up one level in the render tree. No new abstractions, no scope creep.

All 2078 web-shell tests pass, typecheck and build are clean, lint is clean. The new tests cover each compatibility page individually. The agents forward-compat entry is inert and well-documented in the design doc.

LGTM, approving. ✅

中文说明

置信度:5/5

小而专注的修复,只做一件事:将现有 shadowDom.plugins 边界扩展到斜杠命令兼容页面。实现复用了现有 ShadowDomBoundary 组件,新增 5 行 Set 查找,并将边界在渲染树中上移一层。无新抽象,无范围蔓延。

全部 2078 项 web-shell 测试通过,类型检查和构建干净,lint 干净。新测试逐一覆盖各兼容页面。agents 前向兼容条目处于惰性状态,设计文档中有清晰说明。

LGTM,批准。 ✅

Qwen Code · qwen3.8-max-preview

Reviewed at 998a3cd3de25e11960432a33d4bcf1a77ad992bb · 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

Copy link
Copy Markdown
Collaborator

🖼️ web-shell visual preview

Rendered against a mock daemon (no real backend): the PR base vs this PR head 998a3cd. 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

.filter(Boolean)
.join(' ')}
styles={shadowDomOptions.styles}
initialFocusRef={

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] initialFocusRef only covers plugins and extensions; mcp/skills/agents fall through to undefined. If these panels manage focus internally, a brief comment (e.g. // mcp/skills/agents manage focus internally) would prevent future maintainers from thinking it's an oversight.

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

@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 lift of ShadowDomBoundary from per-component to panel-level, conditioned on isPluginShadowPanel(). Tests cover inclusion/exclusion correctly. One inline nit on initialFocusRef coverage.

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

@ytahdn
ytahdn added this pull request to the merge queue Jul 23, 2026
Merged via the queue into QwenLM:main with commit 74a786d Jul 23, 2026
58 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.

3 participants