diff --git a/app/src/components/settings/panels/MessagingPanel.tsx b/app/src/components/settings/panels/MessagingPanel.tsx index 68e53bb514..12ecf14d3a 100644 --- a/app/src/components/settings/panels/MessagingPanel.tsx +++ b/app/src/components/settings/panels/MessagingPanel.tsx @@ -15,7 +15,24 @@ import ChannelSetupModal from '../../channels/ChannelSetupModal'; import SettingsHeader from '../components/SettingsHeader'; import { useSettingsNavigation } from '../hooks/useSettingsNavigation'; -const CHANNEL_ICONS: Record = { telegram: '✈ïļ', discord: 'ðŸŽŪ', web: '🌐' }; +/** + * 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. See #2048. + lark: 'ðŸŠķ', + // DingTalk (钉钉). See #2048. + dingtalk: '🔔', +}; function statusDot(status: ChannelConnectionStatus): string { switch (status) { 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..089ea28b11 --- /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)', () => { + // 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); + }); +}); diff --git a/app/src/lib/channels/definitions.ts b/app/src/lib/channels/definitions.ts index 074f70e55c..d6a5f3d324 100644 --- a/app/src/lib/channels/definitions.ts +++ b/app/src/lib/channels/definitions.ts @@ -118,4 +118,122 @@ 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: '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', + 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/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 51537a8b11..f984887298 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(), }, }; @@ -55,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; 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..445437e193 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,137 @@ 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: "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", + 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..a143591f97 100644 --- a/src/openhuman/channels/controllers/definitions_tests.rs +++ b/src/openhuman/channels/controllers/definitions_tests.rs @@ -135,6 +135,160 @@ 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, 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!( + !field.required, + "lark optional field {} must not be required", + 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" + ); + + // 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] +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 [