feat(web-shell): add opt-in composer add menu - #10379
Merged
Merged
Conversation
Collaborator
Author
E2E / verification report
|
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
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / after
Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Collaborator
Author
|
CI follow-up: the Web Shell smoke failure was a Playwright strict-selector collision after the new hidden file input was added. The history-search input now has a stable semantic data attribute and the E2E targets it directly. Re-ran the three failing responsive composer cases at 1000/800/600px: 3/3 passed. ChatEditor unit tests: 102/102 passed. The separate release-note helper failure was unrelated to this PR and passed when reproduced locally. |
Collaborator
Code reviewMy independent proposal for this problem (opt-in toolbar menu reusing the existing attachment/reference/skill lanes, with the
A follow-up commit landed during this review ( The one ordering decision worth a picture — selection actions are deferred until the menu's close handler so insertion and focus restoration happen after Radix finishes tearing down the flyouts: sequenceDiagram
participant P1 as User
participant P2 as AddMenu
participant P3 as ChatEditor handler
participant P4 as Composer input
P1->>P2: opens the plus menu
P2->>P2: loads submenu from shared sources
P1->>P2: selects an item
P2->>P2: stores the action as pending close work
P2->>P3: menu closes, pending action runs
P3->>P4: appends the tag or prepends the skill
P3->>P4: restores visible focus to the input
Files changed (24 of 24 shown)
Testing evidence — the PR's own CIThe head moved once during this review (one test-targeting commit, covered above); a fresh CI run is in flight on the reviewed commit, so the table below is a mid-flight snapshot — the finalize job will update it when CI settles. macOS/Windows unit suites are skipped by this PR's CI profile, not by a failure. Context on the previous commit's red check, since the same signature may still be around: on the prior head, What already ran green on the current head: Desktop Shell on ubuntu and windows, dependency CVE audit, PR classification. ESLint, Prettier, actionlint, shellcheck, yamllint, Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Sandboxed verification would settle this: 中文说明代码审查：未发现阻塞性问题。我独立的方案设想（可选启用的工具栏菜单，复用现有附件/引用/技能通道，把
审查期间新增了一个提交（ 测试证据：审查期间 head 移动了一次（即上述测试定位提交），当前提交上有一轮新的 CI 正在运行，因此下方表格是运行中的快照——finalize 任务会在 CI 落定后更新它。macOS/Windows 单测是被本 PR 的 CI profile 跳过，不是因为失败。关于上一个提交的红灯作为背景：之前 沙盒验证建议： — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 4/5 — clean review of a well-designed feature; the only things standing between this and an immediate approval are the CI run still in flight and pointer behaviors that only a real browser can prove. Stepping back: this is what good feature work in this repo looks like. The design doc isn't decoration — its constraints section explains why the obvious simplifications don't work (MCP stops at server level because the Web Shell backend ignores resource references; skill invocations must land at position zero; replacing the whole input would clobber drafts), and the four defects it found along the way are deferred to separate issues instead of ballooning this diff. The riskiest part of the change — lifting the reference sources into a shared module — is a faithful extraction I checked line by line, and it removes the one failure mode that would have mattered most: the My independent proposal for this feature matched the PR's shape, and the implementation goes past it on the fragile details: composer-owner invalidation remounts the menu on session/workspace change, the native picker is re-checked at selection time and discarded on cancel, and hover-opened submenus don't steal the composer's focus. The non-blocking notes from the review stand: the split-pane context-usage control and the restored-draft caret ride along beyond the titled scope (both tested, both mentioned in the PR body), and the Before/After evidence is prose rather than a recording. Two honest reservations on the verdict. First, no unit suite has yet run green on any commit of this PR — the earlier ubuntu red was pre-existing CI-helper noise, but that means this assessment leans on static review plus the author's jsdom suite until the in-flight run lands. Second, the central claims are pointer-and-focus behaviors that jsdom cannot see; the sandboxed Approval is deferred until CI lands green on the reviewed commit; the finalize job will post it once every check settles, and withhold it if anything lands red or the head moves again. 中文说明置信度：4/5 —— 对一个设计良好的功能做了干净的审查；唯一挡在立即批准前面的，是还在运行的 CI，以及只有真实浏览器才能证明的指针行为。 退一步看：这是这个仓库里好的功能开发该有的样子。设计文档不是摆设——它的"约束"一节解释了为什么那些显而易见的简化行不通（MCP 停在 server 级，因为 Web Shell 后端忽略资源级引用；技能调用必须落在输入开头；整体替换输入会冲掉草稿），调研中发现的四个问题也被记录为另行提 issue，而不是撑大这个 diff。改动中风险最高的部分——把引用源抽成共享模块——是我逐行核对过的忠实抽取，它消除了最要紧的一种失败模式： 我独立的方案设想与 PR 的形态一致，而实现在脆弱的细节上超出了它：编辑器归属切换时菜单随会话/工作区变化重新挂载、原生文件选择器在选择时二次校验可用性并在取消时丢弃、hover 打开的子菜单不抢占输入框焦点。审查中的非阻塞意见维持不变：分屏的上下文用量控件和恢复草稿的光标位置超出了标题范围（都有测试、PR 正文也提到了），前后对比证据是文字而非录屏。 对结论有两点诚实的保留。其一，这个 PR 目前还没有任何提交跑绿过单测——之前 ubuntu 的红灯是既有的 CI 助手脚本问题，但这意味着当前判断依赖的是静态审查加作者的 jsdom 套件，要等在跑的这轮 CI 落定。其二，核心主张是指针与焦点行为，jsdom 看不到；上面点名的沙盒 批准将推迟到 CI 在被审提交上全绿之后；finalize 任务会在所有检查落定后代发批准，若有检查变红或 head 再次移动则不发。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
pushed a commit
that referenced
this pull request
Aug 28, 2026
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 28, 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. ✅
chiga0
approved these changes
Aug 28, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
No blocking findings.
What I checked (cross-file):
AddMenu.tsx(562 ln) — focus-preservation viapendingCloseActionRef+onCloseAutoFocus; stale-request cancellation inSearchableProviderSubmenu(abort guard + requestId check in both.thenand.catch); MCP coercion buildsinsertTextfromserverNamecorrectly; directory navigation usesevent.preventDefault()to keep the dropdown open;pick()null-guards on empty insert text and null tag.useAtMentionSources.ts(616 ln) —getCachederror eviction is correct;ANSI_RE/BIDI_CONTROL_RE/AT_REFERENCE_UNSAFE_CHARSapplied on all external-facing text paths;fileReferenceInsertTextanddirectoryInsertTextboth escape + sanitize before emitting.prependSkillInvocation.ts— double-prepend guard uses space-terminatedinsertso/skill-name-otherdoes not false-match/skill-name.useAtMentionMenu.ts— extraction is mechanical; re-exports preserve the public surface; no logic change.ChatEditor.tsx—handleAddMenuPrependSkillCodeMirror path + mobile textarea fallback;handleAddMenuInsertReferenceusescreateComposerTagForItemthenaddComposerTags;AddMenukey resets correctly on session/workspace change.App.tsx—composerToolbarAdditionalActionscorrectly ignored (not appended) whencomposerToolbarActionsis provided, consistent with JSDoc: "appended to the context-sensitive default toolbar".ChatPane.tsx—skillsuseMemo is O(n+m) via Map;handleShowContextUsageguard is correct (shouldBlockComposerSubmit+ session owner capture).
CI (head 4a525455): ubuntu unit tests ✅ · web-shell E2E Smoke ✅ · Desktop Shell ubuntu+windows ✅ · coverage ✅. macOS unit tests SKIPPED · Integration Tests (CLI, No Sandbox) SKIPPED. Nothing here is platform-specific in a way those skips would miss.
Existing reviews: one bot review (LGTM, no findings). Nothing to cross-check.
Reviewed with AI assistance.
Collaborator
|
Released in v0.22.3. |
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
Adds an opt-in
+composer menu that groups message attachments, workspace file references, active extensions, MCP servers, and skills in one pointer-oriented surface. The standalone shell and split panes opt in without changing existing embedders, and split panes keep the same session-scoped controls as the main chat while continuing to omit the pane-owned width toggle.The menu preserves composer focus and insertion order after selections, keeps file search and folder browsing open while interacting, constrains flyouts on narrow screens, and shares mention-source loading with the existing
@flow so both surfaces resolve the same workspace-scoped data. Custom keyboard handling is intentionally not included.Why it's needed
Adding context to a message was spread across less discoverable entry points and several interaction paths lost their panel or composer caret during pointer use. A single opt-in menu makes these actions discoverable while preserving the existing toolbar contract and session/workspace ownership boundaries.
Reviewer Test Plan
How to verify
Enable the additional composer action in the standalone shell or use a split pane, then open
+. Confirm that file attachment/upload, workspace file browsing and search, extensions, MCP, and skills are shown only when their backing capability is available. Select each kind of item and confirm the menu closes, the reference or skill is inserted at the expected composer position, and the visible caret returns to the input. On a narrow viewport, confirm submenus stay within the viewport. In a split pane, confirm context usage is sourced from that pane's session.Evidence (Before & After)
Before: adding message context relied on separate or hidden entry points, nested panels could overflow narrow viewports, and pointer selection could leave focus or insertion before existing tags.
After: the opt-in
+surface exposes the five supported add actions, keeps pointer-driven submenus stable, and restores the composer caret after selection.Tested on
Environment (optional)
Local workspace build and jsdom/Vitest Web Shell suite.
Risk & Scope
Linked Issues
N/A
Pictures
中文说明
此 PR 做了什么
新增一个可选启用的 Composer
+菜单，将消息附件、工作区文件引用、已启用扩展、MCP 服务和技能集中到一个以指针操作为主的入口。独立 Web Shell 与分屏会话显式启用该入口，不改变已有嵌入方；分屏继续与主会话保持相同的会话级控制，仅省略由分屏尺寸管理负责的宽度切换。菜单会在选择后保持正确的 Composer 焦点与插入顺序，文件搜索和文件夹浏览在交互过程中不会消失，小屏下的浮层会限制在视口内；同时复用现有
@流程的引用数据加载，使两个入口读取相同的工作区级数据。本功能有意不加入自定义键盘操作代码。为什么需要
给消息添加上下文原本分散在多个不易发现的入口中，并且若干指针交互会导致面板消失、Composer 光标丢失或新内容插入到已有标签之前。统一的可选菜单提升了可发现性，同时保留现有工具栏契约以及会话、工作区的归属边界。
Reviewer 测试计划
验证方式
在独立 Web Shell 中启用新增 Composer action，或打开分屏会话，然后点击
+。确认添加附件/上传、工作区文件浏览与搜索、扩展、MCP、技能只在对应能力可用时展示。逐项选择并确认菜单关闭、引用或技能插入到预期位置、可见光标回到输入框。缩小视口后确认二级菜单不超出屏幕。在分屏中确认上下文用量来自该分屏自己的会话。前后对比证据
之前：添加消息上下文依赖分散或隐藏的入口，二级面板在窄视口可能溢出，指针选择后可能丢失焦点或插入到已有标签之前。
之后：可选的
+菜单提供五类添加操作，指针交互中的二级面板保持稳定，并在选择后恢复 Composer 可见光标。测试环境
本地执行了 workspace build 以及基于 jsdom/Vitest 的 Web Shell 全量测试。
风险与范围
关联 Issue
N/A