From 3002211a07915a2b351fae2fa63cbffab67ae8d9 Mon Sep 17 00:00:00 2001 From: Thiago Moretto Date: Mon, 2 Feb 2026 11:40:56 -0300 Subject: [PATCH] fix: Use sha256 instead of md5 for module name hashing (lint S324) - Add missing mocks to metadata extraction failure test --- lib/crewai/src/crewai/cli/utils.py | 107 +++++++++++++----------- lib/crewai/tests/cli/tools/test_main.py | 32 ++++++- 2 files changed, 84 insertions(+), 55 deletions(-) diff --git a/lib/crewai/src/crewai/cli/utils.py b/lib/crewai/src/crewai/cli/utils.py index a774094b8..87ebc8775 100644 --- a/lib/crewai/src/crewai/cli/utils.py +++ b/lib/crewai/src/crewai/cli/utils.py @@ -1,4 +1,5 @@ from collections.abc import Mapping +from contextlib import contextmanager from functools import lru_cache, reduce import hashlib import importlib.util @@ -547,43 +548,58 @@ def build_env_with_tool_repository_credentials( return env +@contextmanager +def _load_module_from_file(init_file: Path, module_name: str | None = None): + """ + Context manager for loading a module from file with automatic cleanup. + + Yields the loaded module or None if loading fails. + """ + if module_name is None: + module_name = f"temp_module_{hashlib.sha256(str(init_file).encode()).hexdigest()[:8]}" + + spec = importlib.util.spec_from_file_location(module_name, init_file) + if not spec or not spec.loader: + yield None + return + + module = importlib.util.module_from_spec(spec) + sys.modules[module_name] = module + + try: + spec.loader.exec_module(module) + yield module + finally: + sys.modules.pop(module_name, None) + + def _load_tools_from_init(init_file: Path) -> list[dict[str, Any]]: """ Load and validate tools from a given __init__.py file. """ - spec = importlib.util.spec_from_file_location("temp_module", init_file) - - if not spec or not spec.loader: - return [] - - module = importlib.util.module_from_spec(spec) - sys.modules["temp_module"] = module - try: - spec.loader.exec_module(module) + with _load_module_from_file(init_file) as module: + if module is None: + return [] - if not hasattr(module, "__all__"): - console.print( - f"Warning: No __all__ defined in {init_file}", - style="bold yellow", - ) - raise SystemExit(1) - - return [ - { - "name": name, - } - for name in module.__all__ - if hasattr(module, name) and is_valid_tool(getattr(module, name)) - ] + if not hasattr(module, "__all__"): + console.print( + f"Warning: No __all__ defined in {init_file}", + style="bold yellow", + ) + raise SystemExit(1) + return [ + {"name": name} + for name in module.__all__ + if hasattr(module, name) and is_valid_tool(getattr(module, name)) + ] + except SystemExit: + raise except Exception as e: console.print(f"[red]Warning: Could not load {init_file}: {e!s}[/red]") raise SystemExit(1) from e - finally: - sys.modules.pop("temp_module", None) - def _print_no_tools_warning() -> None: """ @@ -642,41 +658,30 @@ def _extract_tool_metadata_from_init(init_file: Path) -> list[dict[str, Any]]: """ from crewai.tools.base_tool import BaseTool - module_name = f"temp_metadata_{hashlib.sha256(str(init_file).encode()).hexdigest()[:8]}" - spec = importlib.util.spec_from_file_location(module_name, init_file) - - if not spec or not spec.loader: - return [] - - module = importlib.util.module_from_spec(spec) - sys.modules[module_name] = module - try: - spec.loader.exec_module(module) + with _load_module_from_file(init_file) as module: + if module is None: + return [] - exported_names = getattr(module, "__all__", None) - if not exported_names: - return [] + exported_names = getattr(module, "__all__", None) + if not exported_names: + return [] - tools_metadata = [] - for name in exported_names: - obj = getattr(module, name, None) - if obj is None or not (inspect.isclass(obj) and issubclass(obj, BaseTool)): - continue - if tool_info := _extract_single_tool_metadata(obj): - tools_metadata.append(tool_info) - - return tools_metadata + tools_metadata = [] + for name in exported_names: + obj = getattr(module, name, None) + if obj is None or not (inspect.isclass(obj) and issubclass(obj, BaseTool)): + continue + if tool_info := _extract_single_tool_metadata(obj): + tools_metadata.append(tool_info) + return tools_metadata except Exception as e: console.print( f"[yellow]Warning: Could not extract metadata from {init_file}: {e}[/yellow]" ) return [] - finally: - sys.modules.pop(module_name, None) - def _extract_single_tool_metadata(tool_class: type) -> dict[str, Any] | None: """ diff --git a/lib/crewai/tests/cli/tools/test_main.py b/lib/crewai/tests/cli/tools/test_main.py index 3ffab377a..aba6f1075 100644 --- a/lib/crewai/tests/cli/tools/test_main.py +++ b/lib/crewai/tests/cli/tools/test_main.py @@ -387,6 +387,14 @@ def test_publish_api_error( @patch("crewai.cli.tools.main.get_project_name", return_value="sample-tool") @patch("crewai.cli.tools.main.get_project_version", return_value="1.0.0") @patch("crewai.cli.tools.main.get_project_description", return_value="A sample tool") +@patch("crewai.cli.tools.main.subprocess.run") +@patch("crewai.cli.tools.main.os.listdir", return_value=["sample-tool-1.0.0.tar.gz"]) +@patch( + "crewai.cli.tools.main.open", + new_callable=unittest.mock.mock_open, + read_data=b"sample tarball content", +) +@patch("crewai.cli.plus_api.PlusAPI.publish_tool") @patch("crewai.cli.tools.main.git.Repository.is_synced", return_value=True) @patch( "crewai.cli.tools.main.extract_available_exports", @@ -400,6 +408,10 @@ def test_publish_metadata_extraction_failure_continues_with_warning( mock_tools_metadata, mock_available_exports, mock_is_synced, + mock_publish, + mock_open, + mock_listdir, + mock_subprocess_run, mock_get_project_description, mock_get_project_version, mock_get_project_name, @@ -407,14 +419,26 @@ def test_publish_metadata_extraction_failure_continues_with_warning( tool_command, ): """Test that metadata extraction failure shows warning but continues publishing.""" - try: - tool_command.publish(is_public=True) - except SystemExit: - pass # May fail later due to API mock, but should get past metadata extraction + mock_publish_response = MagicMock() + mock_publish_response.status_code = 200 + mock_publish_response.json.return_value = {"handle": "sample-tool"} + mock_publish.return_value = mock_publish_response + + tool_command.publish(is_public=True) + output = capsys.readouterr().out assert "Warning: Could not extract tool metadata" in output assert "Publishing will continue without detailed metadata" in output assert "No tool metadata extracted" in output + mock_publish.assert_called_once_with( + handle="sample-tool", + is_public=True, + version="1.0.0", + description="A sample tool", + encoded_file=unittest.mock.ANY, + available_exports=[{"name": "SampleTool"}], + tools_metadata=[], + ) @patch("crewai.cli.tools.main.Settings")