fix: persistent auth fields in Tool mode. #10230
Closed
Conversation
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
WalkthroughRefactors authentication UI/control flow in composio_base.py to clear and re-render dynamic auth fields based on connection status and mode. Integrates handling for auth_link actions, API key changes, and disconnections. Ensures build_config and tool_mode states are updated consistently. Updates update_build_config method signature. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as UI
participant CB as ComposioBase
participant TS as ToolkitSchema
participant CS as ConnectionService
rect rgba(230,240,255,0.4)
note over CB: Build config update
U->>CB: update_build_config(build_config, field_value, field_name)
CB->>CS: Check active connection
alt Active connection
CB->>TS: Clear auth fields for current build_config
CB-->>U: Return config with cleared auth UI
else No active connection
alt Tool mode managed
CB->>TS: Avoid rendering custom auth fields
else Manual mode
CB->>TS: Render custom auth fields per schema
end
CB-->>U: Return updated config
end
end
rect rgba(235,255,235,0.4)
note over CB: auth_link actions
U->>CB: action: init/validate via auth_link
CB->>TS: Clear previously rendered auth fields
CB->>CS: Start/validate connection
alt OAUTH2 flow
CS-->>CB: Connected/validated
CB->>TS: Keep auth fields cleared
CB-->>U: Updated state (connected)
else API_KEY / non-OAUTH2
CB->>TS: Render/clear fields as required
CB-->>U: Updated state with hints/auth_link
end
end
rect rgba(255,240,230,0.4)
note over CB: Changes/disconnects
U->>CB: API key change or disconnect
CB->>TS: Reset auth_mode, auth_link, action_button hints
CB->>TS: Re-render appropriate controls
CB-->>U: Consistent build_config and tool_mode state
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
github-actions
bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lfx/src/lfx/base/composio/composio_base.py (1)
1618-1633: Avoid “Disconnect” tooltip and storing connection_id on redirect flows; defer until validatedWhen initiation returns a redirect_url, the connection isn’t validated yet. Setting auth_tooltip="Disconnect" and storing connection_id at this stage is misleading. Prefer “Open link” (or “Connect”) and only store connection_id once the connection is ACTIVE or non‑redirect (immediate validation) cases.
Apply this diff to clarify state and avoid premature id storage:
- if redirect_url: - build_config["auth_link"]["value"] = redirect_url - build_config["auth_link"]["auth_tooltip"] = "Disconnect" - else: - build_config["auth_link"]["value"] = "validated" - build_config["auth_link"]["auth_tooltip"] = "Disconnect" - build_config["auth_link"]["connection_id"] = connection_id + if redirect_url: + # Not validated yet → present link action; don't store id until ACTIVE + build_config["auth_link"]["value"] = redirect_url + build_config["auth_link"]["auth_tooltip"] = "Open link" + else: + # Validated without redirect → mark connected and store id + build_config["auth_link"]["value"] = "validated" + build_config["auth_link"]["auth_tooltip"] = "Disconnect" + if connection_id: + build_config["auth_link"]["connection_id"] = connection_id # Clear auth fields when connected - schema = self._get_toolkit_schema() - self._clear_auth_fields_from_schema(build_config, schema) + schema = self._get_toolkit_schema() + self._clear_auth_fields_from_schema(build_config, schema)
🧹 Nitpick comments (2)
src/lfx/src/lfx/base/composio/composio_base.py (2)
1360-1364: Good fix clearing auth fields on ACTIVE connection; also sync auth_mode pill for consistencyClearing dynamic auth fields here is correct and aligns UI with the connected state. Recommend also updating auth_mode to reflect the connected scheme (like in the earlier branch at Lines 1290-1328) so the dropdown/pill stays consistent.
Apply this diff after clearing fields:
# Clear auth fields when connected schema = self._get_toolkit_schema() self._clear_auth_fields_from_schema(build_config, schema) + + # Reflect connected scheme in auth_mode UI + scheme, _ = self._get_connection_auth_info(connection_id) + if scheme: + build_config.setdefault("auth_link", {}) + build_config["auth_link"]["auth_scheme"] = scheme + build_config.setdefault("auth_mode", {}) + build_config["auth_mode"]["value"] = scheme + build_config["auth_mode"]["options"] = [scheme] + build_config["auth_mode"]["show"] = False + try: + pill = TabInput( + name="auth_mode", + display_name="Auth Mode", + options=[scheme], + value=scheme, + ).to_dict() + pill["show"] = True + build_config["auth_mode"] = pill + except (TypeError, ValueError, AttributeError): + build_config["auth_mode"] = { + "name": "auth_mode", + "display_name": "Auth Mode", + "type": "tab", + "options": [scheme], + "value": scheme, + "show": True, + }
1759-1778: Tool mode logic is correct; avoid duplicate ACTIVE checks to reduce API callsRendering/hiding custom auth fields in tool mode is correct. This block re‑queries for an active connection even though a similar check/routine runs earlier (Lines 1653-1714), causing extra API calls.
Use the already computed/stored state when available to skip the extra list/get:
- active_connection = self._find_active_connection_for_app(self.app_name) + # Prefer stored state from earlier checks to avoid duplicate API calls + auth_link_cfg = build_config.get("auth_link") or {} + if auth_link_cfg.get("value") == "validated" and auth_link_cfg.get("connection_id"): + active_connection = (auth_link_cfg["connection_id"], "ACTIVE") + else: + active_connection = self._find_active_connection_for_app(self.app_name)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lfx/src/lfx/base/composio/composio_base.py(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Update Starter Projects
|
Closing this PR as it's redundant. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This is an Immediate subset of 10229
Backend changes:
src/lfx/src/lfx/base/composio/composio_base.py— Persistent Auth fields in Tool mode post a valid connection is fixed in this PR.
Summary by CodeRabbit
Bug Fixes
Refactor