feat(channels): add Web Shell management support for GitHub and GitLab - #8310
Conversation
|
Thanks for the PR! Template looks good ✓ Problem: Real consistency gap — GitHub and GitLab channels are fully functional adapters, but the Web Shell management UI only supports DingTalk/WeCom/Feishu. Users have to hand-edit Direction: Aligned. The Web Shell already manages three channel types through this exact descriptor mechanism; extending it to GitHub/GitLab is the natural next step. CHANGELOG has no direct reference but the area is clearly relevant — channel management is an active surface. Size: Not applicable — no core module paths touched. Approach: Scope feels right. The two new field kinds ( Risk: No elevated risk signals — no high-risk paths matched. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:真实的一致性缺口——GitHub 和 GitLab 频道是功能完整的适配器,但 Web Shell 管理 UI 仅支持 DingTalk/WeCom/Feishu。用户必须手动编辑 方向:对齐。Web Shell 已通过完全相同的描述符机制管理三种频道类型;扩展到 GitHub/GitLab 是自然的下一步。 规模:不适用——未触及核心模块路径。 方案:范围合理。两种新字段类型( 风险:无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code reviewClean. My independent take before reading the diff was: add a Verified against the code:
No blockers, no AGENTS.md violations. Test evidence
All checks completed on The maintainer (@wenshao) independently verified this PR on a fully real stack (PR-source daemon + Vite Web Shell + real Chromium) against fake GitHub/GitLab API servers: channel creation, editing, field validation, and the descriptor-driven senderPolicy all work end-to-end. Both minor findings from that run (wrong 中文说明代码审查干净。我在阅读 diff 之前的独立判断是:为两个插件添加镜像 DingTalk 的 已对照代码验证:
无阻塞问题,无 AGENTS.md 违规。 测试证据所有检查在 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — a genuinely minimal, pattern-following change that closes a real consistency gap; every prior review finding is addressed and the maintainer has verified it end-to-end on a real stack. This is the kind of PR that's easy to review and easy to maintain: it solves a real problem (GitHub/GitLab channels were the odd ones out in the Web Shell management UI), does it by reusing the exact descriptor pattern three other channels already use, and carries tests that pin both halves of the change — the registry catalog shape and the store validation for the two new field kinds. The scope is tight; nothing in the diff goes beyond what the stated goal needs. What convinced me this is ready:
If I had to maintain this in six months, I'd thank the author: the descriptor is the single source of truth, the editor renders generically from it, and adding the next channel type is now a copy-paste-and-adjust exercise. Approving. ✅ 中文说明置信度:5/5 —— 一个真正最小化、遵循既有模式的改动,弥合了真实的一致性缺口;所有先前审查发现均已解决,维护者已在真实栈上端到端验证。 这个 PR 易于审查、易于维护:它解决了一个真实问题(GitHub/GitLab 频道是 Web Shell 管理 UI 中的异类),通过复用其他三种频道已在使用的完全相同的描述符模式来实现,并携带了钉住变更两半的测试——注册目录结构和两种新字段类型的存储验证。范围紧凑,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 / afterℹ️ No screenshot changed against the PR base — but this PR edits 4 render-shaping files:
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 Full-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.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max-preview via Qwen Code /review
| 'github', | ||
| 'gitlab', | ||
| ]); |
There was a problem hiding this comment.
[Suggestion] Adding github/gitlab to this allowlist makes them appear in the Channels Manager "Available platforms" grid, but PLATFORM_MARKS in ChannelsManagerPage.tsx (line 77) only defines dingtalk/wecom/feishu, and the read site at ChannelsManagerPage.tsx:531 renders {PLATFORM_MARKS[platform.type]} with no fallback — so the two new platforms render a blank avatar badge. — Concrete cost: the GitHub/GitLab cards show an empty mark where DingTalk/WeCom/Feishu show their initials; the sibling ChannelEditorDialog.tsx:437 already guards the identical lookup with ?? descriptor.displayName[0].
The fix belongs in ChannelsManagerPage.tsx (not this file) — add github/gitlab entries to PLATFORM_MARKS, or mirror the dialog fallback:
// ChannelsManagerPage.tsx:531
{PLATFORM_MARKS[platform.type] ?? platform.displayName[0]}中文说明
将 github/gitlab 加入此白名单后,它们会出现在频道管理页的「可用平台」网格中,但 ChannelsManagerPage.tsx(第 77 行)的 PLATFORM_MARKS 只定义了 dingtalk/wecom/feishu,且第 531 行的渲染 {PLATFORM_MARKS[platform.type]} 没有兜底——因此两个新平台会渲染出空白的头像徽标。— 具体代价:GitHub/GitLab 卡片显示空白标记,而 DingTalk/WeCom/Feishu 显示各自的首字母;同级的 ChannelEditorDialog.tsx:437 已经用 ?? descriptor.displayName[0] 对相同查找做了兜底。
修复应在 ChannelsManagerPage.tsx(而非本文件)中进行——向 PLATFORM_MARKS 添加 github/gitlab 条目,或参照编辑对话框的兜底写法(见上方代码块)。
— qwen3.8-max-preview via Qwen Code /review
| export function isSupportedChannelType( | ||
| type: unknown, | ||
| ): type is 'dingtalk' | 'wecom' | 'feishu' { | ||
| ): type is 'dingtalk' | 'wecom' | 'feishu' | 'github' | 'gitlab' { |
There was a problem hiding this comment.
[Suggestion] Widening the supported set to include github/gitlab leaves the empty-state copy stale: channels.empty.description (i18n.tsx:2400 en, :4848 zh) still reads "Configure DingTalk, WeCom, or Feishu to receive messages in this workspace." / "配置钉钉、企业微信或飞书,让当前工作区接收消息。", rendered in ChannelsManagerPage.tsx:368. — Concrete cost: a workspace with no configured channels is told only DingTalk/WeCom/Feishu are available, even though the platform grid now also offers GitHub and GitLab.
The fix is in i18n.tsx (not this file) — update both locale strings to include GitHub and GitLab, or phrase the list generically (as channels.availablePlatforms.description already does).
中文说明
将支持集合扩展为包含 github/gitlab 后,空状态文案变得过时:channels.empty.description(i18n.tsx:2400 英文,:4848 中文)仍然写着 "Configure DingTalk, WeCom, or Feishu to receive messages in this workspace." / "配置钉钉、企业微信或飞书,让当前工作区接收消息。",渲染于 ChannelsManagerPage.tsx:368。— 具体代价:未配置任何频道的工作区只会被告知 DingTalk/WeCom/Feishu 可用,尽管平台网格现在也提供 GitHub 和 GitLab。
修复在 i18n.tsx(而非本文件)中——更新两个语言的文案以包含 GitHub 和 GitLab,或将列表表述为通用形式(如同 channels.availablePlatforms.description 已有的写法)。
— qwen3.8-max-preview via Qwen Code /review
| import type { DaemonChannelTypeDescriptor } from '@qwen-code/sdk/daemon'; | ||
|
|
||
| const SUPPORTED_CHANNEL_TYPES = new Set(['dingtalk', 'wecom', 'feishu']); | ||
| const SUPPORTED_CHANNEL_TYPES = new Set([ |
There was a problem hiding this comment.
[Suggestion] Making github/gitlab manageable exposes the channel editor for them, but FIELD_LABEL_KEYS in ChannelEditorDialog.tsx (lines 65–79) only has entries for dingtalk/wecom/feishu. fieldLabel() (lines 185–188) therefore falls back to the plugin's hard-coded English field.label ('Personal Access Token', 'Base URL'). — Concrete cost: in the zh locale (where sibling platforms' field labels are translated, i18n.tsx:4875–4882) a user opening the GitHub/GitLab editor sees English field labels inside an otherwise Chinese dialog; future translations also won't be picked up without FIELD_LABEL_KEYS entries. The en locale is unaffected.
The fix is in ChannelEditorDialog.tsx + i18n.tsx (not this file) — add github/gitlab entries to FIELD_LABEL_KEYS (mapping token/baseUrl) and add the corresponding channels.editor.field.github.* / channels.editor.field.gitlab.* keys to both dictionaries, mirroring dingtalk/wecom/feishu.
中文说明
将 github/gitlab 设为可管理后,频道编辑器会对它们开放,但 ChannelEditorDialog.tsx(第 65–79 行)的 FIELD_LABEL_KEYS 只有 dingtalk/wecom/feishu 的条目。因此 fieldLabel()(第 185–188 行)会回退到插件硬编码的英文 field.label('Personal Access Token'、'Base URL')。— 具体代价:在中文语言环境下(同级平台的字段标签已翻译,i18n.tsx:4875–4882),用户打开 GitHub/GitLab 编辑器时会在 otherwise 中文的对话框中看到英文字段标签;未来新增的翻译在没有 FIELD_LABEL_KEYS 条目的情况下也不会生效。英文语言环境不受影响。
修复在 ChannelEditorDialog.tsx + i18n.tsx(而非本文件)中——向 FIELD_LABEL_KEYS 添加 github/gitlab 条目(映射 token/baseUrl),并向两个语言字典添加对应的 channels.editor.field.github.* / channels.editor.field.gitlab.* 键,参照 dingtalk/wecom/feishu 的模式。
— qwen3.8-max-preview via Qwen Code /review
|
Qwen precheck requires maintainer approval before automated triage/review. Head SHA: Reason:
A maintainer with write access can inspect the PR and manually request a run with |
Review:
|
wenshao
left a comment
There was a problem hiding this comment.
[Critical] Missing groupPolicy in the management descriptor makes channels created from the Web Shell silently dead. buildChannelUpsertRequest writes only descriptor fields + senderPolicy, so a new GitHub/GitLab channel persists { "type": "github", "senderPolicy": "pairing" } with no groupPolicy. ChannelBase.ts:821 passes config.groupPolicy (undefined) to GroupGate, which defaults to 'disabled' (GroupGate.ts:13). Both adapters emit only isGroup: true envelopes (GithubAdapter.ts:1005), so the group gate drops every message. The channel connects, polls, consumes notifications/todos, advances the cursor, and dispatches nothing. On GitLab those todos are marked done and are gone. This blocker was raised by @wenshao in comment 5151503348 and still stands at the reviewed commit — the diff adds no groupPolicy field.
— qwen3.8-max-preview via Qwen Code /review (v0.21.2)
| expect( | ||
| catalog.filter((entry) => entry.manageable).map((entry) => entry.type), | ||
| ).toEqual(['dingtalk', 'wecom', 'feishu']); | ||
| ).toEqual(['dingtalk', 'wecom', 'feishu', 'github', 'gitlab']); |
There was a problem hiding this comment.
[Suggestion] The test asserts the manageable type list but not the field descriptor shape. Reverting the management descriptors from the github/gitlab plugins leaves this test green — the test-efficacy probe confirmed this: both management descriptor hunks survived reversion with no test failure. — Concrete cost: if a future change accidentally sets kind: 'string' on the token field, the Web Shell editor would render it as a plain text input instead of a masked secret field, and no test would catch it.
| ).toEqual(['dingtalk', 'wecom', 'feishu', 'github', 'gitlab']); | |
| ).toEqual(['dingtalk', 'wecom', 'feishu', 'github', 'gitlab']); | |
| expect( | |
| catalog.find((entry) => entry.type === 'github')?.fields, | |
| ).toContainEqual( | |
| expect.objectContaining({ key: 'token', kind: 'secret', required: true }), | |
| ); | |
| expect( | |
| catalog.find((entry) => entry.type === 'gitlab')?.fields, | |
| ).toContainEqual( | |
| expect.objectContaining({ key: 'token', kind: 'secret', required: true }), | |
| ); |
中文说明
该测试仅断言了可管理类型列表,未断言字段描述符的结构。测试效力探针确认:回退 github/gitlab 插件的 management 描述符后,所有测试仍然通过。具体代价:如果未来的变更意外将 token 字段的 kind 设为 'string',Web Shell 编辑器会将其渲染为明文输入框而非掩码密钥字段,且没有测试能捕获此回归。
— qwen3.8-max-preview via Qwen Code /review (v0.21.2)
| github: 'GH', | ||
| gitlab: 'GL', | ||
| }; |
There was a problem hiding this comment.
[Suggestion] PLATFORM_MARKS is duplicated identically in ChannelEditorDialog.tsx:59 and here; this PR extends both copies manually. — Concrete cost: when a sixth channel type is added, the developer must update two identical objects in two files. Missing one causes that view to render displayName[0] (e.g. 'G') instead of the intended abbreviation (e.g. 'GH'). The shared module channel-platform.ts in the same directory already exports shared channel utilities and is the natural home for this constant.
中文说明
PLATFORM_MARKS 在 ChannelEditorDialog.tsx:59 和此处完全重复;本 PR 手动扩展了两份副本。具体代价:添加第六种频道类型时,开发者必须同时更新两个文件中的相同对象。遗漏其中一个会导致该视图渲染 displayName[0](如 'G')而非预期缩写(如 'GH')。同目录下的共享模块 channel-platform.ts 已导出共享频道工具函数,是此常量的自然归属。
— qwen3.8-max-preview via Qwen Code /review (v0.21.2)
…Users for GitHub/GitLab channels
b2eb883 to
ac558ab
Compare
|
Thanks for the thorough review! All items addressed in the latest commits ( Blocking — missing Fixed. Added Major — Fixed. IM channels (dingtalk/wecom/feishu) are unchanged — they keep the existing radio with Minor — Fixed via i18n: GitHub shows "API Base URL", GitLab shows "Instance URL". Minor — test coverage Fixed. Minor — dead descriptor labels Acknowledged — consistent with the existing IM plugins. PLATFORM_MARKS duplication (from /review) Fixed. Extracted to New — field-level descriptions in the editor The
This is generic — any plugin can set 中文翻译感谢详细的 review!所有问题已在最新 commit( 阻塞项 — 缺少 已修复。在 GitHub 和 GitLab 的 management 描述符中添加了 主要问题 — 已修复。GitHub/GitLab 的 IM 频道(dingtalk/wecom/feishu)不受影响——保持现有 radio( 次要 — 已通过 i18n 修复:GitHub 显示 "API Base URL",GitLab 显示 "Instance URL"。 次要 — 测试覆盖 已修复。 次要 — 描述符 label 是死字符串 已知——与现有 IM 插件一致。 PLATFORM_MARKS 重复(来自 /review) 已修复。提取到 新增 — 编辑器字段级描述
这是通用功能——任何插件都可以在字段上设置 |
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。 |
…ith record kind support
|
Update — adapter-specific config fields ( Added the remaining private config fields for each adapter, plus a new GitHub — Optional comma-separated allowlist of notification reasons to process (e.g. GitLab — GitLab's The field-level description (rendered below the last key input) documents all template variables ( New Generic addition to 中文翻译更新 — 适配器私有配置字段( 为每个适配器补充了剩余的私有配置字段,并新增 GitHub — 可选的逗号分隔通知原因白名单(如 GitLab — GitLab 的 字段级描述(渲染在最后一个 key 输入框下方)记录了所有模板变量( 新增 对 |
| management: { | ||
| fields: [ | ||
| { | ||
| key: 'token', | ||
| label: 'Personal Access Token', | ||
| kind: 'secret', | ||
| required: true, | ||
| envResolvable: true, | ||
| }, | ||
| { | ||
| key: 'baseUrl', | ||
| label: 'Base URL', | ||
| kind: 'string', | ||
| envResolvable: true, | ||
| }, | ||
| ], | ||
| }, |
There was a problem hiding this comment.
[Critical] Missing groupPolicy in the management descriptor makes channels created from the Web Shell silently dead. — Failure scenario: a user creates a GitHub channel from the Web Shell UI, enters a PAT and optional base URL. The channel connects and reports healthy. buildChannelUpsertRequest (channel-editor-state.ts:172) writes only descriptor fields plus senderPolicy, so groupPolicy is absent from the persisted config. GroupGate (GroupGate.ts:13) defaults to 'disabled', which drops every isGroup: true envelope (GroupGate.ts:36-38). All four GitHub adapter envelope sites (GithubAdapter.ts:1005,1055,1099,1249) unconditionally set isGroup: true, so every message is silently dropped before dispatch. The channel polls, marks notifications as read (advancing the cursor), and dispatches nothing. On GitLab the same flow marks todos as done without dispatching them — those todos are permanently lost (data loss).
This differs from the DingTalk/WeCom/Feishu channels this PR follows: those adapters emit both isGroup: true and isGroup: false envelopes, so DM messages bypass the group gate. GitHub and GitLab have no DM path — every message goes through the group gate.
| management: { | |
| fields: [ | |
| { | |
| key: 'token', | |
| label: 'Personal Access Token', | |
| kind: 'secret', | |
| required: true, | |
| envResolvable: true, | |
| }, | |
| { | |
| key: 'baseUrl', | |
| label: 'Base URL', | |
| kind: 'string', | |
| envResolvable: true, | |
| }, | |
| ], | |
| }, | |
| management: { | |
| fields: [ | |
| { | |
| key: 'token', | |
| label: 'Personal Access Token', | |
| kind: 'secret', | |
| required: true, | |
| envResolvable: true, | |
| }, | |
| { | |
| key: 'baseUrl', | |
| label: 'Base URL', | |
| kind: 'string', | |
| envResolvable: true, | |
| }, | |
| { | |
| key: 'groupPolicy', | |
| label: 'Group Policy', | |
| kind: 'enum', | |
| required: true, | |
| options: [ | |
| { value: 'open', label: 'Open' }, | |
| { value: 'allowlist', label: 'Allowlist' }, | |
| { value: 'disabled', label: 'Disabled' }, | |
| ], | |
| }, | |
| ], | |
| }, |
— qwen3.7-max via Qwen Code /review
| management: { | ||
| fields: [ | ||
| { | ||
| key: 'token', | ||
| label: 'Personal Access Token', | ||
| kind: 'secret', | ||
| required: true, | ||
| envResolvable: true, | ||
| }, | ||
| { | ||
| key: 'baseUrl', | ||
| label: 'Base URL', | ||
| kind: 'string', | ||
| envResolvable: true, | ||
| }, | ||
| ], | ||
| }, |
There was a problem hiding this comment.
[Critical] Same issue as packages/channels/github/src/index.ts — missing groupPolicy field. — Failure scenario: a user creates a GitLab channel from the Web Shell. Same silent-dead channel behavior: GroupGate defaults to 'disabled', dropping every isGroup: true envelope. GitLab adapter emits isGroup: true envelopes (GitlabAdapter.ts:393). Todos are marked done and lost without dispatch. The GitLab adapter even logs a startup warning (GitlabAdapter.ts:81: groupPolicy is "${cfg.groupPolicy ?? 'disabled'}"; must be "open"…) — but this warning goes to stderr, not the Web Shell UI, so the user has no visibility.
| management: { | |
| fields: [ | |
| { | |
| key: 'token', | |
| label: 'Personal Access Token', | |
| kind: 'secret', | |
| required: true, | |
| envResolvable: true, | |
| }, | |
| { | |
| key: 'baseUrl', | |
| label: 'Base URL', | |
| kind: 'string', | |
| envResolvable: true, | |
| }, | |
| ], | |
| }, | |
| management: { | |
| fields: [ | |
| { | |
| key: 'token', | |
| label: 'Personal Access Token', | |
| kind: 'secret', | |
| required: true, | |
| envResolvable: true, | |
| }, | |
| { | |
| key: 'baseUrl', | |
| label: 'Base URL', | |
| kind: 'string', | |
| envResolvable: true, | |
| }, | |
| { | |
| key: 'groupPolicy', | |
| label: 'Group Policy', | |
| kind: 'enum', | |
| required: true, | |
| options: [ | |
| { value: 'open', label: 'Open' }, | |
| { value: 'allowlist', label: 'Allowlist' }, | |
| { value: 'disabled', label: 'Disabled' }, | |
| ], | |
| }, | |
| ], | |
| }, |
— qwen3.7-max via Qwen Code /review
ReviewReviewed at head OverviewThe PR adds Blocker — daemon-side validation rejects the new field kinds; GitLab save can never succeed
const valid =
((field.kind === 'string' || field.kind === 'secret') && ...) ||
(field.kind === 'boolean' && ...) ||
(field.kind === 'number' && ...) ||
(field.kind === 'enum' && ...);
if (!valid) throw invalidConfig(...);
Chain: Fix: teach Medium
Minor
Positives
Overall: the frontend half is in good shape, but the daemon validation gap is a hard blocker — as shipped, GitLab management is entirely non-functional and GitHub management only works for configs that don't use the new fields. |
The daemon-side store validation only accepted string, secret, boolean, number, and enum field kinds. Channels declaring string-list or record fields in their management descriptor (GitHub allowedUsers/reasonFilter, GitLab action_prompt_template) could never be saved through the Web Shell. Teach assertDescriptorValue the two new kinds and add store-level tests covering both acceptance and rejection paths.
- initialFieldValue: for existing instances with an absent enum field, return empty string instead of the first option. This forces the user to explicitly choose rather than silently writing a new value on save. - isMissingField: guard Object.values().every() with typeof check so hand-edited configs with non-string record values show a validation error instead of throwing TypeError.
…mpty Access section - Extract repeated descriptor.fields.some(f => f.key === 'senderPolicy') into a shared hasDescriptorSenderPolicy() helper (was inline ×4). - Conditionally render the Access section: for descriptor-driven types with a non-pairing policy the section would show only a bare heading with no content beneath it; now it is omitted entirely.
|
Addressed the second-round review findings (head Blocker — daemon store rejects new field kinds ( Medium — silent config rewrite ( Medium — Minor — empty Access section ( Minor — repeated expression ( Not addressed (by design):
PR description updated to reflect the full scope. 中文翻译针对第二轮 review 发现进行修复(head Blocker — daemon 存储拒绝新字段类型( Medium — 静默改写配置( Medium — Minor — 空 Access 区域( Minor — 重复表达式( 未处理(设计如此):
PR 描述已更新以反映完整范围。 |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 4 scenario(s). — Qwen Code · serve A/B |
OverviewExtends Web Shell channel management to GitHub and GitLab by (1) adding The layering is right — descriptor is the single source of truth, the store validates independently of the client, and the legacy hardcoded pairing/open radio group is preserved for DingTalk/WeCom/Feishu behind FindingsMajor — GitLab
|
record field options are UI hints for which rows to render, not a closed set. The GitLab adapter resolves action_prompt_template by plain key lookup and accepts any GitLab todo action_name, but the store rejected keys outside the 9 declared options — silently bricking editing of configs that use server-side actions the descriptor doesn't enumerate.
- Guard the three record JSON.parse sites with an isRecord shape check so a non-object payload can't throw during render or upsert. - Add an explicit descriptor `default` for enum fields and prefer it over option order when seeding a new channel, so an access-control default is declared intent rather than an emergent property of array order. - Declare the GitHub reasonFilter reasons as options and validate string-list input against them in the form, surfacing typos before submission instead of failing at channel start. - Fall back to an uppercased display-name initial (or '?') for the platform mark so an empty name can't render blank.
|
Thanks for the thorough review! Pushed two commits addressing the merge blocker and the minor findings: Merge blocker (record key validation) — fixed in Minor findings — fixed in
Added editor-state tests for the explicit enum default and the string-list validation. The design/consistency suggestions and the render-level test coverage are follow-up material per your verdict — tracked for a separate follow-up rather than this PR. 中文翻译感谢细致的 review!推了两个 commit,处理 merge blocker 和 minor findings: Merge blocker(record key 校验) — 在 Minor findings — 在
为显式 enum 默认值和 string-list 校验补了 editor-state 测试。 设计/一致性建议和渲染层测试覆盖按你的结论属于 follow-up 材料——记录到单独的后续处理,不在本 PR。 |
…n errors Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Thanks for the thorough real-stack verification! Addressing the findings: ① Wrong ② Server-side option-key enforcement — Went with your second option (description fix) rather than extending the check:
The description now states this layering explicitly and drops the "keys restricted to declared options" claim (also corrected the stale "JSON textarea" wording — it's per-key inputs). ③ English enum labels under 中文翻译感谢细致的实机验证!逐项回应: ① ② 服务端 option key 强校验 — 采纳你的第二个建议(改描述),而非扩展校验:
描述现已明确说明这一分层,删除 "keys restricted to declared options" 声称(顺带修正过期的 "JSON textarea" 措辞——实为按 key 分行输入框)。 ③ |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 57 passed · 0 failed · 57 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:57 通过 · 0 失败 · 57 总计 Verification reportPR 8310 — deep verificationVerdict: 中文摘要
Central claim + A/BCentral claim: the daemon channel-settings store accepts the two new descriptor field kinds ( Harness
The two discriminating cells flip REJECT→SUCCESS; the three controls are identical on both arms, which proves the revert is surgical (it changes only the new-kinds acceptance, not the unmanageable path or the pre-existing enum/secret path). Witness captures: Type-boundary matrix (head store only, 7 rows, all matched expectation — witness in
The undeclared-key ACCEPT confirms the PR's stated design (store accepts undeclared Secondary claim — editor record/string-list/enum boundariesHarness
Secondary claim — predicate consistency (reasonFilter)The editor validates string-list tokens against the descriptor's Vacuity check on the new store testReverting only the 8-line Targeted gates (PR's own suites + typecheck)
(Counts measured at the verified head. The PR body's "54" web-shell / "38" store figures are from an earlier commit in the 16-commit branch; the suites are green at the verified head — this is a stale prose count, not a code discrepancy.) Advisory (non-blocking)
No blocking findings. Not covered
MethodologyEnvironment: CI verify container ( Evidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Released in v0.21.4. |













What this PR does
Adds full Web Shell management support for GitHub and GitLab channel instances. Users can now create, edit, and configure GitHub/GitLab channels entirely from the Web Shell UI instead of hand-editing
settings.json.The change spans three layers:
Plugin descriptors — Both the GitHub and GitLab plugins now declare a complete
managementdescriptor exposing all user-facing configuration fields:token(secret),baseUrl(string),groupPolicy(enum),senderPolicy(enum),allowedUsers(string-list), and adapter-specific fields — GitHub addsreasonFilter(string-list), GitLab addsaction_prompt_template(record with declared option keys).Frontend editor — The channel editor dialog renders these fields generically from the descriptor. Two new field kinds (
string-listrendered as comma-separated input,recordrendered as one labelled input per declared option key) are supported alongside the existing string/secret/boolean/number/enum kinds. Field descriptions display below inputs. ThesenderPolicyselector is descriptor-driven for these types (supporting allowlist/pairing/open), while the legacy hardcoded pairing/open radio group remains for DingTalk/WeCom/Feishu. The Access section is hidden entirely when it would render empty.Daemon store validation —
assertDescriptorValuein the channel settings store now validatesstring-list(array of strings) andrecord(string-valued object) so upsert requests containing these field kinds succeed rather than returning HTTP 400. Option-key enforcement is left to the editor (which validatesstring-listtokens) and the adapters at connect time (e.g. GitHub rejects unrecognizedreasonFiltervalues); the store deliberately accepts undeclaredrecordkeys so hand-written and forward-compatible configs survive an editor round-trip.Additional fixes: editing an existing channel whose config lacks a required enum field no longer silently writes the first option (the field shows empty, forcing explicit choice); the record validation path in
isMissingFieldguards against non-string values from hand-edited configs.Why it's needed
GitHub and GitLab channels are functionally complete adapters, but the Web Shell management UI only supported DingTalk, WeCom, and Feishu. Users had to configure GitHub/GitLab channels manually in
settings.json, which is inconsistent with the other managed channels and error-prone — misconfiguration (e.g. missinggroupPolicy) silently produces a dead channel that consumes notifications without dispatching them.Reviewer Test Plan
How to verify
cd packages/web-shell && npx vitest run client/components/channels/— 54 tests covering draft creation, validation, upsert shape, enum defaults, string-list/record round-trip, and descriptor-driven senderPolicy.cd packages/cli && npx vitest run src/serve/channel-settings-store.test.ts— 38 tests including acceptance and rejection ofstring-listandrecordfield kinds.cd packages/cli && npx vitest run src/commands/channel/channel-registry.test.ts— confirms github and gitlab are manageable with correct field descriptor shapes.packages/channels/github/src/index.tsandpackages/channels/gitlab/src/index.tsmatch what the adapters actually consume.Evidence (Before & After)
N/A (management UI rendering depends on live daemon integration; logic verified via unit tests)
Tested on
Environment (optional)
Unit tests only (
vitest).Risk & Scope
string-list,record) extend the descriptor type system — future plugins can use them, but the daemon store and frontend editor must stay in sync on supported kinds.pairingflow posting public comments on GitHub/GitLab (functional but noisy — worth revisiting separately).settings.jsonconfigs are unaffected; the editor now surfaces fields that were previously only configurable by hand.Linked Issues
Ref: #7862 (comment)
中文说明
本 PR 做了什么
为 GitHub 和 GitLab 频道实例添加完整的 Web Shell 管理支持。用户现在可以完全通过 Web Shell UI 创建、编辑和配置 GitHub/GitLab 频道,无需手动编辑
settings.json。变更跨越三层:
插件描述符 — GitHub 和 GitLab 插件均声明了完整的
management描述符,暴露所有面向用户的配置字段:token(secret)、baseUrl(string)、groupPolicy(enum)、senderPolicy(enum)、allowedUsers(string-list),以及适配器特有字段——GitHub 增加reasonFilter(string-list),GitLab 增加action_prompt_template(record,带声明的 option keys)。前端编辑器 — 频道编辑对话框从描述符通用渲染这些字段。支持两种新字段类型(
string-list渲染为逗号分隔输入框,record渲染为每个声明的 option key 一个带标签输入框),与现有的 string/secret/boolean/number/enum 类型并列。字段描述显示在输入框下方。senderPolicy选择器对这些类型由描述符驱动(支持 allowlist/pairing/open),而 DingTalk/WeCom/Feishu 保留原有的硬编码 pairing/open 单选组。当 Access 区域无内容时整体隐藏。守护进程存储验证 — 频道设置存储中的
assertDescriptorValue现在验证string-list(字符串数组)和record(字符串值对象),使包含这些字段类型的 upsert 请求成功而非返回 HTTP 400。option key 的约束交由编辑器(校验string-list取值)和适配器在 connect 阶段(如 GitHub 拒绝未识别的reasonFilter值)负责;存储层有意接受未声明的recordkey,使手写配置和前向兼容配置能在编辑器往返后保留。额外修复:编辑配置中缺少必填 enum 字段的现有频道时,不再静默写入第一个选项(字段显示为空,强制用户显式选择);
isMissingField的 record 验证路径对手动编辑配置中的非字符串值做了防护。为什么需要
GitHub 和 GitLab 频道是功能完整的适配器,但 Web Shell 管理 UI 仅支持 DingTalk、WeCom 和 Feishu。用户必须在
settings.json中手动配置 GitHub/GitLab 频道,与其他已托管频道不一致且容易出错——配置错误(如缺少groupPolicy)会静默产生一个消耗通知但不派发消息的死频道。审阅者测试计划
如何验证
cd packages/web-shell && npx vitest run client/components/channels/— 54 个测试覆盖草稿创建、验证、upsert 结构、enum 默认值、string-list/record 往返、描述符驱动的 senderPolicy。cd packages/cli && npx vitest run src/serve/channel-settings-store.test.ts— 38 个测试,包含string-list和record字段类型的接受和拒绝。cd packages/cli && npx vitest run src/commands/channel/channel-registry.test.ts— 确认 github 和 gitlab 标记为可管理且字段描述符结构正确。packages/channels/github/src/index.ts和packages/channels/gitlab/src/index.ts中的描述符结构与适配器实际消费的配置一致。证据(前后对比)
N/A(管理 UI 渲染依赖 daemon 集成;逻辑通过单元测试验证)
测试环境
环境(可选)
仅单元测试(
vitest)。风险与范围
string-list、record)扩展了描述符类型系统——未来插件可以使用它们,但 daemon 存储和前端编辑器必须保持对支持类型的同步。pairing流程在 GitHub/GitLab 上发布公开评论(功能正常但噪音大——值得单独讨论)。settings.json配置不受影响;编辑器现在暴露了以前只能手动配置的字段。关联 Issue
参考:#7862 (comment)