fix(channels): managed-DM credentials surface as connected to chat (#1209)

This commit is contained in:
obchain
2026-05-05 11:24:28 -07:00
committed by GitHub
parent 28b35f48b1
commit 1ac3feed3a
8 changed files with 497 additions and 56 deletions
@@ -13,3 +13,8 @@ pub use schemas::{
all_controller_schemas as all_channels_controller_schemas,
all_registered_controllers as all_channels_registered_controllers,
};
/// Cross-module helpers from the channel controller layer that callers
/// outside the controller registry need (e.g. the welcome agent's
/// onboarding status snapshot).
pub use ops::connected_channel_slugs;
+92 -3
View File
@@ -420,12 +420,15 @@ pub async fn channel_status(
config: &Config,
channel_id: Option<&str>,
) -> Result<RpcOutcome<Vec<ChannelStatusEntry>>, String> {
// List all stored credentials with "channel:" prefix.
let stored = credentials::ops::list_provider_credentials(config, Some("channel:".to_string()))
// List all stored credentials with "channel:" prefix. Uses the
// prefix-match helper because channel credentials are keyed as
// `channel:<id>:<mode>` and no single literal value matches them
// through `list_provider_credentials`'s exact-match filter.
let stored = credentials::ops::list_provider_credentials_by_prefix(config, "channel:")
.await
.map_err(|e| format!("failed to list credentials: {e}"))?;
let stored_providers: Vec<String> = stored.value.iter().map(|p| p.provider.clone()).collect();
let stored_providers: Vec<String> = stored.iter().map(|p| p.provider.clone()).collect();
let defs = match channel_id {
Some(id) => {
@@ -453,6 +456,92 @@ pub async fn channel_status(
Ok(RpcOutcome::new(entries, vec![]))
}
/// Return the slugs of all messaging channels currently connected,
/// merging the two storage layers OpenHuman uses for connection state.
///
/// Two equally-authoritative sources exist today:
///
/// * `config.channels_config.<slug>` — the legacy TOML field set by
/// credential-mode connects that need a runtime listener
/// (`bot_token` / `webhook` / `oauth`). These trigger
/// `restart_required = true` on the connect call.
/// * Provider credentials keyed `channel:<slug>:<mode>` — set by the
/// newer managed-DM and OAuth flows that don't materialise a TOML
/// block but do persist a credential marker.
///
/// Until both stores merge, any caller that only reads one will report
/// stale state to the user (e.g. the agent will say "Telegram not
/// connected" right after a managed-DM link succeeds — issue #1149).
/// This helper centralises the merge so every consumer agrees.
pub async fn connected_channel_slugs(config: &Config) -> Result<Vec<String>, String> {
use std::collections::BTreeSet;
let mut slugs: BTreeSet<String> = BTreeSet::new();
// Layer 1: credential-mode channels written to TOML config.
let cc = &config.channels_config;
if cc.telegram.is_some() {
slugs.insert("telegram".to_string());
}
if cc.discord.is_some() {
slugs.insert("discord".to_string());
}
if cc.slack.is_some() {
slugs.insert("slack".to_string());
}
if cc.mattermost.is_some() {
slugs.insert("mattermost".to_string());
}
if cc.email.is_some() {
slugs.insert("email".to_string());
}
if cc.whatsapp.is_some() {
slugs.insert("whatsapp".to_string());
}
if cc.signal.is_some() {
slugs.insert("signal".to_string());
}
if cc.matrix.is_some() {
slugs.insert("matrix".to_string());
}
if cc.imessage.is_some() {
slugs.insert("imessage".to_string());
}
if cc.irc.is_some() {
slugs.insert("irc".to_string());
}
if cc.lark.is_some() {
slugs.insert("lark".to_string());
}
if cc.dingtalk.is_some() {
slugs.insert("dingtalk".to_string());
}
if cc.linq.is_some() {
slugs.insert("linq".to_string());
}
if cc.qq.is_some() {
slugs.insert("qq".to_string());
}
// Layer 2: managed-DM / OAuth channels stored only as credentials
// under `channel:<slug>:<mode>`.
let stored = credentials::ops::list_provider_credentials_by_prefix(config, "channel:")
.await
.map_err(|e| format!("failed to list channel credentials: {e}"))?;
for entry in &stored {
// provider format: "channel:<slug>:<mode>" — extract slug.
if let Some(rest) = entry.provider.strip_prefix("channel:") {
if let Some((slug, _mode)) = rest.split_once(':') {
if !slug.is_empty() {
slugs.insert(slug.to_string());
}
}
}
}
Ok(slugs.into_iter().collect())
}
/// Test a channel connection without persisting credentials.
pub async fn test_channel(
_config: &Config,
@@ -360,3 +360,123 @@ async fn disconnect_imessage_clears_runtime_config() {
.and_then(|v| v.get("imessage"));
assert!(im_entry.is_none(), "imessage config should be cleared");
}
// ---------------------------------------------------------------------------
// Issue #1149: managed-DM / OAuth channels are stored only in the credential
// layer (`channel:<slug>:<mode>`), not in `channels_config.<slug>`. Both
// `channel_status` and `connected_channel_slugs` must surface them so the
// chat agent stops reporting "Telegram not connected" right after a
// managed-DM link succeeds.
// ---------------------------------------------------------------------------
#[tokio::test]
async fn channel_status_reports_managed_dm_credential_as_connected() {
let (_tmp, config) = isolated_test_config();
// Simulate the post-link state: `telegram_login_check` stored a
// credential marker under `channel:telegram:managed_dm` with no
// corresponding `channels_config.telegram` block.
crate::openhuman::credentials::ops::store_provider_credentials(
&config,
"channel:telegram:managed_dm",
None,
Some("managed".to_string()),
Some(serde_json::json!({ "linked": true })),
Some(true),
)
.await
.expect("seed managed-DM credential");
let result = channel_status(&config, Some("telegram"))
.await
.expect("channel_status should succeed");
let managed_dm = result
.value
.iter()
.find(|e| e.auth_mode == ChannelAuthMode::ManagedDm)
.expect("managed_dm entry");
assert!(
managed_dm.connected,
"managed-DM credential should report connected: {:?}",
result.value
);
assert!(managed_dm.has_credentials);
}
#[tokio::test]
async fn connected_channel_slugs_merges_credentials_and_config() {
let (_tmp, mut config) = isolated_test_config();
// Layer 1: TOML-resident channel (e.g. discord bot_token).
config.channels_config.discord = Some(DiscordConfig {
bot_token: "tok".to_string(),
guild_id: None,
channel_id: None,
allowed_users: vec![],
listen_to_bots: false,
mention_only: false,
});
// Layer 2: credential-only channel (telegram managed_dm).
crate::openhuman::credentials::ops::store_provider_credentials(
&config,
"channel:telegram:managed_dm",
None,
Some("managed".to_string()),
Some(serde_json::json!({ "linked": true })),
Some(true),
)
.await
.expect("seed managed-DM credential");
let slugs = connected_channel_slugs(&config)
.await
.expect("connected_channel_slugs should succeed");
assert!(slugs.contains(&"discord".to_string()), "got {slugs:?}");
assert!(slugs.contains(&"telegram".to_string()), "got {slugs:?}");
}
#[tokio::test]
async fn connected_channel_slugs_dedupes_when_both_layers_present() {
let (_tmp, mut config) = isolated_test_config();
config.channels_config.discord = Some(DiscordConfig {
bot_token: "tok".to_string(),
guild_id: None,
channel_id: None,
allowed_users: vec![],
listen_to_bots: false,
mention_only: false,
});
// Same slug appears in both layers — should collapse to one entry.
crate::openhuman::credentials::ops::store_provider_credentials(
&config,
"channel:discord:managed_dm",
None,
Some("managed".to_string()),
Some(serde_json::json!({ "linked": true })),
Some(true),
)
.await
.expect("seed managed-DM credential");
let slugs = connected_channel_slugs(&config)
.await
.expect("connected_channel_slugs should succeed");
let discord_count = slugs.iter().filter(|s| *s == "discord").count();
assert_eq!(discord_count, 1, "discord should appear once: {slugs:?}");
}
#[tokio::test]
async fn connected_channel_slugs_empty_when_nothing_configured() {
let (_tmp, config) = isolated_test_config();
let slugs = connected_channel_slugs(&config).await.unwrap();
assert!(
slugs.is_empty(),
"fresh config should yield no channels: {slugs:?}"
);
}
+29
View File
@@ -479,6 +479,35 @@ pub async fn list_provider_credentials(
Ok(RpcOutcome::single_log(items, "provider credentials listed"))
}
/// List credentials whose provider key starts with `prefix`.
///
/// Pure prefix variant of [`list_provider_credentials`] for namespaces
/// that group multiple providers under a common stem (e.g.
/// `"channel:"` covers `channel:telegram:managed_dm`,
/// `channel:slack:bot_token`, …). The exact-match filter on
/// `list_provider_credentials` cannot express this without enumerating
/// every concrete provider key up front.
pub async fn list_provider_credentials_by_prefix(
config: &Config,
prefix: &str,
) -> Result<Vec<super::responses::AuthProfileSummary>, String> {
let auth = AuthService::from_config(config);
let profiles = auth.load_profiles().map_err(|e| e.to_string())?;
let mut items = profiles
.profiles
.values()
.filter(|profile| profile.provider != APP_SESSION_PROVIDER)
.filter(|profile| profile.provider.starts_with(prefix))
.map(summarize_auth_profile)
.collect::<Vec<_>>();
items.sort_by(|a, b| {
a.provider
.cmp(&b.provider)
.then_with(|| a.profile_name.cmp(&b.profile_name))
});
Ok(items)
}
pub async fn oauth_connect(
config: &Config,
provider: &str,
+61
View File
@@ -390,3 +390,64 @@ async fn auth_get_me_errors_without_session() {
let err = auth_get_me(&config).await.unwrap_err();
assert!(err.contains("session JWT required"));
}
// ── list_provider_credentials_by_prefix ───────────────────────
/// Issue #1149 root-cause regression: the exact-match filter on
/// `list_provider_credentials` cannot enumerate provider keys grouped
/// under a common stem (e.g. `channel:telegram:managed_dm`,
/// `channel:slack:bot_token`). The prefix variant fixes that — without
/// it, `channel_status` always returned `connected: false`.
#[tokio::test]
async fn list_provider_credentials_by_prefix_matches_namespaced_keys() {
let tmp = TempDir::new().unwrap();
let config = test_config(&tmp);
for provider in [
"channel:telegram:managed_dm",
"channel:slack:bot_token",
"skill:notion",
] {
store_provider_credentials(
&config,
provider,
None,
Some("token-x".to_string()),
None,
Some(true),
)
.await
.expect("seed credential");
}
let channels = list_provider_credentials_by_prefix(&config, "channel:")
.await
.expect("prefix list should succeed");
let providers: Vec<&str> = channels.iter().map(|p| p.provider.as_str()).collect();
assert_eq!(channels.len(), 2, "got {providers:?}");
assert!(providers.contains(&"channel:slack:bot_token"));
assert!(providers.contains(&"channel:telegram:managed_dm"));
}
#[tokio::test]
async fn list_provider_credentials_by_prefix_returns_empty_when_no_match() {
let tmp = TempDir::new().unwrap();
let config = test_config(&tmp);
store_provider_credentials(
&config,
"skill:notion",
None,
Some("token-x".to_string()),
None,
Some(true),
)
.await
.expect("seed credential");
let result = list_provider_credentials_by_prefix(&config, "channel:")
.await
.expect("prefix list should succeed");
assert!(result.is_empty(), "got {result:?}");
}
@@ -125,6 +125,7 @@ async fn check_status() -> anyhow::Result<ToolResult> {
state.ready_to_complete,
&state.ready_to_complete_reason,
&state.composio_connected_toolkits,
&state.connected_channels,
&webview_logins,
);
@@ -130,57 +130,12 @@ pub(crate) fn detect_auth(config: &Config) -> (bool, Value) {
/// for that provider. See `openhuman::webview_accounts`.
/// * `exchange_count` / `ready_to_complete` / `ready_to_complete_reason`
/// — the gate the finalizer enforces.
/// Walk `config.channels_config` and return the connected messaging-channel
/// slugs in a stable order. Shared between `build_status_snapshot` and
/// `format_status_markdown` so the channel list can't drift between the
/// JSON and markdown views.
fn detect_channels(config: &Config) -> Vec<&'static str> {
let cc = &config.channels_config;
let mut out: Vec<&'static str> = Vec::new();
if cc.telegram.is_some() {
out.push("telegram");
}
if cc.discord.is_some() {
out.push("discord");
}
if cc.slack.is_some() {
out.push("slack");
}
if cc.mattermost.is_some() {
out.push("mattermost");
}
if cc.email.is_some() {
out.push("email");
}
if cc.whatsapp.is_some() {
out.push("whatsapp");
}
if cc.signal.is_some() {
out.push("signal");
}
if cc.matrix.is_some() {
out.push("matrix");
}
if cc.imessage.is_some() {
out.push("imessage");
}
if cc.irc.is_some() {
out.push("irc");
}
if cc.lark.is_some() {
out.push("lark");
}
if cc.dingtalk.is_some() {
out.push("dingtalk");
}
if cc.linq.is_some() {
out.push("linq");
}
if cc.qq.is_some() {
out.push("qq");
}
out
}
// Channel detection now lives in
// `crate::openhuman::channels::controllers::ops::connected_channel_slugs`
// (precomputed in `compute_state` because it needs to read the
// credential store), so the welcome-agent surface honors managed-DM /
// OAuth connections that don't materialise a TOML
// `channels_config.<slug>` block (issue #1149).
pub(crate) fn build_status_snapshot(
config: &Config,
@@ -189,10 +144,11 @@ pub(crate) fn build_status_snapshot(
ready_to_complete: bool,
ready_to_complete_reason: &str,
composio_connected_toolkits: &[String],
connected_channels: &[String],
webview_logins: Value,
) -> Value {
let (is_authenticated, auth_source) = detect_auth(config);
let channels_connected = detect_channels(config);
let channels_connected: Vec<&str> = connected_channels.iter().map(|s| s.as_str()).collect();
let composio_enabled = config.composio.enabled;
let delegate_agents: Vec<&str> = config.agents.keys().map(|s| s.as_str()).collect();
@@ -246,10 +202,11 @@ pub(crate) fn format_status_markdown(
ready_to_complete: bool,
ready_to_complete_reason: &str,
composio_connected_toolkits: &[String],
connected_channels: &[String],
webview_logins: &Value,
) -> String {
let (is_authenticated, auth_source) = detect_auth(config);
let channels = detect_channels(config);
let channels: Vec<&str> = connected_channels.iter().map(|s| s.as_str()).collect();
let active_channel = config
.channels_config
@@ -321,6 +278,11 @@ pub(crate) struct OnboardingState {
pub is_authenticated: bool,
pub exchange_count: u32,
pub composio_connected_toolkits: Vec<String>,
/// Slugs of messaging channels currently connected, merged across
/// the legacy `channels_config.<slug>` TOML store and the
/// `channel:<slug>:<mode>` credential store. Computed in
/// [`compute_state`] (issue #1149).
pub connected_channels: Vec<String>,
pub onboarding_status: &'static str,
pub ready_to_complete: bool,
pub ready_to_complete_reason: String,
@@ -340,6 +302,22 @@ pub(crate) async fn compute_state(
.collect();
let composio_connections = composio_connected_toolkits.len() as u32;
// Merge legacy `channels_config.<slug>` with the credential store
// so managed-DM / OAuth channels (e.g. Telegram managed_dm) report
// as connected to the welcome agent (issue #1149). Best-effort —
// a credential-store read failure logs and falls back to empty
// rather than masking the rest of the snapshot.
let connected_channels =
crate::openhuman::channels::controllers::connected_channel_slugs(config)
.await
.unwrap_or_else(|err| {
tracing::warn!(
error = %err,
"[onboarding] connected_channel_slugs failed; reporting empty channel list"
);
Vec::new()
});
let onboarding_status = if !is_authenticated {
"unauthenticated"
} else if config.chat_onboarding_completed {
@@ -365,6 +343,7 @@ pub(crate) async fn compute_state(
is_authenticated,
exchange_count,
composio_connected_toolkits,
connected_channels,
onboarding_status,
ready_to_complete,
ready_to_complete_reason,
@@ -385,6 +364,7 @@ mod tests {
false,
"no_apps_connected",
&[],
&[],
json!({"gmail": false}),
);
assert_eq!(snap["onboarding_status"], "pending");
@@ -400,6 +380,8 @@ mod tests {
0
);
assert_eq!(snap["webview_logins"]["gmail"], false);
assert!(snap["channels_connected"].is_array());
assert_eq!(snap["channels_connected"].as_array().unwrap().len(), 0);
}
#[test]
@@ -412,6 +394,7 @@ mod tests {
false,
"no_apps_connected",
&["gmail".to_string(), "github".to_string()],
&[],
json!({"gmail": true, "whatsapp": false}),
);
let toolkits = snap["composio_connected_toolkits"].as_array().unwrap();
@@ -421,6 +404,51 @@ mod tests {
assert_eq!(snap["webview_logins"]["whatsapp"], false);
}
/// Issue #1149: managed-DM / OAuth channels are stored in the
/// credential layer, not in `channels_config`. The snapshot must
/// reflect them so the welcome agent doesn't say "Telegram not
/// connected" right after a managed-DM link succeeds.
#[test]
fn build_status_snapshot_surfaces_credential_only_channels() {
let config = Config::default();
// `channels_config.telegram` is None — the channel was linked
// via the managed-DM flow which only writes a credential entry.
// The merged slug list (built upstream by `compute_state`) is
// what `build_status_snapshot` consumes.
let snap = build_status_snapshot(
&config,
"pending",
1,
false,
"no_apps_connected",
&[],
&["telegram".to_string()],
json!({}),
);
let channels = snap["channels_connected"].as_array().unwrap();
assert_eq!(channels.len(), 1);
assert_eq!(channels[0], "telegram");
}
#[test]
fn format_status_markdown_surfaces_credential_only_channels() {
let config = Config::default();
let md = format_status_markdown(
&config,
"pending",
1,
false,
"no_apps_connected",
&[],
&["telegram".to_string()],
&json!({}),
);
assert!(
md.contains("**channels:** telegram"),
"markdown should surface credential-stored channel: {md}"
);
}
#[test]
fn detect_auth_on_default_config_is_unauthenticated() {
let config = Config::default();
+108
View File
@@ -3273,3 +3273,111 @@ async fn external_process_with_guessed_token_is_rejected() {
rpc_join.abort();
}
/// End-to-end coverage for issue #1149: storing a managed-DM channel
/// credential under `channel:<slug>:<mode>` and immediately observing
/// `connected:true` from `openhuman.channels_status`.
///
/// Before the fix, `channels_status` always returned `connected:false`
/// because the underlying `list_provider_credentials` call used an
/// exact-match filter (`provider == "channel:"`) that never matched
/// the real credential keys (`channel:telegram:managed_dm`,
/// `channel:slack:bot_token`, …). The user could connect Telegram in
/// the UI but the chat / Settings page would still report it
/// disconnected on the next reload.
///
/// This test exercises the full RPC wire path so a regression in
/// either the prefix helper or the channels controller is caught at
/// the transport layer, not just at the unit level.
#[tokio::test]
async fn channels_status_reflects_managed_dm_credential_e2e() {
let _env_lock = json_rpc_e2e_env_lock();
let tmp = tempdir().expect("tempdir");
let home = tmp.path();
let openhuman_home = home.join(".openhuman");
let _home_guard = EnvVarGuard::set_to_path("HOME", home);
let _workspace_guard = EnvVarGuard::unset("OPENHUMAN_WORKSPACE");
let _backend_url_guard = EnvVarGuard::unset("BACKEND_URL");
let _vite_backend_guard = EnvVarGuard::unset("VITE_BACKEND_URL");
let (mock_addr, mock_join) = serve_on_ephemeral(mock_upstream_router()).await;
let mock_origin = format!("http://{}", mock_addr);
write_min_config(&openhuman_home, &mock_origin);
let (rpc_addr, rpc_join) = serve_on_ephemeral(build_core_http_router(false)).await;
let rpc_base = format!("http://{}", rpc_addr);
tokio::time::sleep(Duration::from_millis(100)).await;
// ── 1. baseline: telegram should report disconnected ────────────────────
let baseline = post_json_rpc(
&rpc_base,
7001,
"openhuman.channels_status",
json!({ "channel": "telegram" }),
)
.await;
let baseline_outer = assert_no_jsonrpc_error(&baseline, "channels_status (baseline)");
let baseline_result = baseline_outer.get("result").unwrap_or(baseline_outer);
let baseline_entries = baseline_result
.as_array()
.unwrap_or_else(|| panic!("expected array: {baseline_result}"));
let baseline_managed = baseline_entries
.iter()
.find(|e| e.get("auth_mode").and_then(Value::as_str) == Some("managed_dm"))
.expect("managed_dm entry should exist for telegram");
assert_eq!(
baseline_managed.get("connected").and_then(Value::as_bool),
Some(false),
"fresh config should report telegram managed_dm disconnected: {baseline_managed}"
);
// ── 2. simulate a successful managed-DM link by storing the credential
// marker the way `telegram_login_check` does in production ─────────
let store = post_json_rpc(
&rpc_base,
7002,
"openhuman.auth_store_provider_credentials",
json!({
"provider": "channel:telegram:managed_dm",
"profile": "default",
"token": "managed",
"fields": { "linked": true },
"setActive": true,
}),
)
.await;
assert_no_jsonrpc_error(&store, "auth_store_provider_credentials");
// ── 3. channels_status must now report telegram managed_dm connected ─
let after = post_json_rpc(
&rpc_base,
7003,
"openhuman.channels_status",
json!({ "channel": "telegram" }),
)
.await;
let after_outer = assert_no_jsonrpc_error(&after, "channels_status (after link)");
let after_result = after_outer.get("result").unwrap_or(after_outer);
let after_entries = after_result
.as_array()
.unwrap_or_else(|| panic!("expected array: {after_result}"));
let after_managed = after_entries
.iter()
.find(|e| e.get("auth_mode").and_then(Value::as_str) == Some("managed_dm"))
.expect("managed_dm entry should exist for telegram");
assert_eq!(
after_managed.get("connected").and_then(Value::as_bool),
Some(true),
"managed-DM credential should surface as connected: {after_managed}"
);
assert_eq!(
after_managed
.get("has_credentials")
.and_then(Value::as_bool),
Some(true)
);
mock_join.abort();
rpc_join.abort();
}