diff --git a/src/openhuman/composio/client.rs b/src/openhuman/composio/client.rs index 12d66f0cd..9b62be845 100644 --- a/src/openhuman/composio/client.rs +++ b/src/openhuman/composio/client.rs @@ -962,7 +962,7 @@ pub async fn direct_list_connections( } /// Direct-mode counterpart to [`ComposioClient::list_tools`]. Calls -/// Composio v3 `/tools?toolkits=` via +/// Composio v3 `/tools?toolkits=&tags=&tags=` via /// [`crate::openhuman::tools::ComposioTool::list_tool_schemas_v3`] and /// reshapes each item into the same [`ComposioToolSchema`] envelope the /// backend-proxied path returns. @@ -972,6 +972,12 @@ pub async fn direct_list_connections( /// and skips schemas the agent can't actually call). `composio_list_tools`'s /// direct branch passes `direct_list_connections`'s active set. /// +/// `tags` mirrors the backend path's tag filter so a self-key user's +/// `composio_list_tools(..., tags)` request narrows by Composio action tag +/// in direct mode too (previously the tag filter was silently dropped on +/// the direct branch). The caller is expected to have already applied +/// [`super::ops::should_forward_tags`] before passing `tags` here. +/// /// Schemas surfaced here are tenant-agnostic — Composio's action /// definitions are the same across tenants, so direct-mode users get /// the same model-callable shape backend-mode does. Downstream curated- @@ -980,13 +986,18 @@ pub async fn direct_list_connections( pub(super) async fn direct_list_tools( direct: &Arc, toolkits: &[String], + tags: Option<&[String]>, ) -> anyhow::Result { let toolkit_refs: Vec<&str> = toolkits.iter().map(|s| s.as_str()).collect(); + let tag_refs: Option> = tags.map(|t| t.iter().map(|s| s.as_str()).collect()); tracing::debug!( toolkits = toolkit_refs.len(), + tags = tag_refs.as_ref().map(Vec::len).unwrap_or(0), "[composio-direct] list_tools: GET v3 /tools" ); - let items = direct.list_tool_schemas_v3(&toolkit_refs).await?; + let items = direct + .list_tool_schemas_v3(&toolkit_refs, tag_refs.as_deref()) + .await?; let tools: Vec = items .into_iter() .filter(|item| !item.slug.is_empty()) diff --git a/src/openhuman/composio/client_tests.rs b/src/openhuman/composio/client_tests.rs index 494924b4f..950017924 100644 --- a/src/openhuman/composio/client_tests.rs +++ b/src/openhuman/composio/client_tests.rs @@ -1110,13 +1110,11 @@ fn store_get_clear_composio_api_key_roundtrip() { // ── Pricing short-circuit ─────────────────────────────────────────── // ── Direct-mode reshapers (`direct_authorize` / `direct_execute` / ─ -// `direct_list_connections`) +// `direct_list_connections` / `direct_list_tools`) // // These helpers wrap a `ComposioTool` and reshape v3 responses into -// the backend-proxied envelope types. We can't easily mock the live -// `backend.composio.dev` endpoints in this unit-test layer (the -// `ComposioTool` builds its own `reqwest::Client`), so the assertions -// below verify the empty/invalid-input paths that don't require HTTP: +// the backend-proxied envelope types. Most still assert the +// empty/invalid-input paths that don't require HTTP: // // * `direct_authorize` rejects an empty toolkit before any network // hit, with an explicit error so the caller can surface it as a @@ -1127,6 +1125,11 @@ fn store_get_clear_composio_api_key_roundtrip() { // * `direct_list_connections` is a thin mapper; the real coverage // for its row → ComposioConnection translation lives in the // `connected_account_*` tests in `composio_tests.rs`. +// +// `direct_list_tools` IS exercised over HTTP: `ComposioTool::new_with_v3_base` +// points its `/tools` GET at a local axum mock, so we can assert both the +// outbound `tags` filter (repeated query params) and the v3 → backend-envelope +// reshape without touching `backend.composio.dev`. fn direct_tool_for_test() -> std::sync::Arc { std::sync::Arc::new(crate::openhuman::tools::ComposioTool::new( @@ -1136,6 +1139,17 @@ fn direct_tool_for_test() -> std::sync::Arc std::sync::Arc { + std::sync::Arc::new(crate::openhuman::tools::ComposioTool::new_with_v3_base( + "ck_test_direct", + Some("default"), + std::sync::Arc::new(crate::openhuman::security::SecurityPolicy::default()), + base_v3, + )) +} + #[tokio::test] async fn direct_authorize_rejects_empty_toolkit() { let tool = direct_tool_for_test(); @@ -1149,6 +1163,57 @@ async fn direct_authorize_rejects_empty_toolkit() { ); } +#[tokio::test] +async fn direct_list_tools_forwards_tags_and_reshapes_v3_envelope() { + use axum::extract::RawQuery; + use std::sync::Mutex; + + let captured: Arc>> = Arc::new(Mutex::new(None)); + let sink = captured.clone(); + let app = Router::new().route( + "/tools", + get(move |RawQuery(q): RawQuery| { + let sink = sink.clone(); + async move { + *sink.lock().unwrap() = q; + Json(json!({ + "items": [ + { + "slug": "GITHUB_STAR_A_REPOSITORY", + "description": "Star a repository", + "input_parameters": { "type": "object" }, + "toolkit": { "slug": "github" } + }, + // Empty-slug rows must be dropped by the reshaper. + { "slug": "", "description": "junk" } + ] + })) + } + }), + ); + let base = start_mock_backend(app).await; + let tool = direct_tool_for_mock(base); + + let resp = super::direct_list_tools( + &tool, + &["github".to_string()], + Some(&["stars".to_string(), "repos".to_string()]), + ) + .await + .expect("direct_list_tools should succeed against the mock"); + + // Outbound: tags forwarded as repeated params, toolkits CSV. + let query = captured.lock().unwrap().clone().expect("server saw query"); + assert!(query.contains("tags=stars"), "query was: {query}"); + assert!(query.contains("tags=repos"), "query was: {query}"); + assert!(query.contains("toolkits=github"), "query was: {query}"); + + // Inbound: reshaped into the backend envelope, empty-slug row dropped. + assert_eq!(resp.tools.len(), 1); + assert_eq!(resp.tools[0].function.name, "GITHUB_STAR_A_REPOSITORY"); + assert_eq!(resp.tools[0].kind, "function"); +} + #[tokio::test] async fn pricing_for_config_short_circuits_in_direct_mode() { // Build a client pointed at an unreachable backend — if the diff --git a/src/openhuman/composio/ops.rs b/src/openhuman/composio/ops.rs index e90240ae1..70b47db69 100644 --- a/src/openhuman/composio/ops.rs +++ b/src/openhuman/composio/ops.rs @@ -787,18 +787,24 @@ pub async fn composio_list_tools( } tracing::debug!( toolkits = scope.len(), + ?effective_tags, "[composio-direct] list_tools: fetching v3 tool schemas" ); - 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 - })?; + // Forward the same `effective_tags` the backend branch uses so the + // tag filter is honoured in direct (BYO-key) mode too — previously + // it was computed above and then dropped on this branch. + let mut resp = direct_list_tools(&direct, &scope, effective_tags.as_deref()) + .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 diff --git a/src/openhuman/tools/impl/network/composio.rs b/src/openhuman/tools/impl/network/composio.rs index 26fc49013..b597ac06c 100644 --- a/src/openhuman/tools/impl/network/composio.rs +++ b/src/openhuman/tools/impl/network/composio.rs @@ -33,6 +33,13 @@ pub struct ComposioTool { api_key: String, default_entity_id: String, security: Arc, + /// Base URL for Composio v3 endpoints (`{base}/tools`). Production + /// always uses [`COMPOSIO_API_BASE_V3`] via [`Self::new`]; the + /// `#[cfg(test)]` `new_with_v3_base` constructor lets unit tests point + /// the direct-mode `/tools` listing at a local axum mock — the same + /// base-URL injection the backend `ComposioClient` gets through + /// `IntegrationClient::new` in `client_tests.rs`. + base_v3: String, } impl ComposioTool { @@ -40,6 +47,43 @@ impl ComposioTool { api_key: &str, default_entity_id: Option<&str>, security: Arc, + ) -> Self { + // Production always pins the real HTTPS v3 endpoint. + Self::new_internal( + api_key, + default_entity_id, + security, + COMPOSIO_API_BASE_V3.to_string(), + ) + } + + /// Test-only seam: construct with an explicit Composio v3 base URL so + /// unit tests can point the direct `/tools` request — including the + /// `tags` filter — at a local mock instead of `backend.composio.dev`. + /// + /// `#[cfg(test)]`-gated on purpose: `list_tool_schemas_v3` attaches the + /// `x-api-key` header to whatever `base_v3` holds, so the only way to + /// reach the v3 endpoint in production is [`Self::new`], which always + /// uses the HTTPS [`COMPOSIO_API_BASE_V3`] const. An injectable base must + /// never carry a non-HTTPS URL outside tests. + #[cfg(test)] + pub(crate) fn new_with_v3_base( + api_key: &str, + default_entity_id: Option<&str>, + security: Arc, + base_v3: String, + ) -> Self { + Self::new_internal(api_key, default_entity_id, security, base_v3) + } + + /// Shared constructor body. Private so the injectable `base_v3` cannot be + /// supplied by production callers — they go through [`Self::new`] (real + /// HTTPS const) and tests through the `#[cfg(test)]` `new_with_v3_base`. + fn new_internal( + api_key: &str, + default_entity_id: Option<&str>, + security: Arc, + base_v3: String, ) -> Self { let trimmed = api_key.trim(); if trimmed.len() != api_key.len() { @@ -59,6 +103,7 @@ impl ComposioTool { api_key: trimmed.to_string(), default_entity_id: normalize_entity_id(default_entity_id.unwrap_or("default")), security, + base_v3, } } @@ -134,6 +179,44 @@ impl ComposioTool { Ok(body.items) } + /// Build the query-parameter pairs for the Composio v3 `GET /tools` + /// listing used by [`Self::list_tool_schemas_v3`]. + /// + /// `toolkits` is sent as a single comma-joined `toolkits=` param (the + /// legacy plural the v3 backend tolerates; cf. `list_actions_v3` which + /// sends both the plural and `toolkit_slug` singular forms). `tags` is + /// encoded as **repeated** `tags=` params (`tags=a&tags=b`) — the shape + /// Composio v3 `/tools` documents for tag filtering ("can be specified + /// multiple times"), NOT the comma-joined form the backend proxy uses. + /// Blank entries are trimmed and dropped; an empty `tags` slice yields + /// no `tags` params (treated as no filter). + /// + /// Pure (no I/O) so the param shape is unit-testable without a live + /// HTTP round trip — mirrors [`Self::build_execute_action_v3_request`]. + fn build_list_tool_schemas_v3_query( + toolkits: &[&str], + tags: Option<&[&str]>, + ) -> Vec<(&'static str, String)> { + let mut params: Vec<(&'static str, String)> = vec![("limit", "200".to_string())]; + + let trimmed: Vec<&str> = toolkits + .iter() + .map(|t| t.trim()) + .filter(|t| !t.is_empty()) + .collect(); + if !trimmed.is_empty() { + params.push(("toolkits", trimmed.join(","))); + } + + if let Some(tags) = tags { + for tag in tags.iter().map(|t| t.trim()).filter(|t| !t.is_empty()) { + params.push(("tags", tag.to_string())); + } + } + + params + } + /// List v3 tool definitions for one or more toolkits, preserving the /// raw `input_parameters` JSON schema each action carries. /// @@ -149,22 +232,31 @@ impl ComposioTool { /// catalogue scan. Empty filter returns every action across every /// toolkit on the user's tenant (potentially large; callers should /// pass a non-empty filter in practice). + /// + /// `tags` narrows the result by Composio action tag (OR semantics — + /// multiple tags broaden the result). This is the direct-mode (BYO + /// key) counterpart to the backend proxy's `tags` query param wired + /// in [`crate::openhuman::composio::client::ComposioClient::list_tools`]; + /// without it a self-key user's `composio_list_tools(..., tags)` + /// request would silently drop the tag filter. Blank/empty `tags` + /// are treated as no filter. pub(crate) async fn list_tool_schemas_v3( &self, toolkits: &[&str], + tags: Option<&[&str]>, ) -> anyhow::Result> { - let url = format!("{COMPOSIO_API_BASE_V3}/tools"); - let mut req = self.client().get(&url).header("x-api-key", &self.api_key); - req = req.query(&[("limit", "200")]); - let trimmed: Vec<&str> = toolkits - .iter() - .map(|t| t.trim()) - .filter(|t| !t.is_empty()) - .collect(); - if !trimmed.is_empty() { - let csv = trimmed.join(","); - req = req.query(&[("toolkits", csv.as_str())]); - } + let url = format!("{}/tools", self.base_v3); + let params = Self::build_list_tool_schemas_v3_query(toolkits, tags); + tracing::debug!( + toolkits = toolkits.len(), + tags = tags.map(<[&str]>::len).unwrap_or(0), + "[composio-direct] list_tool_schemas_v3: GET v3 /tools query built" + ); + let req = self + .client() + .get(&url) + .header("x-api-key", &self.api_key) + .query(¶ms); let resp = req.send().await?; if !resp.status().is_success() { diff --git a/src/openhuman/tools/impl/network/composio_tests.rs b/src/openhuman/tools/impl/network/composio_tests.rs index be6dbe455..b1a9b82da 100644 --- a/src/openhuman/tools/impl/network/composio_tests.rs +++ b/src/openhuman/tools/impl/network/composio_tests.rs @@ -5,6 +5,18 @@ fn test_security() -> Arc { Arc::new(SecurityPolicy::default()) } +/// Spawn a throwaway axum mock bound to an ephemeral port and return its base +/// URL. Mirrors `start_mock_backend` in `client_tests.rs` so both HTTP-level +/// direct-mode tests share one setup model. +async fn start_mock_backend(app: axum::Router) -> String { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + format!("http://127.0.0.1:{}", addr.port()) +} + // ── Constructor ─────────────────────────────────────────── #[test] @@ -336,6 +348,146 @@ fn build_execute_action_v3_request_drops_blank_optional_fields() { assert!(body.get("user_id").is_none()); } +// ── list_tool_schemas_v3 query builder (direct-mode tags) ────────────────── + +#[test] +fn build_list_tool_schemas_v3_query_always_includes_limit() { + let params = ComposioTool::build_list_tool_schemas_v3_query(&[], None); + assert_eq!(params, vec![("limit", "200".to_string())]); +} + +#[test] +fn build_list_tool_schemas_v3_query_joins_toolkits_as_csv() { + let params = ComposioTool::build_list_tool_schemas_v3_query(&["github", "gmail"], None); + assert_eq!( + params, + vec![ + ("limit", "200".to_string()), + ("toolkits", "github,gmail".to_string()), + ] + ); +} + +#[test] +fn build_list_tool_schemas_v3_query_emits_repeated_tags_params() { + // Composio v3 `/tools` takes tags as repeated `tags=` params + // (tags=stars&tags=repos), NOT comma-joined like the backend proxy. + // A Vec of duplicate ("tags", _) keys is exactly what reqwest's + // `.query(¶ms)` serializes into repeated query params. + let params = + ComposioTool::build_list_tool_schemas_v3_query(&["github"], Some(&["stars", "repos"])); + assert_eq!( + params, + vec![ + ("limit", "200".to_string()), + ("toolkits", "github".to_string()), + ("tags", "stars".to_string()), + ("tags", "repos".to_string()), + ] + ); +} + +#[test] +fn build_list_tool_schemas_v3_query_tags_without_toolkit_filter() { + let params = ComposioTool::build_list_tool_schemas_v3_query(&[], Some(&["readOnlyHint"])); + assert_eq!( + params, + vec![ + ("limit", "200".to_string()), + ("tags", "readOnlyHint".to_string()), + ] + ); +} + +#[test] +fn build_list_tool_schemas_v3_query_trims_and_drops_blank_entries() { + let params = ComposioTool::build_list_tool_schemas_v3_query( + &[" github ", " "], + Some(&[" stars ", "", " "]), + ); + assert_eq!( + params, + vec![ + ("limit", "200".to_string()), + ("toolkits", "github".to_string()), + ("tags", "stars".to_string()), + ] + ); +} + +#[test] +fn build_list_tool_schemas_v3_query_empty_tags_slice_is_no_filter() { + // `Some(&[])` and an all-blank slice must both behave like "no tags". + let empty = ComposioTool::build_list_tool_schemas_v3_query(&["gmail"], Some(&[])); + let blank = ComposioTool::build_list_tool_schemas_v3_query(&["gmail"], Some(&[" "])); + let expected = vec![ + ("limit", "200".to_string()), + ("toolkits", "gmail".to_string()), + ]; + assert_eq!(empty, expected); + assert_eq!(blank, expected); +} + +// ── list_tool_schemas_v3 over HTTP (direct-mode tags reach the wire) ─────── + +#[tokio::test] +async fn list_tool_schemas_v3_sends_repeated_tags_to_v3_tools_endpoint() { + use axum::{extract::RawQuery, routing::get, Json, Router}; + use std::sync::Mutex; + + // Capture the raw query string the server sees. `RawQuery` (not + // `Query`) is required because a HashMap would collapse the + // repeated `tags=` params we specifically need to assert on. + let captured: Arc>> = Arc::new(Mutex::new(None)); + let sink = captured.clone(); + let app = Router::new().route( + "/tools", + get(move |RawQuery(q): RawQuery| { + let sink = sink.clone(); + async move { + *sink.lock().unwrap() = q; + Json(json!({ + "items": [{ + "slug": "GITHUB_STAR_A_REPOSITORY", + "description": "Star a repository", + "input_parameters": { "type": "object" }, + "toolkit": { "slug": "github" } + }] + })) + } + }), + ); + let base = start_mock_backend(app).await; + + let tool = ComposioTool::new_with_v3_base("ck_test_direct", None, test_security(), base); + let items = tool + .list_tool_schemas_v3(&["github"], Some(&["stars", "repos"])) + .await + .expect("direct v3 /tools should succeed against the mock"); + + let query = captured + .lock() + .unwrap() + .clone() + .expect("mock server should have observed a query string"); + + // tags must be REPEATED params (tags=stars&tags=repos) — the Composio v3 + // contract — NOT the comma-joined form the backend proxy uses. + assert!(query.contains("tags=stars"), "query was: {query}"); + assert!(query.contains("tags=repos"), "query was: {query}"); + assert!( + !query.contains("stars%2Crepos") && !query.contains("stars,repos"), + "tags must not be comma-joined; query was: {query}" + ); + assert!(query.contains("toolkits=github"), "query was: {query}"); + assert!(query.contains("limit=200"), "query was: {query}"); + + // And the v3 envelope reshapes back into schema items. + assert_eq!(items.len(), 1); + assert_eq!(items[0].slug, "GITHUB_STAR_A_REPOSITORY"); + assert_eq!(items[0].toolkit_slug.as_deref(), Some("github")); +} + // ── ensure_https ────────────────────────────────────────────────────────── #[test]