fix(composio): forward tags filter in direct (BYO-key) tool listing (#2772)

This commit is contained in:
sanil-23
2026-05-28 00:08:06 +05:30
committed by GitHub
parent cde5f657ec
commit d696d8264d
5 changed files with 355 additions and 29 deletions
+13 -2
View File
@@ -962,7 +962,7 @@ pub async fn direct_list_connections(
}
/// Direct-mode counterpart to [`ComposioClient::list_tools`]. Calls
/// Composio v3 `/tools?toolkits=<csv>` via
/// Composio v3 `/tools?toolkits=<csv>&tags=<a>&tags=<b>` 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<crate::openhuman::tools::ComposioTool>,
toolkits: &[String],
tags: Option<&[String]>,
) -> anyhow::Result<ComposioToolsResponse> {
let toolkit_refs: Vec<&str> = toolkits.iter().map(|s| s.as_str()).collect();
let tag_refs: Option<Vec<&str>> = 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<super::types::ComposioToolSchema> = items
.into_iter()
.filter(|item| !item.slug.is_empty())
+70 -5
View File
@@ -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<crate::openhuman::tools::ComposioTool> {
std::sync::Arc::new(crate::openhuman::tools::ComposioTool::new(
@@ -1136,6 +1139,17 @@ fn direct_tool_for_test() -> std::sync::Arc<crate::openhuman::tools::ComposioToo
))
}
/// Like [`direct_tool_for_test`] but with the v3 base pointed at a local
/// mock server so HTTP paths (e.g. `direct_list_tools`) can be asserted.
fn direct_tool_for_mock(base_v3: String) -> std::sync::Arc<crate::openhuman::tools::ComposioTool> {
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<Mutex<Option<String>>> = 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
+16 -10
View File
@@ -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
+104 -12
View File
@@ -33,6 +33,13 @@ pub struct ComposioTool {
api_key: String,
default_entity_id: String,
security: Arc<SecurityPolicy>,
/// 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<SecurityPolicy>,
) -> 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<SecurityPolicy>,
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<SecurityPolicy>,
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<Vec<ComposioToolSchemaV3>> {
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(&params);
let resp = req.send().await?;
if !resp.status().is_success() {
@@ -5,6 +5,18 @@ fn test_security() -> Arc<SecurityPolicy> {
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(&params)` 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<HashMap>`) is required because a HashMap would collapse the
// repeated `tags=` params we specifically need to assert on.
let captured: Arc<Mutex<Option<String>>> = 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]