mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(composio): empty connections in direct mode without api key (TAURI-RUST-R4) (#3198)
This commit is contained in:
@@ -233,7 +233,17 @@ pub fn expected_error_kind(message: &str) -> Option<ExpectedErrorKind> {
|
||||
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.
|
||||
|
||||
@@ -84,6 +84,40 @@ fn resolve_client(config: &Config) -> OpResult<ComposioClient> {
|
||||
})
|
||||
}
|
||||
|
||||
/// 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<bool> {
|
||||
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<RpcOutcome<ComposioConnectionsResponse>> {
|
||||
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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user