diff --git a/src/core/observability.rs b/src/core/observability.rs index fb1c093ec..05e2cf15e 100644 --- a/src/core/observability.rs +++ b/src/core/observability.rs @@ -233,7 +233,17 @@ pub fn expected_error_kind(message: &str) -> Option { if lower.contains("local ai is disabled") { return Some(ExpectedErrorKind::LocalAiDisabled); } - if lower.contains("api key not set") || lower.contains("missing api key") { + // `"no api key is configured"` covers the composio direct-mode factory + // bail (`client.rs`: "composio direct mode selected but no api key is + // configured"). `composio_list_connections` now short-circuits that + // state to an empty list, so the dominant 5 s-poll leak (TAURI-RUST-R4) + // no longer reaches here — but other direct-mode ops the user invokes + // explicitly (execute / authorize) can still surface it, and it is the + // same user-config state with no Sentry-actionable signal. + if lower.contains("api key not set") + || lower.contains("missing api key") + || lower.contains("no api key is configured") + { return Some(ExpectedErrorKind::ApiKeyMissing); } // Check `ChannelSupervisorRestart` BEFORE `is_loopback_unavailable` and @@ -1957,6 +1967,33 @@ mod tests { ); } + /// Sentry TAURI-RUST-R4: the composio direct-mode factory bail must + /// classify as `ApiKeyMissing` so any residual emit (explicit + /// execute/authorize call with no key) stays out of Sentry. Uses the + /// verbatim wire shape from `client.rs`. + #[test] + fn classifies_composio_direct_no_api_key_as_api_key_missing() { + assert_eq!( + expected_error_kind( + "[composio] list_connections: composio direct mode selected but no api key \ + is configured (set via composio.set_api_key RPC or config.composio.api_key)" + ), + Some(ExpectedErrorKind::ApiKeyMissing) + ); + } + + /// Guard against over-suppression: a genuine BYO-key auth failure + /// (wrong/expired key the upstream rejected) is an actionable bug + /// shape and MUST still reach Sentry (stay `None`), not get demoted by + /// the new "no api key is configured" substring. + #[test] + fn does_not_classify_invalid_api_key_401_as_missing() { + assert_eq!( + expected_error_kind("OpenAI API error (401 Unauthorized): invalid_api_key"), + None + ); + } + /// Task B (issue #2898): prove the canonical 429 error message produced by /// the embedding clients is already classified as `TransientUpstreamHttp` /// so Sentry events are suppressed even without backoff. diff --git a/src/openhuman/composio/ops.rs b/src/openhuman/composio/ops.rs index a357cc43f..fd1209f00 100644 --- a/src/openhuman/composio/ops.rs +++ b/src/openhuman/composio/ops.rs @@ -84,6 +84,40 @@ fn resolve_client(config: &Config) -> OpResult { }) } +/// True when the user has selected Composio **direct** mode but has not yet +/// configured an API key (neither in the keychain nor `config.toml`). +/// +/// This is a valid, user-controlled *setup* state — the user just flipped to +/// direct mode and is about to paste their key — NOT an operation failure. +/// Callers short-circuit to an empty result instead of letting the +/// mode-aware factory bail with "composio direct mode selected but no api key +/// is configured", which the desktop UI's 5 s poll would otherwise funnel to +/// Sentry on every tick (TAURI-RUST-R4). +/// +/// Key presence MUST mirror the factory's own resolution in +/// [`create_composio_client`] (`client.rs`): a key counts if it is in the +/// keychain (`credentials::get_composio_api_key`) **or** in `config.toml` +/// (`config.composio.api_key`). Checking only the keychain would wrongly +/// short-circuit to an empty list for a user who configured their key via +/// `config.toml`, hiding their real connections. +fn direct_mode_without_key(config: &Config) -> OpResult { + if config.composio.mode.trim() != crate::openhuman::config::schema::COMPOSIO_MODE_DIRECT { + return Ok(false); + } + let has_key = crate::openhuman::credentials::get_composio_api_key(config) + .map_err(|e| format!("[composio] get_composio_api_key failed: {e}"))? + .or_else(|| { + config + .composio + .api_key + .as_ref() + .map(|k| k.trim().to_string()) + .filter(|k| !k.is_empty()) + }) + .is_some(); + Ok(!has_key) +} + /// Defense-in-depth Sentry funnel for composio op-layer errors. /// /// The shared [`crate::openhuman::integrations::IntegrationClient`] @@ -266,6 +300,30 @@ pub async fn composio_list_connections( config: &Config, ) -> OpResult> { tracing::debug!("[composio] rpc list_connections"); + // [Sentry TAURI-RUST-R4] Direct mode with no API key yet is a valid, + // user-controlled *setup* state — not an operation failure. The desktop + // UI polls this RPC every 5 s; without this guard the mode-aware factory + // bails ("composio direct mode selected but no api key is configured") on + // every tick and the error funnels to Sentry until the user pastes a key + // (~3.2 k events, single user, release 0.57.5). Mirror `periodic.rs`'s + // graceful skip and return the truthful empty list (no key → no tenant → + // no connections). The Settings → Composio panel drives the "enter your + // key" prompt off the separate `api_key_set` status, so the user is still + // told what to do. We return BEFORE `create_composio_client` and + // `sync_cache_with_connections`, so no error is constructed and the + // integrations cache is left untouched. + if direct_mode_without_key(config)? { + tracing::debug!( + "[composio] list_connections: direct mode selected, no api key configured yet \ + — returning empty connection list (valid setup state, not an error)" + ); + return Ok(RpcOutcome::new( + ComposioConnectionsResponse { + connections: Vec::new(), + }, + vec!["composio: direct mode — no api key configured yet, 0 connection(s)".to_string()], + )); + } // Route through the mode-aware factory so direct-mode users do NOT // accidentally see the tinyhumans-tenant connections from the // backend-proxied path. Mixing the two tenants is the bug behind the diff --git a/src/openhuman/composio/ops_tests.rs b/src/openhuman/composio/ops_tests.rs index 5e7d92813..2d31701b1 100644 --- a/src/openhuman/composio/ops_tests.rs +++ b/src/openhuman/composio/ops_tests.rs @@ -1838,6 +1838,82 @@ async fn composio_list_connections_routes_through_direct_mode() { } } +// ── Direct mode with no API key yet (Sentry TAURI-RUST-R4) ──────── +// +// Direct mode selected but no key configured is a valid user *setup* +// state, not an operation failure. `composio_list_connections` must +// return an empty list (no key → no tenant → no connections) instead of +// erroring, so the desktop UI's 5 s poll stops funnelling the factory's +// "no api key is configured" error to Sentry on every tick. + +/// Direct mode selected, but NO key in the keychain and none in +/// `config.toml`. The mode-aware factory would bail here with +/// "composio direct mode selected but no api key is configured". +fn direct_mode_no_key_config(tmp: &tempfile::TempDir) -> Config { + let mut c = Config::default(); + c.workspace_dir = tmp.path().join("workspace"); + c.config_path = tmp.path().join("config.toml"); + c.composio.mode = crate::openhuman::config::schema::COMPOSIO_MODE_DIRECT.into(); + c +} + +#[test] +fn direct_mode_without_key_is_false_in_backend_mode() { + let tmp = tempfile::tempdir().unwrap(); + // Default mode is backend — the guard must never fire there, or + // backend users would get a silent empty list. + let config = test_config(&tmp); + assert!(!direct_mode_without_key(&config).unwrap()); +} + +#[test] +fn direct_mode_without_key_is_true_when_direct_and_no_key() { + let tmp = tempfile::tempdir().unwrap(); + let config = direct_mode_no_key_config(&tmp); + assert!(direct_mode_without_key(&config).unwrap()); +} + +#[test] +fn direct_mode_without_key_is_false_when_key_in_config_toml() { + let tmp = tempfile::tempdir().unwrap(); + // Key supplied via config.toml (not the keychain) still counts — + // the factory accepts it, so the guard must NOT short-circuit and + // hide the user's real connections. + let mut config = direct_mode_no_key_config(&tmp); + config.composio.api_key = Some(" ck_cfg_key ".into()); + assert!(!direct_mode_without_key(&config).unwrap()); +} + +#[test] +fn direct_mode_without_key_is_false_when_key_in_keychain() { + let tmp = tempfile::tempdir().unwrap(); + // `direct_mode_config` stores a key via the auth store — the guard + // must see it and report "has key". + let config = direct_mode_config(&tmp); + assert!(!direct_mode_without_key(&config).unwrap()); +} + +#[tokio::test] +async fn composio_list_connections_returns_empty_when_direct_mode_no_key() { + let tmp = tempfile::tempdir().unwrap(); + let config = direct_mode_no_key_config(&tmp); + // RC of TAURI-RUST-R4: this MUST be Ok(empty), not Err — no error is + // constructed, so nothing reaches Sentry and the 5 s poll stops + // churning. + let outcome = composio_list_connections(&config) + .await + .expect("direct mode with no key must return an empty list, not an error"); + assert!( + outcome.value.connections.is_empty(), + "no key → no tenant → no connections" + ); + assert!( + outcome.logs.iter().any(|l| l.contains("no api key")), + "log must explain the empty list is the no-key setup state, got {:?}", + outcome.logs + ); +} + // ── Direct-mode authorize / list_tools / execute (commit 1, #1710) ─ /// Direct-mode `composio_list_tools` now hits Composio v3 with the