feat(composio): add tags query param to GitHub tool list API#2714
Conversation
… filter pill Toolkits not in the agent-ready catalog caused max-iterations failures (tinyhumansai#2283) when the agent called composio_list_tools on them. Previously they were shown alongside curated toolkits with an amber "PREVIEW" badge but nothing stopped users from clicking them. This change hides non-agent-ready toolkits from the integrations grid by default and adds a dedicated "Preview" pill to the category filter bar so users can still browse and connect them explicitly. Toolkits with an existing connection are always visible regardless of agent-ready status so users can manage or disconnect them. - Add composioAgentReadyEntries memo to filter out preview toolkits (falls back to showing everything while the list is loading or errored) - composioFilteredEntries: when selectedCategory === 'Preview', read from composioGridEntries and invert the agent-ready check - availableCategories: add 'Preview' pill only when preview toolkits exist - Add 'Preview' to SkillCategory type and SKILL_CATEGORY_ORDER - Wire LuEye icon + amber chip style for the Preview category entry
…ests When `selectedCategory === 'Preview'` and agent-ready data is still loading or has errored, return all matching grid entries instead of an empty list (CodeRabbit tinyhumansai#2711). Adds Skills.preview-filter.test.tsx covering all 9 behaviours: default filter, connected-toolkit passthrough, Preview mode, loading/error fallback, search in both modes, and no pill when the full catalog is agent-ready.
The test was a no-op — it set agent-ready loading state but had no assertions and the title implied unreachable behavior. Now verifies that the Preview pill is absent while data loads and that the default grid remains non-empty.
- `client.rs`: `list_tools` now accepts `tags: Option<&[String]>` and appends `&tags=<csv>` to the query string alongside `toolkits` - `ops.rs`: `composio_list_tools` and `fetch_toolkit_actions` both accept a `tags` parameter; tags are only forwarded to the backend when the request is scoped to GitHub (other toolkits ignore the param to avoid unintended filtering on future expansions) - `schemas.rs`: RPC schema for `composio.list_tools` gains a `tags` input field; `handle_list_tools` reads and threads it through - `tools.rs`: agent tool schema gains a `tags` property with OR- semantics docs; extraction and GitHub gate applied at call time - `integrations.mjs`: mock tools handler now parses `tags` query param and resolves per-tag tool lists from `composioToolsByTag_<tag>` knobs (OR union, deduped); falls back to `composioTools` knob when no tags - `composio-github-tools-tags.spec.ts`: E2E spec covering single-tag filter (GT.1), OR union across two tags (GT.2), non-GitHub tag stripping (GT.3), and full agent-turn starred-repos flow (GT.4)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds an optional ChangesComposio GitHub Tag Filtering
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
M3gA-Mind
left a comment
There was a problem hiding this comment.
PR #2714 — feat(composio): add tags query param to GitHub tool list API
Walkthrough
This PR threads an optional tags query parameter from the RPC surface all the way through to the backend GET /agent-integrations/composio/tools?tags=<csv> call. Tags are silently discarded for non-GitHub toolkits (matching a backend-only constraint), applied with OR semantics by the backend (multiple tags broaden the result), and exposed both at the RPC controller level and in the agent-facing composio_list_tools tool. The change is well-contained and comes with solid unit tests in client_tests.rs and a new four-scenario E2E spec. Two call sites in the agent harness (subagent_runner/ops.rs, debug/mod.rs) were mechanically updated to pass the new None default.
Overall the implementation is correct and the test coverage is good. There are two issues worth addressing before merge: (1) tag/toolkit values are concatenated into the query string without URL encoding, which is a latent correctness hazard, and (2) the "GitHub-only" gate predicate is triplicated across three files with no central definition.
Changes
| File | Summary |
|---|---|
src/openhuman/composio/client.rs |
list_tools gains tags: Option<&[String]> — appends &tags=<csv> when non-empty |
src/openhuman/composio/ops.rs |
composio_list_tools + fetch_toolkit_actions accept tags; GitHub-only gate applied before forwarding |
src/openhuman/composio/schemas.rs |
tags field wired into openhuman.composio_list_tools controller schema |
src/openhuman/composio/tools.rs |
Agent tool exposes tags in parameters_schema(); same GitHub gate duplicated here |
src/openhuman/composio/client_tests.rs |
Comprehensive mock-server tests: tags-only, toolkits+tags, empty-tags, whitespace trimming |
scripts/mock-api/routes/integrations.mjs |
composioToolsByTag_<tag> knobs + OR-union logic for E2E tag filtering |
app/test/e2e/specs/composio-github-tools-tags.spec.ts |
New E2E spec GT.1–GT.4 covering the full tags flow |
app/test/e2e/helpers/rpc-preflight.ts |
Adds openhuman.composio_list_tools to the required-methods allowlist |
src/openhuman/agent/debug/mod.rs |
Mechanical None added to updated fetch_toolkit_actions signature |
src/openhuman/agent/harness/subagent_runner/ops.rs |
Same mechanical None for fetch_toolkit_actions |
Actionable comments (3)
⚠️ Major
1. src/openhuman/composio/client.rs:159-168 — Tag values not URL-encoded before query-string concatenation
Tag values are joined with commas and appended verbatim into the URL. If a future tag ever contains &, =, +, or non-ASCII characters, it will silently corrupt the query string or open a query-injection path. The toolkit parameter added earlier in the same function has the same gap, but adding a second unencoded parameter compounds it. list_available_triggers on line 374 of the same file correctly uses urlencoding::encode for its parameters.
Suggested change:
// before
if let Some(list) = tags {
let joined = list
.iter()
.map(|t| t.trim())
.filter(|t| !t.is_empty())
.collect::<Vec<_>>()
.join(",");
if !joined.is_empty() {
params.push(format!("tags={joined}"));
}
}
// after
if let Some(list) = tags {
let joined = list
.iter()
.map(|t| t.trim())
.filter(|t| !t.is_empty())
.map(urlencoding::encode)
.collect::<Vec<_>>()
.join(",");
if !joined.is_empty() {
params.push(format!("tags={joined}"));
}
}Apply the same fix to the toolkits block on lines 148-157.
2. src/openhuman/composio/ops.rs:687-692 + tools.rs:754-761 + ops.rs:2190-2194 — GitHub-only gate predicate triplicated
The identical check kits.iter().any(|k| k.trim().eq_ignore_ascii_case("github")) (plus the fetch_toolkit_actions variant at line 2190) appears in three separate files. When tag support expands to additional toolkits this gate will need to be updated in all three places. A central constant or helper eliminates that risk.
Suggested change:
// In src/openhuman/composio/mod.rs or a new helper at the top of ops.rs:
/// Toolkits whose Composio tag catalog is queryable via ?tags=<csv>.
/// Tags are silently dropped for every other toolkit — the backend
/// only honours this param for toolkits listed here.
const TAG_QUERYABLE_TOOLKITS: &[&str] = &["github"];
pub(super) fn toolkit_supports_tags(toolkit: &str) -> bool {
TAG_QUERYABLE_TOOLKITS
.iter()
.any(|t| toolkit.trim().eq_ignore_ascii_case(t))
}Then replace the three inline checks with calls to toolkit_supports_tags(k).
💡 Refactor / suggestion
3. app/test/e2e/specs/composio-github-tools-tags.spec.ts:321-334 — GT.4 composio-execute assertion is non-failing
The check for the composio execute call is wrapped in if (execHit) ... else console.warn(...). This means GT.4 passes even if the mock never received the execute request — the test proves the LLM replied correctly but not that the Composio action was actually dispatched through the mock backend.
Suggested change:
// before (soft check, passes even when execute wasn't mocked)
const execHit = log.find(
r => r.method === 'POST' && r.url.includes('/agent-integrations/composio/execute')
);
if (execHit) {
console.log(`${LOG_PREFIX} GT.4: composio execute confirmed`);
} else {
console.warn(`${LOG_PREFIX} GT.4: composio execute not in mock log...`);
}
// after (hard assertion)
const execHit = log.find(
r => r.method === 'POST' && r.url.includes('/agent-integrations/composio/execute')
);
expect(execHit).toBeDefined();
console.log(`${LOG_PREFIX} GT.4: composio execute confirmed`);Nitpicks (3)
app/test/e2e/specs/composio-github-tools-tags.spec.ts:1—// @ts-nochecksuppresses all type-checking across a 340-line test file. Add a comment explaining why it is needed (WDIO global injection, browser driver types, etc.) so a future reader doesn't strip it accidentally.src/openhuman/composio/schemas.rs:331— The field comment says "Case-insensitive" but case normalisation happens server-side; the Rust layer passes tags verbatim. Consider "Tags are matched case-insensitively by the backend" to avoid misleading callers.src/openhuman/composio/ops.rs:2182—fetch_toolkit_actionsnow acceptstagsbut all current callers passNone. A brief doc comment on the parameter (// Currently only honored for GitHub; future callers may pass tags for other tag-queryable toolkits.) prevents the next reviewer from wondering why the param exists.
Questions for the author (1)
src/openhuman/composio/ops.rs:2179-2182— Is there a plan to surfacetagsthrough the subagent runner's per-toolkit action discovery (run_typed_mode/render_integrations_agent)? A// TODOwould help if yes. If not, acceptingtagsinfetch_toolkit_actionswithout ever using it creates dead API surface.
Verified / looks good
client_tests.rs:305-380— The newlist_tools_filters_pass_through_as_csv_query_paramtest covers tags-only, toolkits+tags, empty-tags slice, and whitespace trimming through a real Axum mock server — solid coverage.ops.rs:687-692— The GitHub gate incomposio_list_toolscorrectly handles thetoolkits == Nonecase (tags never forwarded) and mixed toolkit lists.schemas.rs:326-333—tagsis correctly typed asOption<Array<String>>/required: falsein the controller schema.rpc-preflight.ts:30— Addingopenhuman.composio_list_toolsto the required-methods allowlist is the right thing; the method was callable before but not guarded at the contract level.- The debug/subagent call-site changes are purely mechanical
Noneadditions with no behaviour change.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
app/test/e2e/specs/composio-github-tools-tags.spec.ts (1)
95-104: ⚡ Quick winUse shared E2E helpers instead of DOM probing for the New-thread fallback.
At Line 97,
browser.executewith manual button scanning bypasses the shared cross-platform helpers and is more brittle across webview/native surfaces.Suggested refactor
-import { textExists } from '../helpers/element-helpers'; +import { clickNativeButton, textExists } from '../helpers/element-helpers'; const clicked = (await clickByTitle('New thread', 8_000)) || (await clickByTitle('New thread (/new)', 3_000)) || - (await browser.execute(() => { - const btn = Array.from(document.querySelectorAll('button')).find( - b => (b.textContent ?? '').trim() === 'New' - ) as HTMLButtonElement | undefined; - if (!btn) return false; - btn.click(); - return true; - })); + (await clickNativeButton('New'));As per coding guidelines: "Use helpers from
app/test/e2e/helpers/element-helpers.tsin E2E specs ... for cross-platform element interaction."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/test/e2e/specs/composio-github-tools-tags.spec.ts` around lines 95 - 104, The inline browser.execute DOM probe is brittle; replace that fallback with the shared E2E helper from app/test/e2e/helpers/element-helpers.ts (instead of manual DOM scanning) so cross-platform surfaces work. Concretely, remove the browser.execute(...) block and call the appropriate helper (e.g., clickByText('New', timeout) or the project’s equivalent helper) as a third OR branch after the existing clickByTitle('New thread', ...) and clickByTitle('New thread (/new)', ...), keeping the same timeouts and preserving the OR chaining. Ensure you reference and import the helper from element-helpers.ts and remove the manual button lookup in the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/test/e2e/specs/composio-github-tools-tags.spec.ts`:
- Around line 324-335: The test currently only warns when the Composio execute
mock is missing (the execHit check using log, LOG_PREFIX and the
'/agent-integrations/composio/execute' URL) which allows GT.4 to pass silently;
change this to a hard assertion so the spec fails if the side effect is absent —
replace the console.warn branch with a failing assertion (e.g., assert/expect
that execHit is truthy) and include a clear message referencing LOG_PREFIX and
the missing composio execute entry so the failure points to the backend/mock
effect.
In `@scripts/mock-api/routes/integrations.mjs`:
- Around line 349-375: The current branch applies requestedTags unconditionally
which bypasses the toolkit gate; change the initial if to only run tag-based
selection when tags were requested AND either no toolkit filter was provided or
the toolkit filter permits GitHub-style tools (e.g. requestedToolkits is empty
or requestedToolkits.some(tk => tk.toUpperCase() === 'GITHUB')). If that check
fails, fall back to the else path (parseBehaviorJson("composioTools", [])) and
then apply the existing toolkit filter on tools; update the condition that
checks requestedTags.length and reference variables/functions: requestedTags,
requestedToolkits, parseBehaviorJson, tools, knobKey.
In `@src/openhuman/composio/client.rs`:
- Around line 147-167: When building the query params in client.rs (the params
Vec created from the toolkits and tags Option lists), percent-encode each token
before joining into a CSV so reserved characters (spaces, &, =) won't corrupt
the query; in the mapping for toolkits and tags (the closures used to
trim/filter tokens) replace the direct token use with a percent-encoding call
(e.g., via url::form_urlencoded or percent_encoding) so you push
"toolkits={encoded_csv}" and "tags={encoded_csv}" and also apply the same
encoding where the final path/query is constructed (the later path assembly
around the same block).
In `@src/openhuman/composio/ops.rs`:
- Around line 687-692: The current effective_tags computation incorrectly drops
tags unless toolkits explicitly contains "github"; update the logic that sets
effective_tags so tags are preserved for tag-only requests by returning
Some(tags) when toolkits is None or when toolkits is an empty list, while still
gating tags when toolkits is present and does not include "github". Modify the
match handling around effective_tags (the variable and the tuple of &toolkits
and tags) to cover these cases: (None, Some(t)) and (Some(kits), Some(t)) if
kits.is_empty() should yield Some(t), keep the existing Some(kits) with any ==
"github" branch, and otherwise return None.
---
Nitpick comments:
In `@app/test/e2e/specs/composio-github-tools-tags.spec.ts`:
- Around line 95-104: The inline browser.execute DOM probe is brittle; replace
that fallback with the shared E2E helper from
app/test/e2e/helpers/element-helpers.ts (instead of manual DOM scanning) so
cross-platform surfaces work. Concretely, remove the browser.execute(...) block
and call the appropriate helper (e.g., clickByText('New', timeout) or the
project’s equivalent helper) as a third OR branch after the existing
clickByTitle('New thread', ...) and clickByTitle('New thread (/new)', ...),
keeping the same timeouts and preserving the OR chaining. Ensure you reference
and import the helper from element-helpers.ts and remove the manual button
lookup in the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cc9fb965-6de1-4ea5-9833-00ba77fe7753
📒 Files selected for processing (10)
app/test/e2e/helpers/rpc-preflight.tsapp/test/e2e/specs/composio-github-tools-tags.spec.tsscripts/mock-api/routes/integrations.mjssrc/openhuman/agent/debug/mod.rssrc/openhuman/agent/harness/subagent_runner/ops.rssrc/openhuman/composio/client.rssrc/openhuman/composio/client_tests.rssrc/openhuman/composio/ops.rssrc/openhuman/composio/schemas.rssrc/openhuman/composio/tools.rs
- URL-encode toolkit/tag tokens in client.rs before building query string - Extract shared `should_forward_tags` helper (pub crate) to ops.rs with TAG_QUERYABLE_TOOLKITS const; eliminates triplicated GitHub gate predicate - Apply CodeRabbit fix: forward tags when toolkits is None/empty (no-filter requests), not only when GitHub is explicit - Mirror same gate in mock integrations route (github-only or no-toolkit-filter) - Harden GT.4 composio execute assertion from warn to hard expect
Summary
tagsquery parameter to the composiolist_toolsoperation — passed as?tags=<csv>toGET /agent-integrations/composio/toolstagsthrough the full stack: Rust client →ops.rs→ RPC schema → agenttools.rsparameter schemaChanges
src/openhuman/composio/client.rslist_tools(toolkits, tags)— appends&tags=<csv>when non-emptysrc/openhuman/composio/ops.rscomposio_list_tools+fetch_toolkit_actionsaccepttags; GitHub gate appliedsrc/openhuman/composio/schemas.rstagsfield added toopenhuman.composio_list_toolscontroller schemasrc/openhuman/composio/tools.rstagsinparameters_schema(); same GitHub gatesrc/openhuman/composio/client_tests.rsscripts/mock-api/routes/integrations.mjscomposioToolsByTag_<tag>knobs with OR unionapp/test/e2e/specs/composio-github-tools-tags.spec.tsapp/test/e2e/helpers/rpc-preflight.tsopenhuman.composio_list_toolsto required RPC methodsTest plan
pnpm test:rust—client_tests.rsnew cases passpnpm test(Vitest) — no regressionstags=["stars"]returns only starred tools, mock URL has?toolkits=github&tags=starstags=["stars","repos"]returns the union (5 tools)tags=["stars"]→ tags stripped, mock URL has notags=awesome-rust?toolkits=github&tags=starsreturns ~3 tools vs ~50 without tagsSummary by CodeRabbit
New Features
Tests
Bug Fixes / Mocks
Chores