feat: pick active connection only

[himanshu-dixit]

🔍 Review Summary

The update involves a minor enhancement in base.toolset.ts by adding a filter for active connections, ensuring minimal impact on the overall codebase.

Original Description

None

---

Important

Enhance ComposioToolSet.executeAction to filter and use only active connected accounts.

• Behavior:
  • In ComposioToolSet.executeAction, filters connected accounts by status: "ACTIVE" and showActiveOnly: true to ensure only active accounts are considered.
• Misc:
  • No other files or functions are affected by this change.

^{This description was created by for 9c07c50. It will automatically update as commits are pushed.}

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
composio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 3, 2025 11:29am

[entelligence-ai-pr-reviews]

Walkthrough

The PR introduces a minor change by adding two lines to filter active connections in base.toolset.ts. This does not significantly impact the codebase.

🔗 Related PRs

• feat: update types with BE and release rc for v0.5 #1082: The pull request involves refactoring backend types and updating API methods to meet new specifications, including renaming functions and enhancing models with new error types and DTOs.
• fix: bring eslint + TS issues to zero #1013: The pull request aims to enhance code quality and type safety through stricter ESLint rules, code refactoring, and improved error handling, resulting in a more maintainable and robust codebase.
• feat: Support requesting connected_account in custom action #1090: The update adds support for the connected_account parameter in custom actions within Composio, allowing retrieval of user-specific access tokens, such as for Gmail, through modifications in runtime.py.

Entelligence.ai can learn from your feedback. Simply add 👍 / 👎 emojis to teach it your preferences. More shortcuts below

Emoji Descriptions:

• ⚠️ Potential Issue - May require further investigation.
• 🔒 Security Vulnerability - Fix to ensure system safety.
• 💻 Code Improvement - Suggestions to enhance code quality.
• 🔨 Refactor Suggestion - Recommendations for restructuring code.
• ℹ️ Others - General comments and information.

Interact with the Bot:

• Send a message or request using the format:
  @bot + *your message*

Example: @bot Can you suggest improvements for this code?

• Help the Bot learn by providing feedback on its responses.
  @bot + *feedback*

Example: @bot Do not comment on `save_auth` function !

[ellipsis-dev]

❌ Changes requested. Reviewed everything up to 9c07c50 in 50 seconds

More details

• Looked at 13 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_ype1LxFe5WAzp85D

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

The status: "ACTIVE" parameter is redundant when showActiveOnly: true is used. Consider removing status: "ACTIVE".

[github-actions]

This comment was generated by github-actions[bot]!

JS SDK Coverage Report

📊 Coverage report for JS SDK can be found at the following URL:
https://pub-92e668239ab84bfd80ee07d61e9d2f40.r2.dev/coverage-12597009954/coverage/index.html

📁 Test report folder can be found at the following URL:
https://pub-92e668239ab84bfd80ee07d61e9d2f40.r2.dev/html-report-12597009954/html-report/report.html

[shreysingla11]

There appears to be redundancy in the parameters. Both status: "ACTIVE" and showActiveOnly: true seem to serve the same purpose. Consider using just one of these parameters to maintain cleaner code. Additionally, the string literal "ACTIVE" should be defined as a constant for better maintainability.

[shreysingla11]

The code assumes there will be active accounts by directly accessing items[0]. Consider adding explicit error handling for cases where no active accounts are found. For example:

if (!connectedAccounts?.items?.length) {
  throw CEG.getCustomError(
    COMPOSIO_SDK_ERROR_CODES.SDK.NO_ACTIVE_ACCOUNT_FOUND,
    {
      message: "No active connected account found for the user",
      description: "Please ensure user has at least one active connected account",
    }
  );
}

[shreysingla11]

Code Review Summary

The changes introduce filtering for active accounts, which is a good addition, but there are a few areas that could be improved:

1. Parameter Redundancy: The PR adds both status: "ACTIVE" and showActiveOnly: true which appear to serve the same purpose. Consider using just one parameter.

2. Error Handling: The code could benefit from explicit error handling when no active accounts are found.

3. Code Quality Improvements:

   • Define "ACTIVE" as a constant
   • Add error handling for no active accounts case
   • Add documentation explaining the active account filtering

Suggested implementation:

const ACCOUNT_STATUS = {
  ACTIVE: 'ACTIVE'
} as const;

// Fetch connected account id, prioritizing active accounts
const connectedAccounts = await this.client.connectedAccounts.list({
  user_uuid: entityId,
  status: ACCOUNT_STATUS.ACTIVE,
});

if (!connectedAccounts?.items?.length) {
  throw CEG.getCustomError(
    COMPOSIO_SDK_ERROR_CODES.SDK.NO_ACTIVE_ACCOUNT_FOUND,
    {
      message: "No active connected account found for the user",
      description: "Please ensure user has at least one active connected account",
    }
  );
}

Overall, while the changes are moving in the right direction, addressing these points would make the code more robust and maintainable.