feat(web-shell): manage Channel pairing requests - #7909
Conversation
E2E test reportBrowser flow
Scope and failure coverage
Visual coverage
Supporting validation
|
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: This is a feature addition, not a bug fix — the problem is well-established through the linked issue #7209 and the predecessor PR #7893. Pairing approval currently requires the CLI; this closes that loop in the Web Shell. Observed need, not theoretical. Direction: Aligned. The daemon already exposes Size: Not applicable — all changes are in Approach: The scope feels right. The PR is limited to the two operations the daemon actually supports (list and approve), and explicitly defers reject/delete since no daemon route exists. The component is self-contained, follows existing patterns (CSS modules, i18n, Alert/Button/Spinner primitives), and the stale-response handling (channel switch mid-flight) is a real concern that's addressed cleanly. No unrelated changes or drive-by refactors in the diff. Risk: No elevated risk signals — no high-risk paths matched. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:这是一个功能新增,不是 bug 修复——需求通过关联的 issue #7209 和前序 PR #7893 已经明确。配对审批目前只能通过 CLI 完成,本 PR 在 Web Shell 中补齐这个闭环。是已观测到的需求,不是理论性加固。 方向:对齐。daemon 已经通过 规模:不适用——所有改动都在 方案:范围合理。PR 严格限制在 daemon 实际支持的两个操作(查询和批准),并明确推迟了拒绝/删除(因为 daemon 没有对应路由)。组件自包含,遵循现有模式(CSS modules、i18n、Alert/Button/Spinner 基础组件),过期响应处理(切换频道时的竞态)是一个真实 concern,处理得干净。diff 中没有无关改动或顺手重构。 风险:无升级风险信号——未匹配高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewClean implementation. The new The stale-response handling is the trickiest part and it's done correctly: the The integration into Test coverage is thorough: the new component test covers load, approve, approval failure with retry, load failure with retry, stale approval after channel switch, and stale list during loading. The existing dialog and page tests are updated for the new props. E2e coverage adds the full approve flow to the channels spec and includes the pairing panel in the visual screenshot scenario. The mock daemon correctly handles both GET (list) and POST (approve) pairing routes with proper 404 for unknown codes. No critical blockers or convention violations found. CI Test EvidenceFinal CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 The main unit test suite and the web-shell visual capture job are still running on this commit. No failures so far. The visual capture job will post dark/light before-and-after composites inline on this PR once it completes. Not verified: live behavior in a real browser — the e2e Playwright specs in this PR ( 中文说明代码审查实现干净。新的 过期响应处理是最复杂的部分,实现正确: 与 测试覆盖全面:新组件测试覆盖了加载、审批、审批失败重试、加载失败重试、切换频道后的过期审批、以及加载中的过期列表。现有的对话框和页面测试已更新新 prop。E2e 覆盖在频道测试中增加了完整的审批流程,并在视觉截图场景中包含了配对面板。mock daemon 正确处理了 GET(列表)和 POST(审批)配对路由,对未知 code 返回 404。 未发现关键阻塞项或规范违反。 CI 测试证据主单元测试套件和 Web Shell 视觉捕获任务仍在该 commit 上运行。目前无失败。视觉捕获任务完成后会在本 PR 中发布暗色/亮色主题的前后对比图。 未验证:真实浏览器中的实际行为——本 PR 中的 e2e Playwright 测试( — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean across every stage; would merge without hesitation. This is a well-executed feature PR. The scope is tight — list and approve, the two operations the daemon actually supports — and the implementation follows existing Web Shell patterns throughout. The stale-response handling (the hardest part of this kind of UI) is correct and well-tested: channel switches mid-flight are guarded both in the effect cleanup and in the approval callback, and state is cleared so old data never flashes. The test suite covers the meaningful edge cases (approval failure, load failure, stale responses) rather than just the happy path, and the e2e spec exercises the full approve flow against the mock daemon. Every change in the diff serves the stated goal. No drive-by refactors, no unrelated edits, no over-abstraction. The component reuses existing utilities and primitives rather than introducing parallel ones. If I had to maintain this in six months, I'd thank the author. Approval deferred until CI lands green on 中文说明置信度:5/5 —— 每个阶段都很干净,毫不犹豫可以合并。 这是一个执行良好的功能 PR。范围紧凑——查询和批准,daemon 实际支持的两个操作——实现全程遵循 Web Shell 的现有模式。过期响应处理(这类 UI 最难的部分)正确且测试充分:切换频道时的竞态在 effect 清理和审批回调中都有防护,状态被清空所以旧数据不会闪现。测试套件覆盖了有意义的边界情况(审批失败、加载失败、过期响应),而不仅仅是 happy path;e2e 测试针对 mock daemon 演练了完整的审批流程。 diff 中的每个改动都服务于既定目标。没有顺手重构、无关编辑或过度抽象。组件复用现有工具函数和基础组件,而非引入平行的实现。如果六个月后我来维护这段代码,我会感谢作者。 审批推迟到 CI 在 — Qwen Code · qwen3.8-max-preview Reviewed at |
🖼️ 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 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM!✅
— qwen3.7-max via Qwen Code /review
|
@qewn-code /takeover |
Local verification against a real daemon (no
|
| Layer | What actually ran |
|---|---|
| Daemon | qwen serve --port 4180 --workspace ws-a --workspace ws-b, isolated QWEN_HOME |
| Routes | the real GET /workspaces/:ws/channels/:name/pairing-requests and POST …/approve, served by ChannelManagementService + PairingStore on disk |
| Pending requests | produced by production code — new PairingStore(name, cwd) + SenderGate.check(senderId, senderName), i.e. the exact calls ChannelBase makes for an inbound message |
| Channel config | created through this editor; asserted against the real ws-a/.qwen/settings.json the daemon wrote |
| Client | this PR's packages/web-shell/client/** on Vite, proxying to the daemon; @qwen-code/webui and @qwen-code/sdk aliased to worktree source |
| Browser | Playwright Chromium. No response stubbing anywhere, except one deliberate 6 s delay used for the race test |
Two arms on the same daemon: merge-base b3873571 (BEFORE) and PR head 39f05662 (AFTER).
Your reviewer test plan, item by item
| # | Check | Result |
|---|---|---|
| 1 | Pairing Channel shows Pending requests under the policy cards | ✅ verified on all three manageable types — DingTalk release-bot, WeCom support-bot, Feishu release bot+2 |
| 2 | Displayed identity and code match what the user was given | ✅ codes rendered in the UI (6TP8WFZ5, PGCDHPKL, …) are byte-identical to what SenderGate.check() returned to the "sender" |
| 3 | Approve → row disappears, success message, user can now use the Channel | ✅ and I checked the effect, not just the UI: after approving, the real allowlist file contains the sender, SenderGate.isAllowed() flips to true, and re-running the inbound path returns {allowed: true} |
| 4 | Daemon unavailable / failed approval → request stays visible with a retry path | ✅ both. Killed the daemon process → error + Try again; restarted it → Try again recovers. Separately, approving a code that had been consumed elsewhere returned a real 404 and the row stayed put |
| 5 | Switching Channel or workspace mid-flight never shows stale data | ✅ for Channel switch, with a positive control: release-bot's code is provably displayable on its own, then leaving release-bot while its list request was still in flight (6 s injected latency, real daemon still answering) landed on support-bot showing only support-bot's code. Workspace switch — see Notes |
| 6 | New config, or Open → Pairing, asks you to save first | ✅ both branches: a fresh config, and a saved Open Channel switched back to Pairing in the draft |
Also checked: instance names with URL-significant characters (release bot+2) round-trip correctly (200, correct list); a blank senderName falls back to the sender ID and suppresses the duplicate ID line; light and dark themes both render.
Repo gates on the PR head: vitest 29/29 in client/components/channels, eslint --max-warnings 0 clean, tsc --noEmit clean for the changed files (the two errors I see are sandbox dist drift and are byte-identical on the merge base). CI on the PR: 14 pass, 0 fail.
Screenshots
Before / after — same daemon, same saved Channel
Happy path
Failure paths — real process kill, real 404
Scope isolation and theming
Findings — none blocking
1. Cancel does not undo an approval, and there is no revoke. Approve commits to the daemon immediately, so the editor's Save/Cancel framing is misleading for this one control. Measured: approve a request, then click Cancel → the sender is still on the allowlist (allowlist: ["user-42","user-55"]). Because this is an access grant and the reject/delete route does not exist yet, I would either move the panel out of the Save/Cancel form or make the immediacy explicit in the copy.
2. Raw transport text reaches the operator on an outage. With the daemon down the alert body is GET /workspaces/:workspace/channels/:name/pairing-requests: HTTP 500 — the route template and a status code (visible in the failure composite above). The 404 path reads nicely ("Pairing request was not found or has expired.") only because the daemon sends a JSON error. A fallback in extractErrorDetail's consumer when the detail looks like a route/status string would keep this actionable.
3. A stale row survives its 404 and can only fail again. After a code is approved elsewhere, the correct message appears but the phantom row stays with an enabled Approve. I clicked it twice: two POST …/approve → 404, row still present, list unchanged until Try again. Refreshing the list on channel_pairing_request_not_found would self-heal here.
4. Every row's button has the same accessible name. The aria snapshot of a three-request list is button "Approve" ×3 with no aria-label, so a screen-reader user cannot tell the rows apart. Suggest aria-label={t('…approveFor', { sender, code })}.
5. zh wording: 允许 vs 批准. In the same panel the description says "批准前,请核对…" and the policy card says "经您批准后…", but the button is "允许". The rest of the codebase uses 批准 for approval. Suggest 批准 for channels.editor.pairing.approve.
Notes (context, not defects)
- Workspace switching is not reachable from this page today.
main.tsxmounts<DaemonWorkspaceProvider>with noworkspaceCwd, soChannelsManagerPageis always bound to the daemon's primary workspace — with two workspaces registered, the page stayed onws-a. Your guard inChannelsManagerPage(editor.workspaceCwd === workspace.workspaceCwd) already unmounts the panel on a workspace change, so this is ready for when fix(web-shell): report intended workspace to host when starting a new chat #7910 lands; I just could not exercise it end to end. PairingStorecaps pending requests at 3. A fourth waiting user silently getspairingCode: nulland the panel shows nothing about it — backend behaviour, not this PR, but it does mean prompt approval matters.- The list request fires twice per mount in my run; that is React
StrictModein dev, not a double fetch.
Nice, tightly scoped stage — the loop genuinely closes against a real daemon. Happy to see it merged; the five points above are fine as follow-ups.
中文说明
基于真实 daemon 的本地验证(不使用 mockDaemon)
为了给合并提供不依赖手写 mockDaemon 的证据,我搭了一套请求路径上每一层都是真实的环境,并用 Chromium 驱动它。
| 层 | 实际运行的东西 |
|---|---|
| Daemon | qwen serve --port 4180 --workspace ws-a --workspace ws-b,隔离的 QWEN_HOME |
| 路由 | 真实的 GET /workspaces/:ws/channels/:name/pairing-requests 与 POST …/approve,由 ChannelManagementService + 磁盘上的 PairingStore 提供 |
| 待处理请求 | 由生产代码产生 —— new PairingStore(name, cwd) + SenderGate.check(senderId, senderName),即 ChannelBase 处理入站消息时的同一条调用链 |
| 频道配置 | 通过本编辑器创建,并与 daemon 真实写入的 ws-a/.qwen/settings.json 做断言 |
| 客户端 | 本 PR 的 packages/web-shell/client/** 跑在 Vite 上并代理到 daemon;@qwen-code/webui 与 @qwen-code/sdk 别名指向 worktree 源码 |
| 浏览器 | Playwright Chromium。除了竞态测试中刻意注入的 6 秒延迟外,没有任何响应被 stub |
同一个 daemon 下跑两组:merge-base b3873571(修改前)与 PR head 39f05662(修改后)。
按你的 Reviewer 测试计划逐条核对
| # | 检查项 | 结果 |
|---|---|---|
| 1 | 配对模式频道在准入策略下方显示"待处理的配对请求" | ✅ 三种可管理类型全部验证 —— 钉钉 release-bot、企业微信 support-bot、飞书 release bot+2 |
| 2 | 显示的用户身份与配对码和用户拿到的一致 | ✅ 界面上渲染的配对码(6TP8WFZ5、PGCDHPKL 等)与 SenderGate.check() 返回给"发送者"的完全一致 |
| 3 | 批准后请求消失、显示成功提示、用户可以使用频道 | ✅ 而且我校验的是实际效果而不只是界面:批准后真实的 allowlist 文件包含该用户,SenderGate.isAllowed() 变为 true,重新走一遍入站路径返回 {allowed: true} |
| 4 | daemon 不可用 / 批准失败时请求仍保留并提供重试 | ✅ 两种都验证了。杀掉 daemon 进程 → 报错 + Try again;重启后点 Try again 即恢复。另外,批准一个已在别处被消费掉的配对码会收到真实的 404,该行仍然保留 |
| 5 | 加载或批准过程中切换频道/工作区不会出现过期数据 | ✅ 频道切换已验证,并带正向对照:先证明 release-bot 的配对码本身是能显示出来的,然后在它的列表请求仍在飞行途中(注入 6 秒延迟,daemon 仍真实响应)离开它,最终 support-bot 只显示自己的配对码。工作区切换见"补充说明" |
| 6 | 新建配置、或从开放模式切到配对模式时提示先保存 | ✅ 两个分支都验证:全新配置,以及一个已保存为开放模式、在草稿里切回配对模式的频道 |
另外还验证了:包含 URL 敏感字符的实例名(release bot+2)能正确往返(200,列表正确);senderName 为空白时回退到 sender ID 并且不再重复显示 ID 行;明暗两套主题都能正常渲染。
PR head 上的仓库门禁:client/components/channels 下 vitest 29/29 通过,eslint --max-warnings 0 干净,改动文件的 tsc --noEmit 干净(我这里看到的两个报错是沙箱 dist 漂移,在 merge base 上一模一样)。PR 的 CI:14 通过,0 失败。
截图
修改前 / 修改后 —— 同一个 daemon、同一个已保存的频道
主流程
失败路径 —— 真实杀进程、真实 404
作用域隔离与主题
发现的问题 —— 都不阻塞合并
1. Cancel 无法撤销已完成的批准,而且没有撤回入口。 批准是立即提交到 daemon 的,因此编辑器的"保存/取消"语义对这个控件来说会产生误导。实测:批准一条请求后点 Cancel,该用户仍在 allowlist 中(allowlist: ["user-42","user-55"])。考虑到这是一次访问授权、而且 daemon 目前没有拒绝/删除路由,我建议要么把这个面板移出"保存/取消"表单,要么在文案上明确说明该操作立即生效。
2. daemon 掉线时把底层传输错误原样抛给了运维同学。 daemon 关闭时提示内容是 GET /workspaces/:workspace/channels/:name/pairing-requests: HTTP 500 —— 路由模板加状态码(见上面失败路径的截图)。404 那条之所以好看("Pairing request was not found or has expired."),是因为 daemon 返回了 JSON error。建议在 extractErrorDetail 的调用方加一层兜底:当 detail 看起来像"路由 + 状态码"时改用可操作的文案。
3. 404 之后残留行会一直留在列表里,而且只会继续失败。 配对码在别处被批准后,提示文案是对的,但那一行仍然保留且 Approve 仍可点击。我连点了两次:两次 POST …/approve → 404,行还在,列表在点 Try again 之前都不会变。在收到 channel_pairing_request_not_found 时顺带刷新一次列表就能自愈。
4. 每一行的按钮无障碍名称完全相同。 三条请求的列表,aria 快照是三个 button "Approve",没有 aria-label,读屏用户无法区分。建议加 aria-label={t('…approveFor', { sender, code })}。
5. 中文用词:允许 vs 批准。 同一个面板里,描述写的是"批准前,请核对…",策略卡片写的是"经您批准后…",但按钮是"允许"。仓库里其他地方统一用"批准"。建议把 channels.editor.pairing.approve 改成"批准"。
补充说明(属于背景,不是缺陷)
- 目前这个页面还切不了工作区。
main.tsx里<DaemonWorkspaceProvider>没有传workspaceCwd,所以ChannelsManagerPage永远绑定在 daemon 的主工作区上 —— 我注册了两个工作区,页面始终停在ws-a。你在ChannelsManagerPage里的守卫(editor.workspaceCwd === workspace.workspaceCwd)已经会在工作区变化时卸载面板,所以等 fix(web-shell): report intended workspace to host when starting a new chat #7910 合入后这条就能生效,我这边只是没法端到端跑出来。 PairingStore的待处理上限是 3。第四个等待中的用户会静默地拿到pairingCode: null,界面上也没有任何提示 —— 这是后端行为、不属于本 PR,但确实说明及时批准很重要。- 我这边每次挂载会发两次列表请求;这是开发模式下 React
StrictMode的行为,不是重复请求的 bug。
这个阶段拆得很干净,闭环在真实 daemon 上确实跑通了。我这边支持合并,上面五点作为后续跟进即可。
Verified locally with Claude Code (Claude Opus 5, 1M context).
|
@qwen-code /takeover |
|
Released in v0.21.1. |








What this PR does
Adds workspace-scoped pairing request management to the existing Channel editor. When a saved Channel uses Pairing access, pending users appear directly below the access policy with their identity, request time, and pairing code. Operators can refresh the list and approve a request without leaving the editor.
The list is loaded through the selected workspace's daemon client. Pending data is cleared while its scope changes, and late list or approval responses are ignored so one Channel's requests cannot appear under another. New or newly switched configurations explain that Pairing mode must be saved before requests can be managed.
Why it's needed
The previous Channel configuration stage allowed users to select Pairing access, but approval still required the CLI. This stage closes the supported daemon loop in Web Shell while keeping the PR limited to the two operations that currently exist: list and approve.
Reviewer Test Plan
How to verify
Evidence (Before & After)
Before: Pairing access could be selected, but the editor had no way to see or approve pending users. After: the Pairing card is followed by a responsive pending-request list with refresh, approval, success, empty, and error states. The Web Shell visual preview job will post dark and light before/after composites inline on this PR.
Tested on
Environment (optional)
Node.js 22 workspace install, Web Shell Vite development server, and Playwright Chromium.
Risk & Scope
Linked Issues
Part of #7209. Follows #7893.
中文说明
本 PR 做了什么
在现有频道编辑器中增加工作区级配对请求管理。已保存的频道使用配对模式时,待处理用户会直接显示在准入策略下方,并展示用户身份、请求时间和配对码。管理员可以在编辑器中刷新列表并批准请求。
列表通过当前工作区的 daemon 客户端加载。作用域变化时会立即清空旧的待处理数据,并忽略过期的列表或批准响应,避免把一个频道的请求显示到另一个频道。对于新建配置或刚切换到配对模式的配置,界面会说明需要先保存,之后才能管理请求。
为什么需要
上一阶段的频道配置已经允许用户选择配对模式,但批准仍必须通过 CLI 完成。本阶段在 Web Shell 中补齐 daemon 当前支持的闭环,同时把 PR 严格限制在现有的两个操作:查询和批准。
Reviewer 测试计划
如何验证
证据(修改前后)
修改前:可以选择配对模式,但编辑器无法查看或批准待处理用户。修改后:配对卡片下方增加响应式待处理请求列表,覆盖刷新、批准、成功、空状态和错误状态。Web Shell 视觉预览任务会在本 PR 中自动发布暗色和亮色主题的前后对比图。
测试平台
环境(可选)
Node.js 22 workspace 安装、Web Shell Vite 开发服务器和 Playwright Chromium。
风险与范围
关联 Issue
属于 #7209 的一部分,承接 #7893。