fix(composio): collect WABA ID before WhatsApp Business OAuth flow#1550
Open
YellowSnnowmann wants to merge 7 commits into
Open
fix(composio): collect WABA ID before WhatsApp Business OAuth flow#1550YellowSnnowmann wants to merge 7 commits into
YellowSnnowmann wants to merge 7 commits into
Conversation
…nd update authorization flow
…ameters for toolkit integration
…od across components
… parameters for toolkit
Contributor
📝 WalkthroughWalkthroughThreads optional ChangesWhatsApp Business OAuth with WABA ID Support
Sequence DiagramsequenceDiagram
participant Modal as ComposioConnectModal
participant API as composioApi.authorize
participant RpcHandler as handle_authorize
participant Ops as composio_authorize
participant Client as ComposioClient::authorize
Modal->>API: authorize(toolkit, extraParams)
API->>RpcHandler: JSON-RPC { toolkit, extra_params }
RpcHandler->>Ops: composio_authorize(toolkit, extra_params)
Ops->>Client: client.authorize(toolkit, extra_params)
Client->>Client: validate & merge extra_params into POST body
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/composio/schemas.rs (1)
600-606:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep
authorizeschema metadata in sync with handler params.
handle_authorizenow acceptsextra_params, but theauthorizecontroller schema still only declarestoolkit. This can break schema-driven clients/docs.🤖 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 `@src/openhuman/composio/schemas.rs` around lines 600 - 606, The controller schema for "authorize" is out of sync with the handler: handle_authorize accepts an optional extra_params but the schema only declares toolkit. Update the authorize controller schema metadata to add an optional "extra_params" parameter (matching the handler's type, e.g. a JSON object/Map<String, Value> or serde_json::Value) so schema-driven clients/docs reflect the actual handler signature; ensure the parameter is marked optional/nullable and its description explains it forwards additional auth params to composio_authorize.
🧹 Nitpick comments (1)
src/openhuman/composio/client_tests.rs (1)
176-197: ⚡ Quick winAdd one authorize test that asserts
extra_paramsforwarding.The new behavior is only partially covered right now (
Nonepath). Please add a mock test asserting fields likewaba_idare posted whenSome(...)is provided.🤖 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 `@src/openhuman/composio/client_tests.rs` around lines 176 - 197, Add a new tokio test similar to authorize_posts_toolkit_and_returns_connect_url that calls client.authorize with Some(extra_params) and asserts forwarding of those params; in the mock handler (the closure registered for "/agent-integrations/composio/authorize") parse the incoming JSON body and assert body["extra_params"]["waba_id"] (and any other expected keys) equals the value you passed (e.g., "waba-123"), then return the same success JSON with connectUrl and connectionId; name the test (e.g., authorize_forwards_extra_params_and_returns_connect_url) and verify the response via client.authorize returns the expected connect_url and connection_id as in the existing 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 `@src/openhuman/composio/client.rs`:
- Around line 84-89: The merge logic that inserts keys from extra_params into
the root JSON (the block using body.as_object_mut(), extra.as_object(), and
iterating extra_obj) must not allow callers to override reserved keys (e.g.,
"toolkit"); update that merge to skip or reject any keys in a reserved set
(e.g., "toolkit", "toolkit_version", "auth", "client_id") before calling
obj.insert. Locate the block that uses extra_params/extra_obj and either filter
extra_obj entries by checking key names against the reserved set or return an
error when a reserved key is present so reserved fields remain unchanged and
cannot be overridden.
---
Outside diff comments:
In `@src/openhuman/composio/schemas.rs`:
- Around line 600-606: The controller schema for "authorize" is out of sync with
the handler: handle_authorize accepts an optional extra_params but the schema
only declares toolkit. Update the authorize controller schema metadata to add an
optional "extra_params" parameter (matching the handler's type, e.g. a JSON
object/Map<String, Value> or serde_json::Value) so schema-driven clients/docs
reflect the actual handler signature; ensure the parameter is marked
optional/nullable and its description explains it forwards additional auth
params to composio_authorize.
---
Nitpick comments:
In `@src/openhuman/composio/client_tests.rs`:
- Around line 176-197: Add a new tokio test similar to
authorize_posts_toolkit_and_returns_connect_url that calls client.authorize with
Some(extra_params) and asserts forwarding of those params; in the mock handler
(the closure registered for "/agent-integrations/composio/authorize") parse the
incoming JSON body and assert body["extra_params"]["waba_id"] (and any other
expected keys) equals the value you passed (e.g., "waba-123"), then return the
same success JSON with connectUrl and connectionId; name the test (e.g.,
authorize_forwards_extra_params_and_returns_connect_url) and verify the response
via client.authorize returns the expected connect_url and connection_id as in
the existing 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: fd1c1a78-3e34-4f31-a2b1-2eb62766c0ce
📒 Files selected for processing (8)
app/src/components/composio/ComposioConnectModal.tsxapp/src/lib/composio/composioApi.tssrc/openhuman/composio/client.rssrc/openhuman/composio/client_tests.rssrc/openhuman/composio/ops.rssrc/openhuman/composio/ops_tests.rssrc/openhuman/composio/schemas.rssrc/openhuman/composio/tools.rs
… add extra_params test - client.rs: block all four reserved keys (toolkit, toolkit_version, auth, client_id) instead of only toolkit - schemas.rs: add optional extra_params: Json field to the authorize ControllerSchema so schema-driven docs reflect the handler signature - client_tests.rs: add authorize_forwards_extra_params_and_returns_connect_url test that verifies waba_id is forwarded in the POST body Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openhuman/composio/client_tests.rs (1)
199-223: ⚡ Quick winAdd negative tests for the new
authorizeguards.You now test the happy path for forwarded
waba_id; please also pin the two new failure branches (non-object extra_params, reserved-key override) to avoid silent regressions.Suggested test additions
+#[tokio::test] +async fn authorize_rejects_non_object_extra_params() { + let inner = Arc::new(crate::openhuman::integrations::IntegrationClient::new( + "http://127.0.0.1:0".into(), + "test".into(), + )); + let client = ComposioClient::new(inner); + let err = client + .authorize("whatsapp", Some(json!("waba-123"))) + .await + .unwrap_err(); + assert!(err.to_string().contains("extra_params must be a JSON object")); +} + +#[tokio::test] +async fn authorize_rejects_reserved_extra_params_key() { + let inner = Arc::new(crate::openhuman::integrations::IntegrationClient::new( + "http://127.0.0.1:0".into(), + "test".into(), + )); + let client = ComposioClient::new(inner); + let err = client + .authorize("whatsapp", Some(json!({ "toolkit": "gmail" }))) + .await + .unwrap_err(); + assert!(err.to_string().contains("cannot override reserved key")); +}🤖 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 `@src/openhuman/composio/client_tests.rs` around lines 199 - 223, Add two negative tests to complement authorize_forwards_extra_params_and_returns_connect_url: (1) a test that calls client.authorize with extra_params set to a non-object (e.g., a string or array) and asserts the mock route returns a 400/error and the client surfaces that as an Err; (2) a test that passes an object in extra_params that contains the reserved key "toolkit" (or other reserved param) and asserts the server rejects it (mock handler should assert the override attempt and return an error) and the client returns an Err. Use the same mock route setup pattern used in authorize_forwards_extra_params_and_returns_connect_url and name the tests clearly (e.g., authorize_rejects_non_object_extra_params and authorize_rejects_reserved_key_override) so they pin the two failure branches.
🤖 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.
Nitpick comments:
In `@src/openhuman/composio/client_tests.rs`:
- Around line 199-223: Add two negative tests to complement
authorize_forwards_extra_params_and_returns_connect_url: (1) a test that calls
client.authorize with extra_params set to a non-object (e.g., a string or array)
and asserts the mock route returns a 400/error and the client surfaces that as
an Err; (2) a test that passes an object in extra_params that contains the
reserved key "toolkit" (or other reserved param) and asserts the server rejects
it (mock handler should assert the override attempt and return an error) and the
client returns an Err. Use the same mock route setup pattern used in
authorize_forwards_extra_params_and_returns_connect_url and name the tests
clearly (e.g., authorize_rejects_non_object_extra_params and
authorize_rejects_reserved_key_override) so they pin the two failure branches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 426bdc0e-048b-43b0-aea5-e12cba8db42a
📒 Files selected for processing (3)
src/openhuman/composio/client.rssrc/openhuman/composio/client_tests.rssrc/openhuman/composio/schemas.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/openhuman/composio/schemas.rs
Pin the non-object and reserved-key failure branches so regressions are caught immediately without requiring a mock backend. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
graycyrus
reviewed
May 12, 2026
Contributor
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Review Summary
Clean, well-layered fix — the generic extra_params plumbing is the right call. Four findings below (2 major, 2 minor). All CodeRabbit items were already addressed — nice work.
graycyrus
reviewed
May 12, 2026
Contributor
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Review — 3 findings (1 major, 2 minor)
Clean, well-layered fix. The generic extra_params plumbing is the right design call, and the reserved-key guard + tests are solid. All prior CodeRabbit items were already addressed. Three items below.
| extraParams?: Record<string, string> | ||
| ): Promise<ComposioAuthorizeResponse> { | ||
| const raw = await callCoreRpc<unknown>({ | ||
| method: 'openhuman.composio_authorize', |
Contributor
There was a problem hiding this comment.
[major] extraParams type is narrower than Rust accepts
This is typed Record<string, string> but the Rust side accepts Option<serde_json::Value> — any JSON value. The schema also declares TypeSchema::Json. For forward compat (and type-level consistency across the stack), consider widening to Record<string, unknown>.
Suggested change
| method: 'openhuman.composio_authorize', | |
| extraParams?: Record<string, unknown> |
| @@ -73,6 +73,9 @@ export default function ComposioConnectModal({ | |||
| ); | |||
| const [error, setError] = useState<string | null>(null); | |||
| const [connectUrl, setConnectUrl] = useState<string | null>(null); | |||
Contributor
There was a problem hiding this comment.
[minor] wabaId not reset on disconnect
If a user connects WhatsApp, disconnects, then reconnects in the same modal session, the old WABA ID is pre-populated without any indication it was carried over. Consider clearing it in handleDisconnect:
setWabaId('');| ], | ||
| outputs: vec![ | ||
| FieldSchema { | ||
| name: "connectUrl", |
Contributor
There was a problem hiding this comment.
[minor] Typo in schema comment — mismatched paren
The embedded JSON example closes with ) instead of }:
(e.g. {"waba_id": "...") for toolkits ← paren
Should be:
(e.g. {"waba_id": "..."} for toolkits ← brace
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
POST /agent-integrations/composio/authorizefor thewhatsapptoolkit requires awaba_idfield (WhatsApp Business Account ID). The previous call only sent{ toolkit }, which Composio rejected with a 400ConnectedAccount_MissingRequiredFields.extra_paramsmap through the full authorize stack (Rustclient.rs→ops.rs→schemas.rs→composioApi.ts), and add a WABA ID input toComposioConnectModalthat surfaces only for thewhatsapptoolkit.extra_paramsdefaults toNone/undefined).Changed files:
src/openhuman/composio/client.rs—authorize()acceptsOption<serde_json::Value>extra params, merges into POST bodysrc/openhuman/composio/ops.rs— forwardsextra_paramsto clientsrc/openhuman/composio/schemas.rs— reads optionalextra_paramsfrom RPC callapp/src/lib/composio/composioApi.ts—authorize()accepts optionalextraParams, serializes asextra_paramsapp/src/components/composio/ComposioConnectModal.tsx— shows WABA ID input forwhatsapptoolkit; validates non-empty before calling authorizeTest plan
Manually testedpnpm typecheckpasses cleanSummary by CodeRabbit
New Features
Improvements
Tests