Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
148 changes: 148 additions & 0 deletions src/core/observability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,42 @@ fn is_provider_user_state_message(lower: &str) -> bool {
return true;
}

// TAURI-RUST-X9 (#1166): direct-mode composio call against the user's
// personal Composio v3 tenant rejected with a 401 because the stored
// API key is invalid / revoked / has the wrong prefix. The canonical
// wire shape rendered by
// `src/openhuman/composio/tools/impl/network/composio.rs::response_error`
// and the various direct-mode op wrappers is:
//
// `[composio-direct] list_connections failed: Composio v3
// connected_accounts failed: HTTP 401: Invalid API key: ak_…`
//
// The "Invalid API key" body is rendered for every direct-mode
// endpoint (list_connections / list_tools / authorize / etc.), so we
// gate on the **`[composio-direct]` prefix** + either of the two
// anchors that prove the failure came from the v3 auth wall:
// - `HTTP 401` (the status the v3 wall returns)
// - `Invalid API key` (the body Composio puts in the JSON)
//
// Requiring the `[composio-direct]` prefix keeps this from
// accidentally swallowing unrelated bugs — backend-mode 401s from
// `integrations/composio/*` still carry the `Backend returned 401`
// shape (handled by the failure-tag flow with `status="401"`),
// not the `HTTP 401: Invalid API key` shape.
//
// Remediation is purely user-state: the user must rotate / re-enter
// their Composio key via Settings → Composio → Direct mode. Sentry
// has no actionable signal — the UI surfaces the "Invalid API key"
// toast and the polling layer already retries every 5 s.
//
// Drops Sentry TAURI-RUST-X9 (~15.7 k events / ~22 h, single user,
// release openhuman@0.54.0+c25fc8e5fd3e).
if lower.contains("[composio-direct]")
&& (lower.contains("http 401") || lower.contains("invalid api key"))
{
return true;
}

false
}

Expand Down Expand Up @@ -2004,6 +2040,118 @@ mod tests {
);
}

// ── TAURI-RUST-X9 (#1166): composio-direct 401 / Invalid API key ────

#[test]
fn classifies_composio_direct_invalid_api_key_as_provider_user_state() {
// Canonical Sentry TAURI-RUST-X9 wire shape — the verbatim title
// body from the issue, captured 15,732 times in ~22h on a single
// user with a bad direct-mode key. The classifier must demote
// this to `ProviderUserState` so the polling layer's 5 s retry
// doesn't keep flooding Sentry.
let msg = "[composio-direct] list_connections failed: \
Composio v3 connected_accounts failed: \
HTTP 401: Invalid API key: ak_VsUvq*****";
assert_eq!(
expected_error_kind(msg),
Some(ExpectedErrorKind::ProviderUserState),
"composio-direct HTTP 401 + Invalid API key must demote to ProviderUserState"
);
}

#[test]
fn classifies_composio_direct_invalid_api_key_for_other_ops() {
// Same arm must cover every op-name the direct branches emit —
// not just `list_connections`. The matcher gates on the
// `[composio-direct]` prefix, not on a specific op string, so
// `list_tools` / `authorize` / `list_connections` all demote.
let shapes = [
// list_tools prefetch fails before the actual list_tools call
"[composio-direct] list_tools: prefetch connections failed: \
Composio v3 connected_accounts failed: HTTP 401: Invalid API key: ak_…",
// direct authorize hits the v3 /connected_accounts/link wall
"[composio-direct] authorize failed: \
Composio v3 connected_accounts/link failed: HTTP 401: Invalid API key: ak_…",
// direct list_tools itself
"[composio-direct] list_tools failed: \
Composio v3 tools failed: HTTP 401: Invalid API key: ak_…",
// periodic-tick rendering (no "[composio-direct]" prefix because
// periodic.rs wraps differently, but the failure still gets the
// hook — handled by ops.rs's report path, not the
// expected_error_kind body shape, so we only verify the
// composio-direct branch here)
];
for msg in shapes {
assert_eq!(
expected_error_kind(msg),
Some(ExpectedErrorKind::ProviderUserState),
"every [composio-direct] op with HTTP 401 / Invalid API key must demote: {msg}"
);
}
}

#[test]
fn classifies_composio_direct_with_invalid_api_key_only_no_http_401() {
// The matcher accepts EITHER `HTTP 401` OR `Invalid API key`
// alongside the `[composio-direct]` prefix. Catches the wire
// shape variant where the body anchor lands but the status text
// is rendered differently (e.g. "401 Unauthorized" instead of
// "HTTP 401") — same user-state condition.
let msg = "[composio-direct] list_connections failed: \
Composio v3 connected_accounts failed: \
401 Unauthorized: Invalid API key: ak_…";
assert_eq!(
expected_error_kind(msg),
Some(ExpectedErrorKind::ProviderUserState),
"composio-direct + Invalid API key body must demote even without literal 'HTTP 401'"
);
}

#[test]
fn does_not_classify_unrelated_http_401_as_composio_direct_user_state() {
// Discrimination test: a generic 401 that does NOT carry the
// `[composio-direct]` prefix must NOT match this arm. This
// protects against the arm accidentally swallowing backend-mode
// composio 401s, unrelated integration 401s, or any other
// 401-containing message that lacks the direct-mode anchor.
//
// The backend-mode shape is `Backend returned 401 …`; it does
// not contain `[composio-direct]`, so the new arm rightly skips
// it. Backend-mode 401s remain a real Sentry signal (bad
// service-to-service auth, expired token, etc.).
let backend_401 = "[composio] list_connections failed: \
Backend returned 401 Unauthorized for GET \
https://api.tinyhumans.ai/agent-integrations/composio/connections: \
Invalid API key";
assert_ne!(
expected_error_kind(backend_401),
Some(ExpectedErrorKind::ProviderUserState),
"backend-mode 401 must NOT demote via the composio-direct arm"
);

let unrelated_401 = "GitHub API error: HTTP 401: Bad credentials";
assert_ne!(
expected_error_kind(unrelated_401),
Some(ExpectedErrorKind::ProviderUserState),
"unrelated 401 (no [composio-direct] anchor) must NOT match the composio-direct arm"
);
}

#[test]
fn does_not_classify_composio_direct_500_as_user_state() {
// Real bug shapes — a 500 from the direct v3 path with no auth
// body anchor — must still fall through to `None` so Sentry
// sees them. Without this guard the arm could be too permissive
// and silence genuine backend faults.
let msg = "[composio-direct] list_connections failed: \
Composio v3 connected_accounts failed: HTTP 500";
assert_eq!(
expected_error_kind(msg),
None,
"composio-direct 500 with no auth body must NOT demote — it is a real bug shape"
);
}

#[test]
fn classifies_local_ai_binary_missing_errors() {
// OPENHUMAN-TAURI-9N: `local_ai_tts` returns this exact string
Expand Down
57 changes: 48 additions & 9 deletions src/openhuman/composio/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ fn resolve_client(config: &Config) -> OpResult<ComposioClient> {
/// handshake eof`, …), we tag `failure="transport"` instead so the
/// `before_send` filter's transport-phrase branch fires — and keep the
/// status tag absent (transport failures don't carry a status).
fn report_composio_op_error<E: std::fmt::Display + ?Sized>(operation: &str, err: &E) {
pub(super) fn report_composio_op_error<E: std::fmt::Display + ?Sized>(operation: &str, err: &E) {
// `{err:#}` renders the full anyhow chain when applicable; for plain
// `String` / `&str` errors it falls back to the Display impl.
let rendered = format!("{err:#}");
Expand Down Expand Up @@ -243,9 +243,22 @@ pub async fn composio_list_connections(
"[composio-direct] list_connections: fetching v3 \
/connected_accounts for the user's personal Composio tenant"
);
let resp = direct_list_connections(&direct)
.await
.map_err(|e| format!("[composio-direct] list_connections failed: {e:#}"))?;
let resp = direct_list_connections(&direct).await.map_err(|e| {
// [#1166 / Sentry TAURI-RUST-X9] Restore symmetric error
// routing for the direct-mode branch. Without this hook the
// direct-mode 401 ("Invalid API key …") wire shape bypassed
// `report_error_or_expected` and leaked ~15.7k events in ~22h
// — same UI 5 s poll + `periodic.rs` tick that the
// backend branch (line ~266) was already classifying.
//
// Render WITH the `[composio-direct]` anchor BEFORE
// reporting so the classifier arm in
// `is_provider_user_state_message` (which gates on that
// prefix) actually fires.
let rendered = format!("[composio-direct] list_connections failed: {e:#}");
report_composio_op_error("list_connections", &rendered);
rendered
})?;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
let active = resp.connections.iter().filter(|c| c.is_active()).count();
let total = resp.connections.len();
// Reconcile the integrations cache against this fresh live
Expand Down Expand Up @@ -338,7 +351,16 @@ pub async fn composio_authorize(
.await
.map_err(|e| {
let wrapped = super::oauth_handoff::wrap_authorize_rate_limit_error(toolkit, e);
format!("[composio-direct] authorize failed: {wrapped:#}")
// [#1166 / Sentry TAURI-RUST-X9] Symmetric with the
// backend branch's `report_composio_op_error` on the
// same handler — direct-mode 401s from
// `connected_accounts/link` were leaking otherwise.
// Render WITH the `[composio-direct]` anchor so the
// classifier arm fires; wrapped error preserves any
// rate-limit classifications fed up the ladder.
let rendered = format!("[composio-direct] authorize failed: {wrapped:#}");
report_composio_op_error("authorize", &rendered);
rendered
})?
}
};
Expand Down Expand Up @@ -480,7 +502,17 @@ pub async fn composio_list_tools(
Some(list) if !list.is_empty() => list,
_ => {
let conns = direct_list_connections(&direct).await.map_err(|e| {
format!("[composio-direct] list_tools: prefetch connections failed: {e:#}")
// [#1166 / Sentry TAURI-RUST-X9] Symmetric error
// routing — the prefetch call goes to the same v3
// `/connected_accounts` endpoint as `list_connections`
// and would emit the same 401 wire shape. Render
// WITH the `[composio-direct]` anchor so the
// classifier arm fires on the prefetch path too.
let rendered = format!(
"[composio-direct] list_tools: prefetch connections failed: {e:#}"
);
report_composio_op_error("list_connections", &rendered);
rendered
})?;
let mut v: Vec<String> = conns
.connections
Expand Down Expand Up @@ -510,9 +542,16 @@ pub async fn composio_list_tools(
toolkits = scope.len(),
"[composio-direct] list_tools: fetching v3 tool schemas"
);
let mut resp = direct_list_tools(&direct, &scope)
.await
.map_err(|e| format!("[composio-direct] list_tools failed: {e:#}"))?;
let mut resp = direct_list_tools(&direct, &scope).await.map_err(|e| {
// [#1166 / Sentry TAURI-RUST-X9] Symmetric with the backend
// branch's hook (line ~451). Direct-mode `list_tools`
// failures are user-state when the API key is bad. Render
// WITH the `[composio-direct]` anchor so the classifier
// arm fires.
let rendered = format!("[composio-direct] list_tools failed: {e:#}");
report_composio_op_error("list_tools", &rendered);
rendered
})?;
// Apply the same curated-whitelist + user-scope filter the
// backend path runs — schemas may be tenant-agnostic but
// OpenHuman's curation policy isn't, and direct-mode users
Expand Down
82 changes: 82 additions & 0 deletions src/openhuman/composio/ops_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1619,3 +1619,85 @@ fn composio_transport_timeout_is_dropped_by_before_send() {
"composio transport timeout must be dropped by integrations filter (#1608)"
);
}

// ── TAURI-RUST-X9 (#1166): direct-mode auth-rejection routing ───────────
//
// Pins the contract that direct-mode 401 / Invalid API key shapes are
// classified by the observability matcher AND their failure-tag stays
// `non_2xx` so the `before_send` integrations filter has consistent
// inputs. Together with the classifier-arm tests in
// `core::observability` these tests prove the leak path (~15.7 k events
// in ~22h before #1166) is closed end-to-end.

#[test]
fn composio_direct_invalid_api_key_classifies_as_provider_user_state() {
// The verbatim Sentry TAURI-RUST-X9 wire shape — emitted by
// `ops.rs::composio_list_connections` direct branch via the
// `report_composio_op_error` hook restored in #1166. Routing this
// through `expected_error_kind` is what demotes it to
// `ProviderUserState` (info breadcrumb) instead of firing a Sentry
// event.
let msg = "[composio-direct] list_connections failed: \
Composio v3 connected_accounts failed: \
HTTP 401: Invalid API key: ak_VsUvq*****";
assert_eq!(
crate::core::observability::expected_error_kind(msg),
Some(crate::core::observability::ExpectedErrorKind::ProviderUserState),
"the canonical TAURI-RUST-X9 wire shape must demote via the composio-direct arm"
);
}

#[test]
fn composio_direct_invalid_api_key_failure_tag_is_non_2xx() {
// Belt-and-suspenders: even if `expected_error_kind` ever stops
// matching the body (regression in the classifier arm), the
// failure tag must STILL be `non_2xx`. Combined with the
// `before_send` filter's transient-status handling and a
// future-added `status="401"` tag (Patch 1 doesn't extract status
// from the `HTTP 401` shape — only the `Backend returned <status>`
// shape — so this just pins the safe default), this is the
// backstop drop path.
let rendered = "[composio-direct] list_connections failed: \
Composio v3 connected_accounts failed: \
HTTP 401: Invalid API key: ak_VsUvq*****";
assert_eq!(
classify_composio_failure_tag(rendered),
"non_2xx",
"direct-mode auth-rejection must tag as non_2xx (not transport)"
);
}

#[test]
fn composio_direct_invalid_api_key_extract_status_returns_none() {
// Pins the contract: `extract_backend_returned_status` only parses
// the integrations-layer `Backend returned <status>` rendering, NOT
// the direct-mode `HTTP 401` shape. The direct-mode arm relies on
// the classifier demotion + the failure-tag drop path instead of
// status extraction; if this ever changes (e.g. we extend the
// status extractor to cover both shapes), the new behaviour should
// come with an explicit test, not be inferred.
let rendered = "[composio-direct] list_connections failed: \
Composio v3 connected_accounts failed: \
HTTP 401: Invalid API key: ak_…";
assert_eq!(
extract_backend_returned_status(rendered),
None,
"direct-mode HTTP 401 must not parse via extract_backend_returned_status"
);
}

#[test]
fn composio_direct_500_does_not_demote() {
// Discrimination test from the composio side — a real bug shape
// (500 with no auth body) MUST escape the classifier and reach
// `report_error_message`. Without this guard the matcher in
// `observability.rs` could be tightened too far and silence
// genuine backend faults.
let msg = "[composio-direct] list_connections failed: \
Composio v3 connected_accounts failed: HTTP 500";
assert_eq!(
crate::core::observability::expected_error_kind(msg),
None,
"composio-direct 500 with no auth body must remain an unclassified bug shape"
);
}
19 changes: 16 additions & 3 deletions src/openhuman/composio/periodic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,22 @@ pub(crate) async fn run_one_tick() -> Result<(), String> {
.list_connections()
.await
.map_err(|e| format!("list_connections (backend): {e}"))?,
ComposioClientKind::Direct(direct) => direct_list_connections(direct)
.await
.map_err(|e| format!("list_connections (direct): {e:#}"))?,
ComposioClientKind::Direct(direct) => {
direct_list_connections(direct).await.map_err(|e| {
// [#1166 / Sentry TAURI-RUST-X9] The server-side periodic
// tick re-renders the same v3 `/connected_accounts` 401
// shape that `ops::composio_list_connections` emits, so
// route it through the observability classifier too.
// Without this, the tick-side 401s leak as unclassified
// Sentry events even when the UI poll's identical failure
// is correctly classified. Render WITH the
// `[composio-direct]` anchor so the classifier arm in
// `is_provider_user_state_message` actually fires.
let rendered = format!("[composio-direct] list_connections (direct): {e:#}");
super::ops::report_composio_op_error("list_connections", &rendered);
rendered
})?
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
};

let sync_map = last_sync_map();
Expand Down
15 changes: 14 additions & 1 deletion src/openhuman/composio/tools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,20 @@ impl Tool for ComposioListConnectionsTool {
Ok(ComposioClientKind::Direct(direct)) => {
tracing::debug!("[composio-direct] list_connections.execute: direct variant");
direct_list_connections(&direct).await.map_err(|e| {
anyhow::anyhow!("composio_list_connections (direct) failed: {e}")
// [#1166 / Sentry TAURI-RUST-X9] Symmetric error
// routing with `ops.rs::composio_list_connections`.
// The agent-tool path can also fire 401s when a
// direct-mode user has a bad API key — without this
// hook the failure escapes the classifier and lands
// as an unclassified Sentry event. Render WITH the
// `[composio-direct]` anchor BEFORE reporting so the
// classifier arm in `is_provider_user_state_message`
// (gated on that prefix) actually fires.
let rendered = format!(
"[composio-direct] composio_list_connections (direct) failed: {e:#}"
);
super::ops::report_composio_op_error("list_connections", &rendered);
anyhow::anyhow!("{rendered}")
Comment thread
coderabbitai[bot] marked this conversation as resolved.
})?
}
Err(e) => {
Expand Down
Loading