From 9c1c36fbbbb1dc5812fa4bd4ab4b734eb9b91ff8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E5=86=A0=E8=BE=B0?= Date: Mon, 18 May 2026 17:49:22 +0800 Subject: [PATCH 1/5] feat(channels): surface Lark/Feishu and DingTalk in the Settings UI (#2048) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both channel runtimes already exist in the codebase: - src/openhuman/channels/providers/lark.rs — Lark / Feishu Stream WebSocket implementation - src/openhuman/channels/providers/dingtalk.rs — DingTalk Stream Mode WebSocket implementation - LarkConfig + DingTalkConfig in config/schema/channels.rs - Both wired in channels::commands and channels::runtime::startup The Settings UI couldn't reach them because all_channel_definitions() in src/openhuman/channels/controllers/definitions.rs only emitted entries for telegram / discord / web / imessage. Users had to hand-edit config.toml — the feature was effectively invisible to anyone not reading source. Both platforms together cover hundreds of millions of Chinese enterprise users. Backend changes --------------- src/openhuman/channels/controllers/definitions.rs: * lark_definition() — id 'lark', icon 'lark', ApiKey auth mode with seven fields matching LarkConfig 1:1: - app_id (string, required) - app_secret (secret, required) - encrypt_key (secret, optional) - verification_token (secret, optional) - use_feishu (boolean, optional) - receive_mode (string, optional — 'websocket' or 'webhook') - allowed_users (string, optional) * dingtalk_definition() — id 'dingtalk', icon 'dingtalk', ApiKey auth mode with three fields matching DingTalkConfig: - client_id (string, required) - client_secret (secret, required) - allowed_users (string, optional) * Both registered in all_channel_definitions() after imessage. Frontend changes ---------------- * app/src/types/channels.ts — ChannelType union extended with 'lark' and 'dingtalk' so all downstream Redux slices, routing, and ChannelSetupModal can type-check the new ids. * app/src/components/settings/panels/MessagingPanel.tsx — CHANNEL_ICONS gains entries for lark (🪶) and dingtalk (🔔). Lark and Feishu share the same icon since they share the same backend. * app/src/lib/channels/definitions.ts — FALLBACK_DEFINITIONS gains full mirror entries for lark and dingtalk. The hook hits the backend RPC first and only falls back when the core sidecar is unreachable, but the fallback list must stay in lockstep with Rust definitions.rs or the schema-driven form renderer in ChannelSetupModal will produce a blank form when the user is offline mid-setup. Tests ----- src/openhuman/channels/controllers/definitions_tests.rs gains six cases: * lark_definition_is_registered — id + display_name + icon pinned. * lark_uses_api_key_auth_with_app_id_and_secret_required — covers the two required fields by type + required-flag, and exhaustively verifies all five optional fields exist by key. The config-schema-rename guard rail: if anyone renames a LarkConfig field, this test fails at compile/test time rather than the UI silently breaking. * lark_validate_credentials_rejects_missing_app_secret — failure-path case (the testing-strategy.md requirement). * dingtalk_definition_is_registered — mirror for DingTalk. * dingtalk_requires_client_id_and_client_secret — required + types pinned. * dingtalk_validate_credentials_rejects_missing_client_secret — failure-path case. * all_definitions_include_lark_and_dingtalk — top-level registry smoke check. Existing all_definitions_have_unique_ids / every_definition_has_at_least_one_auth_mode / required_fields_have_non_empty_key_and_label tests automatically cover the new entries. Local checks ------------ * cargo fmt --manifest-path Cargo.toml -- --check: clean * pnpm prettier --check on the three changed TS files: clean * cargo check on the changed Rust files compiles (full crate check on Apple Silicon macOS still trips the pre-existing whisper-rs / ggml-cpu issue noted on #2045 / #2053 / #2080 — CI Linux runs the full suite). The new definitions only consume types that already exist in this module. Out of scope for this PR ------------------------ * ChannelSetupModal form rendering is already schema-driven — it iterates field_type 'string' | 'secret' | 'boolean' generically, so the new Lark/DingTalk fields render automatically. No modal changes required. * OAuth flow for Lark — using ApiKey credentials matches DingTalk's Stream Mode and avoids inventing a new auth_mode for one channel. * Polish on receive_mode — currently a plain string field documented as 'websocket' / 'webhook'. A future PR can promote it to an enum select once we have UX hooks for that field type. --- .../settings/panels/MessagingPanel.tsx | 10 +- app/src/lib/channels/definitions.ts | 107 ++++++++++++++ app/src/types/channels.ts | 2 +- .../channels/controllers/definitions.rs | 120 ++++++++++++++++ .../channels/controllers/definitions_tests.rs | 132 ++++++++++++++++++ 5 files changed, 369 insertions(+), 2 deletions(-) diff --git a/app/src/components/settings/panels/MessagingPanel.tsx b/app/src/components/settings/panels/MessagingPanel.tsx index 68e53bb514..442b63a90d 100644 --- a/app/src/components/settings/panels/MessagingPanel.tsx +++ b/app/src/components/settings/panels/MessagingPanel.tsx @@ -15,7 +15,15 @@ import ChannelSetupModal from '../../channels/ChannelSetupModal'; import SettingsHeader from '../components/SettingsHeader'; import { useSettingsNavigation } from '../hooks/useSettingsNavigation'; -const CHANNEL_ICONS: Record = { telegram: '✈️', discord: '🎮', web: '🌐' }; +const CHANNEL_ICONS: Record = { + telegram: '✈️', + discord: '🎮', + web: '🌐', + // Lark (国际版) / Feishu (中国版) — same backend, single icon. + lark: '🪶', + // DingTalk (钉钉). + dingtalk: '🔔', +}; function statusDot(status: ChannelConnectionStatus): string { switch (status) { diff --git a/app/src/lib/channels/definitions.ts b/app/src/lib/channels/definitions.ts index 074f70e55c..114cdcc533 100644 --- a/app/src/lib/channels/definitions.ts +++ b/app/src/lib/channels/definitions.ts @@ -118,4 +118,111 @@ export const FALLBACK_DEFINITIONS: ChannelDefinition[] = [ ], capabilities: ['send_text', 'send_rich_text', 'receive_text'], }, + // Lark / Feishu — fields must stay aligned with `LarkConfig` in + // `src/openhuman/config/schema/channels.rs` and `lark_definition()` in + // `src/openhuman/channels/controllers/definitions.rs`. See #2048. + { + id: 'lark', + display_name: 'Lark / Feishu', + description: 'Send and receive via Lark (international) or Feishu (中国版).', + icon: 'lark', + auth_modes: [ + { + mode: 'api_key', + description: 'Provide your Lark/Feishu app credentials from the Open Platform.', + fields: [ + { + key: 'app_id', + label: 'App ID', + field_type: 'string', + required: true, + placeholder: 'cli_xxxxxxxxxxxx', + }, + { + key: 'app_secret', + label: 'App Secret', + field_type: 'secret', + required: true, + placeholder: 'Your Lark app secret', + }, + { + key: 'encrypt_key', + label: 'Encrypt Key', + field_type: 'secret', + required: false, + placeholder: 'Optional — required only if you enabled message encryption', + }, + { + key: 'verification_token', + label: 'Verification Token', + field_type: 'secret', + required: false, + placeholder: 'Optional — used for HTTP webhook verification', + }, + { + key: 'use_feishu', + label: 'Use Feishu (中国版)', + field_type: 'boolean', + required: false, + placeholder: 'On = open.feishu.cn (China); off = open.larksuite.com', + }, + { + key: 'receive_mode', + label: 'Receive Mode', + field_type: 'string', + required: false, + placeholder: 'websocket (default) or webhook', + }, + { + key: 'allowed_users', + label: 'Allowed Users', + field_type: 'string', + required: false, + placeholder: 'Comma-separated open_id / union_id; leave empty to allow any', + }, + ], + auth_action: undefined, + }, + ], + capabilities: ['send_text', 'receive_text', 'threaded_replies'], + }, + // DingTalk (钉钉) — fields must stay aligned with `DingTalkConfig` in + // `src/openhuman/config/schema/channels.rs`. See #2048. + { + id: 'dingtalk', + display_name: 'DingTalk (钉钉)', + description: 'Send and receive via DingTalk Stream Mode (钉钉).', + icon: 'dingtalk', + auth_modes: [ + { + mode: 'api_key', + description: 'Provide your DingTalk app credentials from the developer console.', + fields: [ + { + key: 'client_id', + label: 'Client ID (AppKey)', + field_type: 'string', + required: true, + placeholder: 'ding_xxxxxxxxxxxx', + }, + { + key: 'client_secret', + label: 'Client Secret (AppSecret)', + field_type: 'secret', + required: true, + placeholder: 'Your DingTalk app secret', + }, + { + key: 'allowed_users', + label: 'Allowed Users', + field_type: 'string', + required: false, + placeholder: 'Comma-separated DingTalk userIds; leave empty to allow any', + }, + ], + auth_action: undefined, + }, + ], + capabilities: ['send_text', 'receive_text'], + }, ]; diff --git a/app/src/types/channels.ts b/app/src/types/channels.ts index 715156c27d..353c0be728 100644 --- a/app/src/types/channels.ts +++ b/app/src/types/channels.ts @@ -1,4 +1,4 @@ -export type ChannelType = 'telegram' | 'discord' | 'web'; +export type ChannelType = 'telegram' | 'discord' | 'web' | 'lark' | 'dingtalk'; export type ChannelAuthMode = 'managed_dm' | 'oauth' | 'bot_token' | 'api_key'; diff --git a/src/openhuman/channels/controllers/definitions.rs b/src/openhuman/channels/controllers/definitions.rs index 2a14bf4118..ba35f5b7b2 100644 --- a/src/openhuman/channels/controllers/definitions.rs +++ b/src/openhuman/channels/controllers/definitions.rs @@ -158,6 +158,8 @@ pub fn all_channel_definitions() -> Vec { discord_definition(), web_definition(), imessage_definition(), + lark_definition(), + dingtalk_definition(), ] } @@ -311,6 +313,124 @@ fn imessage_definition() -> ChannelDefinition { } } +/// Lark (国际版) / Feishu (国内版) — Stream WebSocket channel. Wire-protocol +/// already implemented in `src/openhuman/channels/providers/lark.rs`; this +/// definition exposes the existing config surface to the Settings UI so +/// users no longer need to hand-edit `config.toml` to enable it. +/// +/// Field names match `config::schema::channels::LarkConfig` exactly so the +/// frontend can persist credentials through the same RPC the other channels +/// use. See #2048. +fn lark_definition() -> ChannelDefinition { + ChannelDefinition { + id: "lark", + display_name: "Lark / Feishu", + description: "Send and receive via Lark (international) or Feishu (中国版).", + icon: "lark", + auth_modes: vec![AuthModeSpec { + mode: ChannelAuthMode::ApiKey, + description: "Provide your Lark/Feishu app credentials from the Open Platform.", + fields: vec![ + FieldRequirement { + key: "app_id", + label: "App ID", + field_type: "string", + required: true, + placeholder: "cli_xxxxxxxxxxxx", + }, + FieldRequirement { + key: "app_secret", + label: "App Secret", + field_type: "secret", + required: true, + placeholder: "Your Lark app secret", + }, + FieldRequirement { + key: "encrypt_key", + label: "Encrypt Key", + field_type: "secret", + required: false, + placeholder: "Optional — required only if you enabled message encryption", + }, + FieldRequirement { + key: "verification_token", + label: "Verification Token", + field_type: "secret", + required: false, + placeholder: "Optional — used for HTTP webhook verification", + }, + FieldRequirement { + key: "use_feishu", + label: "Use Feishu (中国版)", + field_type: "boolean", + required: false, + placeholder: "On = open.feishu.cn (China); off = open.larksuite.com", + }, + FieldRequirement { + key: "receive_mode", + label: "Receive Mode", + field_type: "string", + required: false, + placeholder: "websocket (default) or webhook", + }, + FieldRequirement { + key: "allowed_users", + label: "Allowed Users", + field_type: "string", + required: false, + placeholder: "Comma-separated open_id / union_id; leave empty to allow any", + }, + ], + auth_action: None, + }], + capabilities: vec![ + ChannelCapability::SendText, + ChannelCapability::ReceiveText, + ChannelCapability::ThreadedReplies, + ], + } +} + +/// DingTalk (钉钉) Stream Mode WebSocket channel. Wire-protocol already +/// implemented in `src/openhuman/channels/providers/dingtalk.rs`. See #2048. +fn dingtalk_definition() -> ChannelDefinition { + ChannelDefinition { + id: "dingtalk", + display_name: "DingTalk (钉钉)", + description: "Send and receive via DingTalk Stream Mode (钉钉).", + icon: "dingtalk", + auth_modes: vec![AuthModeSpec { + mode: ChannelAuthMode::ApiKey, + description: "Provide your DingTalk app credentials from the developer console.", + fields: vec![ + FieldRequirement { + key: "client_id", + label: "Client ID (AppKey)", + field_type: "string", + required: true, + placeholder: "ding_xxxxxxxxxxxx", + }, + FieldRequirement { + key: "client_secret", + label: "Client Secret (AppSecret)", + field_type: "secret", + required: true, + placeholder: "Your DingTalk app secret", + }, + FieldRequirement { + key: "allowed_users", + label: "Allowed Users", + field_type: "string", + required: false, + placeholder: "Comma-separated DingTalk userIds; leave empty to allow any", + }, + ], + auth_action: None, + }], + capabilities: vec![ChannelCapability::SendText, ChannelCapability::ReceiveText], + } +} + #[cfg(test)] #[path = "definitions_tests.rs"] mod tests; diff --git a/src/openhuman/channels/controllers/definitions_tests.rs b/src/openhuman/channels/controllers/definitions_tests.rs index ae6bb31f8c..0bddb95287 100644 --- a/src/openhuman/channels/controllers/definitions_tests.rs +++ b/src/openhuman/channels/controllers/definitions_tests.rs @@ -135,6 +135,138 @@ fn serialization_produces_expected_structure() { assert_eq!(caps.len(), def.capabilities.len()); } +// -- #2048: Lark / Feishu + DingTalk channel definitions ------------------ + +#[test] +fn lark_definition_is_registered() { + let def = find_channel_definition("lark").expect("lark definition not registered"); + assert_eq!(def.display_name, "Lark / Feishu"); + assert_eq!(def.icon, "lark"); +} + +#[test] +fn lark_uses_api_key_auth_with_app_id_and_secret_required() { + let def = find_channel_definition("lark").expect("lark not found"); + let spec = def + .auth_mode_spec(ChannelAuthMode::ApiKey) + .expect("lark must support ApiKey auth mode"); + + // The two non-negotiable fields are app_id + app_secret — every + // Lark/Feishu open-platform app gets these from the developer console. + let app_id = spec + .fields + .iter() + .find(|f| f.key == "app_id") + .expect("missing app_id field"); + assert!(app_id.required, "app_id must be required"); + assert_eq!(app_id.field_type, "string"); + + let app_secret = spec + .fields + .iter() + .find(|f| f.key == "app_secret") + .expect("missing app_secret field"); + assert!(app_secret.required, "app_secret must be required"); + assert_eq!( + app_secret.field_type, "secret", + "app_secret must be a secret-typed field" + ); + + // Optional but supported: encrypt_key, verification_token, use_feishu, + // receive_mode, allowed_users. Field names map 1:1 to `LarkConfig` in + // `src/openhuman/config/schema/channels.rs` — any rename on the backend + // side will fail this assertion before the UI silently breaks. + for key in [ + "encrypt_key", + "verification_token", + "use_feishu", + "receive_mode", + "allowed_users", + ] { + assert!( + spec.fields.iter().any(|f| f.key == key), + "lark spec missing optional field: {}", + key + ); + } + + let use_feishu = spec.fields.iter().find(|f| f.key == "use_feishu").unwrap(); + assert_eq!(use_feishu.field_type, "boolean"); +} + +#[test] +fn lark_validate_credentials_rejects_missing_app_secret() { + let def = find_channel_definition("lark").expect("lark not found"); + let mut creds = serde_json::Map::new(); + creds.insert("app_id".into(), serde_json::Value::String("cli_xx".into())); + // app_secret intentionally omitted. + let err = def + .validate_credentials(ChannelAuthMode::ApiKey, &creds) + .expect_err("must reject when app_secret is missing"); + assert!(err.contains("app_secret"), "{err}"); +} + +#[test] +fn dingtalk_definition_is_registered() { + let def = find_channel_definition("dingtalk").expect("dingtalk definition not registered"); + assert_eq!(def.display_name, "DingTalk (钉钉)"); + assert_eq!(def.icon, "dingtalk"); +} + +#[test] +fn dingtalk_requires_client_id_and_client_secret() { + let def = find_channel_definition("dingtalk").expect("dingtalk not found"); + let spec = def + .auth_mode_spec(ChannelAuthMode::ApiKey) + .expect("dingtalk must support ApiKey auth mode"); + + let client_id = spec + .fields + .iter() + .find(|f| f.key == "client_id") + .expect("missing client_id field"); + assert!(client_id.required); + assert_eq!(client_id.field_type, "string"); + + let client_secret = spec + .fields + .iter() + .find(|f| f.key == "client_secret") + .expect("missing client_secret field"); + assert!(client_secret.required); + assert_eq!( + client_secret.field_type, "secret", + "client_secret must be a secret-typed field" + ); +} + +#[test] +fn dingtalk_validate_credentials_rejects_missing_client_secret() { + let def = find_channel_definition("dingtalk").expect("dingtalk not found"); + let mut creds = serde_json::Map::new(); + creds.insert( + "client_id".into(), + serde_json::Value::String("ding_xx".into()), + ); + let err = def + .validate_credentials(ChannelAuthMode::ApiKey, &creds) + .expect_err("must reject when client_secret is missing"); + assert!(err.contains("client_secret"), "{err}"); +} + +#[test] +fn all_definitions_include_lark_and_dingtalk() { + let ids: Vec<&str> = all_channel_definitions().iter().map(|d| d.id).collect(); + assert!( + ids.contains(&"lark"), + "lark missing from all_channel_definitions" + ); + assert!( + ids.contains(&"dingtalk"), + "dingtalk missing from all_channel_definitions" + ); +} + #[test] fn auth_mode_display_and_parse() { for mode in [ From ee9f56ecaa801793ceb1673cf90868262df1c13c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E5=86=A0=E8=BE=B0?= Date: Mon, 18 May 2026 18:04:26 +0800 Subject: [PATCH 2/5] fix(channels): address CoderRabbit + CI TS failure on #2083 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three follow-ups for the CHANGES_REQUESTED review and the failed Type Check TypeScript CI gate: 1. Add lark/dingtalk entries to channelConnectionsSlice initial state. Widening ChannelType in the previous commit made the Record-typed connections object require all five keys; the initial state still only listed telegram/discord/web, which is why the CI Type Check failed with TS2739 saying lark and dingtalk were missing. Both new keys get makeEmptyChannelModes() so the Record is total without claiming any pre-existing connection state. 2. Add the missing 'port' optional field on Lark (CoderRabbit review). LarkConfig in src/openhuman/config/schema/channels.rs has port as Option u16 for webhook receive mode, and the issue description (#2048) explicitly lists Port as one of the Lark form fields; the previous commit shipped six of the seven optional Lark fields and omitted port. Added to both lark_definition() (backend) and FALLBACK_DEFINITIONS (frontend mirror) with field_type='string' because the form renderer only accepts string/secret/boolean — LarkConfig parses the string back to u16 on the deserialise side. 3. Extend dingtalk_requires_client_id_and_client_secret to also pin the optional allowed_users field by key + required-flag + field_type (CoderRabbit review). Same schema-rename guard rail the Lark test already provides for its optional set. Local: cargo fmt clean; pnpm prettier --check on all changed TS files clean. Schema-driven setup modal continues to render generically over string/secret/boolean — no modal change required for the port field. --- app/src/lib/channels/definitions.ts | 11 +++++++ app/src/store/channelConnectionsSlice.ts | 5 +++ .../channels/controllers/definitions.rs | 13 ++++++++ .../channels/controllers/definitions_tests.rs | 32 ++++++++++++++++--- 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/app/src/lib/channels/definitions.ts b/app/src/lib/channels/definitions.ts index 114cdcc533..d6a5f3d324 100644 --- a/app/src/lib/channels/definitions.ts +++ b/app/src/lib/channels/definitions.ts @@ -173,6 +173,17 @@ export const FALLBACK_DEFINITIONS: ChannelDefinition[] = [ required: false, placeholder: 'websocket (default) or webhook', }, + { + key: 'port', + label: 'Webhook Port', + // Numeric — field_type stays 'string' because the schema-driven + // form renderer only accepts 'string' | 'secret' | 'boolean'. + // LarkConfig parses it back to u16. Keep aligned with the Rust + // lark_definition() entry. + field_type: 'string', + required: false, + placeholder: 'Optional — local HTTP port when receive_mode = webhook (e.g. 8080)', + }, { key: 'allowed_users', label: 'Allowed Users', diff --git a/app/src/store/channelConnectionsSlice.ts b/app/src/store/channelConnectionsSlice.ts index 51537a8b11..7a62ec64a9 100644 --- a/app/src/store/channelConnectionsSlice.ts +++ b/app/src/store/channelConnectionsSlice.ts @@ -26,6 +26,11 @@ const initialState: ChannelConnectionsState = { telegram: makeEmptyChannelModes(), discord: makeEmptyChannelModes(), web: makeEmptyChannelModes(), + // Required by `ChannelType` after #2048 widened the union. Empty + // entries keep the `Record` total — runtime state + // populates them when the user wires up credentials. + lark: makeEmptyChannelModes(), + dingtalk: makeEmptyChannelModes(), }, }; diff --git a/src/openhuman/channels/controllers/definitions.rs b/src/openhuman/channels/controllers/definitions.rs index ba35f5b7b2..445437e193 100644 --- a/src/openhuman/channels/controllers/definitions.rs +++ b/src/openhuman/channels/controllers/definitions.rs @@ -373,6 +373,19 @@ fn lark_definition() -> ChannelDefinition { required: false, placeholder: "websocket (default) or webhook", }, + FieldRequirement { + key: "port", + label: "Webhook Port", + // FieldRequirement.field_type currently accepts + // "string" / "secret" / "boolean" only (see the doc + // comment on FieldRequirement). Numeric port values + // are typed in as plain strings; the LarkConfig + // deserialiser parses them back to u16. + field_type: "string", + required: false, + placeholder: + "Optional — local HTTP port when receive_mode = webhook (e.g. 8080)", + }, FieldRequirement { key: "allowed_users", label: "Allowed Users", diff --git a/src/openhuman/channels/controllers/definitions_tests.rs b/src/openhuman/channels/controllers/definitions_tests.rs index 0bddb95287..a143591f97 100644 --- a/src/openhuman/channels/controllers/definitions_tests.rs +++ b/src/openhuman/channels/controllers/definitions_tests.rs @@ -173,19 +173,25 @@ fn lark_uses_api_key_auth_with_app_id_and_secret_required() { ); // Optional but supported: encrypt_key, verification_token, use_feishu, - // receive_mode, allowed_users. Field names map 1:1 to `LarkConfig` in - // `src/openhuman/config/schema/channels.rs` — any rename on the backend - // side will fail this assertion before the UI silently breaks. + // receive_mode, port, allowed_users. Field names map 1:1 to `LarkConfig` + // in `src/openhuman/config/schema/channels.rs` — any rename on the + // backend side will fail this assertion before the UI silently breaks. for key in [ "encrypt_key", "verification_token", "use_feishu", "receive_mode", + "port", "allowed_users", ] { + let field = spec + .fields + .iter() + .find(|f| f.key == key) + .unwrap_or_else(|| panic!("lark spec missing optional field: {}", key)); assert!( - spec.fields.iter().any(|f| f.key == key), - "lark spec missing optional field: {}", + !field.required, + "lark optional field {} must not be required", key ); } @@ -238,6 +244,22 @@ fn dingtalk_requires_client_id_and_client_secret() { client_secret.field_type, "secret", "client_secret must be a secret-typed field" ); + + // DingTalkConfig in `src/openhuman/config/schema/channels.rs` also + // accepts `allowed_users` (Vec, defaults to empty). Pin it + // as an optional field here for the same reason we pin Lark's + // optional set — schema renames blow up at test time, not in + // production UI. + let allowed_users = spec + .fields + .iter() + .find(|f| f.key == "allowed_users") + .expect("dingtalk spec missing optional allowed_users field"); + assert!( + !allowed_users.required, + "dingtalk allowed_users must not be required" + ); + assert_eq!(allowed_users.field_type, "string"); } #[test] From 456f388845c88d13999d8db14aa55a3352c6982a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E5=86=A0=E8=BE=B0?= Date: Mon, 18 May 2026 18:11:45 +0800 Subject: [PATCH 3/5] fix(channels): repair completeBreakingMigration for lark/dingtalk (#2083 CoderRabbit followup) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CoderRabbit's outside-diff review on PR #2083 caught a second instance of the same bug-class as the first fix: completeBreakingMigration only reset telegram/discord/web. For a user upgrading from a build before #2048 — where redux-persist already has migrationCompleted=true but the persisted connections object lacks the lark/dingtalk keys — the first upsertChannelConnection or setChannelConnectionStatus call for either channel would crash on state.connections[channel] being undefined. UI components defensively short-circuit on missing connections, but the reducer actions don't. Fix --- * channelConnectionsSlice.ts — completeBreakingMigration now also resets connections.lark and connections.dingtalk to empty mode maps. Total migration over the full ChannelType union. Tests ----- * channelConnectionsSlice.test.ts — the existing "completes one-time breaking migration" test now also asserts that all five channels (telegram, discord, web, lark, dingtalk) exist in the migrated state. * New case: "upsert on a newly-introduced channel does not crash after migration (#2083)" — runs migration first, then upserts a lark connection and a dingtalk connection. Pre-fix this would crash; post-fix the upsert succeeds and the api_key entry holds the expected status/capabilities. Vitest run: 5/5 passing (was 4/4) in ~800ms. Prettier --check clean on both files. --- .../__tests__/channelConnectionsSlice.test.ts | 38 +++++++++++++++++++ app/src/store/channelConnectionsSlice.ts | 7 ++++ 2 files changed, 45 insertions(+) diff --git a/app/src/store/__tests__/channelConnectionsSlice.test.ts b/app/src/store/__tests__/channelConnectionsSlice.test.ts index 928bbc869e..8efd668abf 100644 --- a/app/src/store/__tests__/channelConnectionsSlice.test.ts +++ b/app/src/store/__tests__/channelConnectionsSlice.test.ts @@ -11,6 +11,44 @@ describe('channelConnectionsSlice', () => { const state = reducer(undefined, completeBreakingMigration()); expect(state.migrationCompleted).toBe(true); expect(state.defaultMessagingChannel).toBe('telegram'); + // Migration must reset every channel in ChannelType so subsequent + // upsert/setStatus/disconnect actions never crash on `state.connections + // [channel]` being undefined for users rehydrating persisted state + // from before #2048 added lark + dingtalk. See CoderRabbit review on + // PR #2083. + expect(state.connections.telegram).toBeDefined(); + expect(state.connections.discord).toBeDefined(); + expect(state.connections.web).toBeDefined(); + expect(state.connections.lark).toBeDefined(); + expect(state.connections.dingtalk).toBeDefined(); + }); + + it('upsert on a newly-introduced channel does not crash after migration (#2083)', () => { + // Regression for the persisted-state crash CoderRabbit flagged: + // before this fix, an old user who had `migrationCompleted: true` in + // redux-persist but no `connections.lark` key would crash on the + // first call to upsertChannelConnection for lark. + const migrated = reducer(undefined, completeBreakingMigration()); + const next = reducer( + migrated, + upsertChannelConnection({ + channel: 'lark', + authMode: 'api_key', + patch: { status: 'connected', capabilities: ['send_text'] }, + }) + ); + expect(next.connections.lark.api_key?.status).toBe('connected'); + expect(next.connections.lark.api_key?.capabilities).toEqual(['send_text']); + + const next2 = reducer( + migrated, + upsertChannelConnection({ + channel: 'dingtalk', + authMode: 'api_key', + patch: { status: 'connected', capabilities: ['send_text'] }, + }) + ); + expect(next2.connections.dingtalk.api_key?.status).toBe('connected'); }); it('sets default messaging channel', () => { diff --git a/app/src/store/channelConnectionsSlice.ts b/app/src/store/channelConnectionsSlice.ts index 7a62ec64a9..f984887298 100644 --- a/app/src/store/channelConnectionsSlice.ts +++ b/app/src/store/channelConnectionsSlice.ts @@ -60,6 +60,13 @@ const channelConnectionsSlice = createSlice({ state.connections.telegram = makeEmptyChannelModes(); state.connections.discord = makeEmptyChannelModes(); state.connections.web = makeEmptyChannelModes(); + // After #2048 widened ChannelType, redux-persist rehydrated states + // from before the channels existed wouldn't have these keys; without + // explicit initialisation here, the first `upsertChannelConnection` + // for either channel would crash on `state.connections[channel]` + // being undefined. Pin them by default so the migration is total. + state.connections.lark = makeEmptyChannelModes(); + state.connections.dingtalk = makeEmptyChannelModes(); state.defaultMessagingChannel = 'telegram'; state.migrationCompleted = true; state.schemaVersion = SCHEMA_VERSION; From 4ef60ea2e5f0fdb9ca3430c45dbca9267d89a5b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E5=86=A0=E8=BE=B0?= Date: Mon, 18 May 2026 18:44:42 +0800 Subject: [PATCH 4/5] test(messaging): cover CHANNEL_ICONS mapping to clear diff-coverage gate (#2083) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Coverage Gate (diff-cover ≥ 80%) check on PR #2083 was the only remaining red signal on this branch. It reported: app/src/components/settings/panels/MessagingPanel.tsx (0.0%): Missing lines 18 Coverage: 66% Line 18 is the CHANNEL_ICONS Record declaration where the previous commit added the lark and dingtalk entries. The full panel renders the icons but exercising that render in a unit test requires standing up Redux + i18n + routing scaffolding, which is heavy for what amounts to verifying a static mapping. Two-line refactor: * MessagingPanel.tsx — promote CHANNEL_ICONS to a named export with a short rustdoc-style block describing why it's exported (kept-in-sync contract with the backend definitions registry). Zero behavioural change; the panel still consumes it locally. * messagingPanelIcons.test.ts — new focused unit test that imports the exported mapping and asserts the five shipped icon slugs. Three cases: - "includes icons for every shipped channel slug" — telegram / discord / web (regression guard against accidental key renames). - "includes Lark/Feishu and DingTalk icons (#2048)" — the new additions. - "has no duplicate emoji values (icons remain visually distinct)" — guards against the easy mistake of copy-pasting one emoji into two channels. Vitest run: 3/3 pass in 1.20s. Prettier --check clean (the prettier --write step reports unchanged, indicating no formatting drift). Diff coverage should now report 100% on the touched lines for both files. --- .../settings/panels/MessagingPanel.tsx | 15 +++++++-- .../__tests__/messagingPanelIcons.test.ts | 32 +++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 app/src/components/settings/panels/__tests__/messagingPanelIcons.test.ts diff --git a/app/src/components/settings/panels/MessagingPanel.tsx b/app/src/components/settings/panels/MessagingPanel.tsx index 442b63a90d..12ecf14d3a 100644 --- a/app/src/components/settings/panels/MessagingPanel.tsx +++ b/app/src/components/settings/panels/MessagingPanel.tsx @@ -15,13 +15,22 @@ import ChannelSetupModal from '../../channels/ChannelSetupModal'; import SettingsHeader from '../components/SettingsHeader'; import { useSettingsNavigation } from '../hooks/useSettingsNavigation'; -const CHANNEL_ICONS: Record = { +/** + * Mapping from `ChannelDefinition.icon` slugs to the emoji rendered next to + * each channel in the Messaging settings panel. Exported so unit tests can + * assert against it without rendering the full panel (the panel pulls in + * Redux, i18n, routing, and `useChannelDefinitions`, all of which make a + * focused render test more expensive than a direct mapping assertion). + * Keep in sync with the icon slugs returned by the backend + * `channels::controllers::definitions::all_channel_definitions`. + */ +export const CHANNEL_ICONS: Record = { telegram: '✈️', discord: '🎮', web: '🌐', - // Lark (国际版) / Feishu (中国版) — same backend, single icon. + // Lark (国际版) / Feishu (中国版) — same backend, single icon. See #2048. lark: '🪶', - // DingTalk (钉钉). + // DingTalk (钉钉). See #2048. dingtalk: '🔔', }; diff --git a/app/src/components/settings/panels/__tests__/messagingPanelIcons.test.ts b/app/src/components/settings/panels/__tests__/messagingPanelIcons.test.ts new file mode 100644 index 0000000000..07013d17d5 --- /dev/null +++ b/app/src/components/settings/panels/__tests__/messagingPanelIcons.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from 'vitest'; + +import { CHANNEL_ICONS } from '../MessagingPanel'; + +describe('MessagingPanel CHANNEL_ICONS', () => { + it('includes icons for every shipped channel slug', () => { + // The backend `channels::controllers::definitions::all_channel_definitions` + // emits these icon slugs; the Messaging panel must render an emoji for + // each or the channel row gets a blank gap. Pin them by key so a renamed + // slug on either side fails this test instead of a silent visual break. + expect(CHANNEL_ICONS.telegram).toBe('✈️'); + expect(CHANNEL_ICONS.discord).toBe('🎮'); + expect(CHANNEL_ICONS.web).toBe('🌐'); + }); + + it('includes Lark/Feishu and DingTalk icons (#2048)', () => { + // Regression for #2048 — adding the channel definitions without the + // matching icon entries produced a blank chip next to each channel in + // the Messaging settings panel. + expect(CHANNEL_ICONS.lark).toBe('🪶'); + expect(CHANNEL_ICONS.dingtalk).toBe('🔔'); + }); + + it('has no duplicate emoji values (icons remain visually distinct)', () => { + const values = Object.values(CHANNEL_ICONS); + const unique = new Set(values); + expect(unique.size).toBe( + values.length, + 'two channels share the same emoji — users cannot visually distinguish their rows' + ); + }); +}); From e1f6a557c65f9b3f05e19e11335abc6c32f45b19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E5=86=A0=E8=BE=B0?= Date: Mon, 18 May 2026 18:50:15 +0800 Subject: [PATCH 5/5] fix(test): drop Jest-style 2nd arg from .toBe() in CHANNEL_ICONS uniqueness check (#2083) CI Type Check TypeScript failed on: src/components/settings/panels/__tests__/messagingPanelIcons.test.ts(29,7): error TS2554: Expected 1 arguments, but got 2. The new uniqueness test used the Jest convention where the second positional arg to expect().toBe() is a failure-message string. Vitest's .toBe() has only one parameter; the second arg silently runs at runtime but trips tsc --noEmit at compile time. Drop the second arg; the rationale moves into a comment block above the assertion so the failure-mode explanation isn't lost. Local: pnpm compile (tsc --noEmit) clean; pnpm test:unit messagingPanelIcons 3/3 pass in 1.09s. --- .../settings/panels/__tests__/messagingPanelIcons.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/src/components/settings/panels/__tests__/messagingPanelIcons.test.ts b/app/src/components/settings/panels/__tests__/messagingPanelIcons.test.ts index 07013d17d5..089ea28b11 100644 --- a/app/src/components/settings/panels/__tests__/messagingPanelIcons.test.ts +++ b/app/src/components/settings/panels/__tests__/messagingPanelIcons.test.ts @@ -22,11 +22,11 @@ describe('MessagingPanel CHANNEL_ICONS', () => { }); it('has no duplicate emoji values (icons remain visually distinct)', () => { + // Two channels sharing the same emoji would make their rows visually + // indistinguishable in the panel. Asserting uniqueness here catches + // the easy copy-paste mistake at test time. const values = Object.values(CHANNEL_ICONS); const unique = new Set(values); - expect(unique.size).toBe( - values.length, - 'two channels share the same emoji — users cannot visually distinguish their rows' - ); + expect(unique.size).toBe(values.length); }); });