Refactor workspace tools to not take workspace id from agent

kaavee315 commented 6 days ago

PR Type

Enhancement, Bug fix

Description

Added execute command to actions group for executing actions with parameters.
Replaced _get_enum_key with get_enum_key from composio.utils.enums across multiple files.
Added ExecutionEnvironment and Env classes to handle different execution environments.
Modified LocalToolHandler to initialize and execute actions based on the execution environment.
Simplified tools initialization by removing old toolset code and importing ComposioToolSet.
Added execute_action method to DockerWorkspace and abstract method in base_workspace.
Modified create_workspace in workspace_factory to return Workspace object instead of ID.

Changes walkthrough 📝

Relevant files

Enhancement

13 files

actions.py `Add execute command and replace enum key function` python/composio/cli/actions.py Added `execute` command to `actions` group for executing actions with parameters. Replaced `_get_enum_key` with `get_enum_key` from `composio.utils.enums`. Added JSON parsing for action parameters.	+29/-8
apps.py `Replace local enum key function with utility function` python/composio/cli/apps.py Replaced `_get_enum_key` with `get_enum_key` from `composio.utils`.	+8/-16
context.py `Initialize LocalToolHandler in Context client` python/composio/cli/context.py - Added `LocalToolHandler` initialization in `Context.client`.	+2/-0
__init__.py `Add local handler parameter to Composio client` python/composio/client/__init__.py Added `local_handler` parameter to `Composio` client initialization.	+3/-1
base.py `Add local handler parameter to Collection initialization` python/composio/client/base.py - Added `local_handler` parameter to `Collection` initialization.	+5/-1
collections.py `Add local handler check in Actions get method` python/composio/client/collections.py - Added check for `local_handler` in `Actions.get` method.	+5/-1
local_handler.py `Add execution environment handling in LocalToolHandler` python/composio/client/local_handler.py Added `ExecutionEnvironment` and `Env` classes. Added workspace initialization based on execution environment. Modified `execute_action` to handle different execution environments.	+61/-2
__init__.py `Simplify tools initialization` python/composio/tools/__init__.py - Removed old toolset code and imported `ComposioToolSet`.	+1/-288
__init__.py `Add get_enum_key import` python/composio/utils/__init__.py - Added import for `get_enum_key`.	+2/-0
base_workspace.py `Add execute_action abstract method` python/composio/workspace/base_workspace.py - Added abstract method `execute_action`.	+5/-0
docker_workspace.py `Add execute_action method in DockerWorkspace` python/composio/workspace/docker_workspace.py Added `execute_action` method to handle action execution in Docker environment.	+13/-0
workspace_clients.py `Add LOCAL to WorkspaceType enum` python/composio/workspace/workspace_clients.py - Added `LOCAL` to `WorkspaceType` enum.	+1/-0
workspace_factory.py `Modify create_workspace to return Workspace object` python/composio/workspace/workspace_factory.py Modified `create_workspace` to return `Workspace` object instead of ID.	+2/-2

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

ellipsis-dev[bot] commented 6 days ago

Your free trial has expired. To keep using Ellipsis, sign up at https://app.ellipsis.dev for $20/seat/month or reach us at help@ellipsis.dev

codiumai-pr-agent-pro[bot] commented 6 days ago

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review

**Possible Bug:** The `execute` command in `actions.py` may not handle exceptions from `context.client.actions.execute` properly. It catches `ComposioSDKError` but does not handle other potential exceptions that could arise from the action execution logic. **Code Duplication:** The method `get_enum_key` is used across multiple files to replace special characters in strings with underscores and convert them to uppercase. This logic is repeated in several places and could be centralized to reduce redundancy and improve maintainability. **Error Handling:** In `local_handler.py`, the method `execute_action` checks the execution environment and executes actions differently based on it. However, the error handling seems inconsistent, and there might be cases where errors are not properly propagated or logged, leading to difficulties in debugging.

codiumai-pr-agent-pro[bot] commented 6 days ago

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Add null checks for `local_handler` to prevent runtime errors ___ Add a check to ensure `local_handler` is not `None` before using it in the `Actions` class to prevent `AttributeError`. [python/composio/client/collections.py [865-867]](https://github.com/ComposioHQ/composio/pull/248/files#diff-4bae1393a4b5e1f07b5b87855fed9d56811d949225a47921cad5d4cc75c71e13R865-R867) ```diff +if self.local_handler is None: + raise ValueError("Local handler is not initialized.") local_items = self.local_handler.get_action_schemas( apps=local_apps, actions=local_actions, tags=tags ) ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 10 Why: Adding a null check for `local_handler` is essential to prevent potential `AttributeError` at runtime, ensuring the code is more robust and error-resistant.	10
Error handling	Improve error handling in Docker command executions ___ Add error handling for Docker command execution failures to provide more detailed error information and to prevent the application from crashing. [python/composio/workspace/docker_workspace.py [272-274]](https://github.com/ComposioHQ/composio/pull/248/files#diff-6ef3ed19ef240f6524ab638b04172966193130fb5fa9e669d16dfaa0c7aa33c1R272-R274) ```diff if communicate_resp.return_code != 0: + logger.error(f"Failed to execute Docker command: {communicate_resp.output}") return { "execute_action": {"success": False, "error": communicate_resp.output} } ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 9 Why: Adding error logging for Docker command failures provides more detailed error information, which is crucial for debugging and preventing the application from crashing.	9
Error handling	✅ Improve JSON parsing error handling in the CLI command ___ Suggestion Impact: The commit added error handling for JSON parsing in the execute command, which aligns with the suggestion to provide a more user-friendly error message. code diff: ```diff -@_actions.command(name="execute") -@click.argument("action_name", type=str) -@click.option("--params", "-p", type=str, help="Action parameters as a JSON string") -@pass_context -def execute(context: Context, action_name: str, params: str) -> None: - """Execute a Composio action""" - try: - # Parse JSON parameters - action_params = json.loads(params) if params else {} - # Execute the action - result = context.client.actions.execute(Action(action_name), action_params) - context.console.print(result) - - except json.JSONDecodeError: - raise click.ClickException("Invalid JSON format for parameters") - except ComposioSDKError as e: - raise click.ClickException(message=e.message) from e ``` ___ Add error handling for JSON parsing in the `execute` command to provide a more user-friendly error message. [python/composio/cli/actions.py [153]](https://github.com/ComposioHQ/composio/pull/248/files#diff-4302aa45d73b49ed2882648d340a05ad8c10d38eea026c8eafe37d40512364e7R153-R153) ```diff -action_params = json.loads(params) if params else {} +try: + action_params = json.loads(params) if params else {} +except json.JSONDecodeError: + raise click.ClickException("Invalid JSON format for parameters. Please check your input.") ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 8 Why: Adding error handling for JSON parsing in the CLI command provides a more user-friendly error message, improving the user experience and making it easier to diagnose input issues.	8
Maintainability	Replace hardcoded default values with constants or configuration settings ___ Replace the hardcoded string for the default image name in the `LocalDockerArgumentsModel` with a constant or a configuration value. This will make the code more maintainable and flexible. [python/composio/client/local_handler.py [75]](https://github.com/ComposioHQ/composio/pull/248/files#diff-9ef7ad39289b51d9166ef93a943f9c8987c1f30ab207b7854f5d94259bb5a33aR75-R75) ```diff -image_name=local_tools_env.image_name or "" +image_name=local_tools_env.image_name or DEFAULT_DOCKER_IMAGE ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 8 Why: Replacing hardcoded values with constants or configuration settings improves maintainability and flexibility, making the code easier to update and manage.	8

codiumai-pr-agent-pro[bot] commented 6 days ago

CI Failure Feedback 🧐

(Checks updated until commit https://github.com/ComposioHQ/composio/commit/51426ccd38e994664b9b8d2fa4561d95ab631913)

**Action:** test (ubuntu-latest, 3.9)

**Failed stage:** [Unittests](https://github.com/ComposioHQ/composio/actions/runs/9797416431/job/27054006548) [❌]

**Failed test name:** composio/tools/local/shelltool/tests/test_workspace.py

**Failure summary:** The action failed because the test composio/tools/local/shelltool/tests/test_workspace.py
encountered an ImportError.

The specific error was: ImportError: cannot import name 'ExecutionEnvironment' from
'composio.tools.env.factory'.

This indicates that the ExecutionEnvironment class or function is either missing or not correctly
defined in the composio.tools.env.factory module.

Relevant error logs:

```yaml 1: ##[group]Operating System 2: Ubuntu ... 495: * [new branch] featembed-tool -> origin/featembed-tool 496: * [new branch] fix/readme -> origin/fix/readme 497: * [new branch] fix/readme-logo -> origin/fix/readme-logo 498: * [new branch] fix/swe-agent -> origin/fix/swe-agent 499: * [new branch] ft-add-better-help-text -> origin/ft-add-better-help-text 500: * [new branch] ft-apps-id -> origin/ft-apps-id 501: * [new branch] ft-bring-back-core-sdk -> origin/ft-bring-back-core-sdk 502: * [new branch] ft-did-you-mean -> origin/ft-did-you-mean 503: * [new branch] ft-error-tracking -> origin/ft-error-tracking ... 880: ✔ Actions updated 881: ⚠️ Triggers does not require update 882: unittests: commands[1]> pytest -vvv -rfE --doctest-modules composio/ tests/ --cov=composio --cov=examples --cov-report=html --cov-report=xml --cov-report=term --cov-report=term-missing --cov-config=.coveragerc 883: ============================= test session starts ============================== 884: platform linux -- Python 3.9.19, pytest-7.4.2, pluggy-1.5.0 -- /home/runner/work/composio/composio/python/.tox/unittests/bin/python 885: cachedir: .tox/unittests/.pytest_cache 886: rootdir: /home/runner/work/composio/composio/python 887: plugins: codecov-0.5.1, anyio-4.4.0, cov-5.0.0 888: collecting ... collected 44 items / 2 errors 889: ==================================== ERRORS ==================================== 890: ___ ERROR collecting composio/tools/local/shelltool/tests/test_workspace.py ____ ... 905: :986: in _find_and_load_unlocked 906: ??? 907: :680: in _load_unlocked 908: ??? 909: .tox/unittests/lib/python3.9/site-packages/_pytest/assertion/rewrite.py:178: in exec_module 910: exec(co, module.__dict__) 911: composio/tools/local/shelltool/tests/test_workspace.py:6: in 912: from composio.tools.env.factory import ExecutionEnvironment, WorkspaceFactory 913: E ImportError: cannot import name 'ExecutionEnvironment' from 'composio.tools.env.factory' (/home/runner/work/composio/composio/python/composio/tools/env/factory.py) 914: ___ ERROR collecting composio/tools/local/shelltool/tests/test_workspace.py ____ 915: ImportError while importing test module '/home/runner/work/composio/composio/python/composio/tools/local/shelltool/tests/test_workspace.py'. ... 928: :986: in _find_and_load_unlocked 929: ??? 930: :680: in _load_unlocked 931: ??? 932: .tox/unittests/lib/python3.9/site-packages/_pytest/assertion/rewrite.py:178: in exec_module 933: exec(co, module.__dict__) 934: composio/tools/local/shelltool/tests/test_workspace.py:6: in 935: from composio.tools.env.factory import ExecutionEnvironment, WorkspaceFactory 936: E ImportError: cannot import name 'ExecutionEnvironment' from 'composio.tools.env.factory' (/home/runner/work/composio/composio/python/composio/tools/env/factory.py) ... 1080: composio/utils/shared.py 117 104 11% 43-83, 99-108, 139-143, 153-158, 174-221, 247-292, 324-337 1081: composio/utils/url.py 10 6 40% 19, 24-35 1082: examples/crewai_ci_chart.py 14 14 0% 1-38 1083: -------------------------------------------------------------------------------------------------------------- 1084: TOTAL 7793 1706 78% 1085: Coverage HTML written to dir htmlcov 1086: Coverage XML written to file coverage.xml 1087: =========================== short test summary info ============================ 1088: ERROR composio/tools/local/shelltool/tests/test_workspace.py - ImportError: cannot import name 'ExecutionEnvironment' from 'composio.tools.env.factory' (/home/runner/work/composio/composio/python/composio/tools/env/factory.py) 1089: ERROR composio/tools/local/shelltool/tests/test_workspace.py 1090: !!!!!!!!!!!!!!!!!!! Interrupted: 2 errors during collection !!!!!!!!!!!!!!!!!!!! 1091: ========================= 1 warning, 2 errors in 4.49s ========================= 1092: unittests: exit 2 (5.78 seconds) /home/runner/work/composio/composio/python> pytest -vvv -rfE --doctest-modules composio/ tests/ --cov=composio --cov=examples --cov-report=html --cov-report=xml --cov-report=term --cov-report=term-missing --cov-config=.coveragerc pid=5596 1093: .pkg: _exit> python /opt/hostedtoolcache/Python/3.9.19/x64/lib/python3.9/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__ 1094: unittests: FAIL code 2 (29.98=setup[19.81]+cmd[4.38,5.78] seconds) 1095: evaluation failed :( (30.41 seconds) 1096: ##[error]Process completed with exit code 2. ```

✨ CI feedback usage guide:

The CI feedback tool (`/checks)` automatically triggers when a PR has a failed check. The tool analyzes the failed checks and provides several feedbacks: - Failed stage - Failed test name - Failure summary - Relevant error logs In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR: ``` /checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}" ``` where `{repo_name}` is the name of the repository, `{run_number}` is the run number of the failed check, and `{job_number}` is the job number of the failed check. #### Configuration options - `enable_auto_checks_feedback` - if set to true, the tool will automatically provide feedback when a check is failed. Default is true. - `excluded_checks_list` - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list. - `enable_help_text` - if set to true, the tool will provide a help message with the feedback. Default is true. - `persistent_comment` - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true. - `final_update_message` - if `persistent_comment` is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true. See more information about the `checks` tool in the [docs](https://pr-agent-docs.codium.ai/tools/ci_feedback/).

ComposioHQ / composio