From 1086d5d70d049eabaec3fde6fc01751cd1835105 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 4 Apr 2026 13:01:25 +0000 Subject: [PATCH] fix: rename loop variable to avoid shadowing provider parameter in create_crew() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #5270. The for-loop iterating over ENV_VARS used 'provider' as its loop variable, which shadowed the 'provider' function parameter. Renamed the loop variable to 'env_provider' so the original parameter value is preserved throughout the function. Added three regression tests to verify the fix. Co-Authored-By: João --- lib/crewai/src/crewai/cli/create_crew.py | 4 +- lib/crewai/tests/cli/test_create_crew.py | 131 +++++++++++++++++++++++ 2 files changed, 133 insertions(+), 2 deletions(-) diff --git a/lib/crewai/src/crewai/cli/create_crew.py b/lib/crewai/src/crewai/cli/create_crew.py index 9bca7c499..6b1327599 100644 --- a/lib/crewai/src/crewai/cli/create_crew.py +++ b/lib/crewai/src/crewai/cli/create_crew.py @@ -211,12 +211,12 @@ def create_crew( return existing_provider = None - for provider, env_keys in ENV_VARS.items(): + for env_provider, env_keys in ENV_VARS.items(): if any( "key_name" in details and details["key_name"] in env_vars for details in env_keys ): - existing_provider = provider + existing_provider = env_provider break if existing_provider: diff --git a/lib/crewai/tests/cli/test_create_crew.py b/lib/crewai/tests/cli/test_create_crew.py index 478372f7f..912626e95 100644 --- a/lib/crewai/tests/cli/test_create_crew.py +++ b/lib/crewai/tests/cli/test_create_crew.py @@ -313,6 +313,137 @@ def test_create_folder_structure_rejects_reserved_names(): create_folder_structure(capitalized, parent_folder=temp_dir) +@mock.patch("crewai.cli.create_crew.create_folder_structure") +@mock.patch("crewai.cli.create_crew.copy_template") +@mock.patch("crewai.cli.create_crew.load_env_vars") +@mock.patch("crewai.cli.create_crew.get_provider_data") +@mock.patch("crewai.cli.create_crew.select_provider") +@mock.patch("crewai.cli.create_crew.select_model") +@mock.patch("click.prompt") +def test_provider_param_not_shadowed_by_env_vars_loop( + mock_prompt, + mock_select_model, + mock_select_provider, + mock_get_provider_data, + mock_load_env_vars, + mock_copy_template, + mock_create_folder_structure, + tmp_path, +): + """Test that the provider parameter is not overwritten by the ENV_VARS loop variable. + + Regression test for https://github.com/crewAIInc/crewAI/issues/5270: + The for-loop iterating over ENV_VARS used `provider` as its loop variable, + which shadowed the `provider` function parameter. After the fix, the loop + uses `env_provider` instead, so the original parameter value is preserved. + """ + crew_path = tmp_path / "test_crew" + crew_path.mkdir() + mock_create_folder_structure.return_value = (crew_path, "test_crew", "TestCrew") + + # Simulate existing env vars that match the "openai" provider key + mock_load_env_vars.return_value = {"OPENAI_API_KEY": "sk-existing"} + mock_get_provider_data.return_value = {"openai": ["gpt-4"]} + mock_select_provider.return_value = "openai" + mock_select_model.return_value = "gpt-4" + mock_prompt.return_value = "fake-api-key" + + # Patch click.confirm to accept the override prompt for existing provider + with mock.patch("click.confirm", return_value=True): + create_crew("Test Crew", provider="openai") + + # The key assertion: select_provider should still be called because + # the provider parameter should retain its original value (not be + # overwritten by the last ENV_VARS key from the loop). + mock_select_provider.assert_called() + + +@mock.patch("crewai.cli.create_crew.create_folder_structure") +@mock.patch("crewai.cli.create_crew.copy_template") +@mock.patch("crewai.cli.create_crew.load_env_vars") +@mock.patch("crewai.cli.create_crew.get_provider_data") +@mock.patch("crewai.cli.create_crew.select_provider") +@mock.patch("crewai.cli.create_crew.select_model") +@mock.patch("click.prompt") +def test_provider_param_preserved_after_env_vars_loop_no_match( + mock_prompt, + mock_select_model, + mock_select_provider, + mock_get_provider_data, + mock_load_env_vars, + mock_copy_template, + mock_create_folder_structure, + tmp_path, +): + """Test that provider parameter is preserved when no existing env var matches. + + When no ENV_VARS key matches the loaded environment, existing_provider + should be None and the provider parameter should still hold its original + value (not the last key from the ENV_VARS dict). + """ + crew_path = tmp_path / "test_crew" + crew_path.mkdir() + mock_create_folder_structure.return_value = (crew_path, "test_crew", "TestCrew") + + # No existing env vars — the loop iterates all of ENV_VARS without breaking + mock_load_env_vars.return_value = {} + mock_get_provider_data.return_value = {"anthropic": ["claude-3-opus-20240229"]} + mock_select_provider.return_value = "anthropic" + mock_select_model.return_value = "claude-3-opus-20240229" + mock_prompt.return_value = "fake-api-key" + + create_crew("Test Crew", provider="anthropic") + + # Verify the function completed without errors and the provider selection + # proceeded correctly (the provider param was not corrupted by the loop) + mock_select_provider.assert_called() + mock_select_model.assert_called_once_with( + "anthropic", {"anthropic": ["claude-3-opus-20240229"]} + ) + + +@mock.patch("crewai.cli.create_crew.create_folder_structure") +@mock.patch("crewai.cli.create_crew.copy_template") +@mock.patch("crewai.cli.create_crew.load_env_vars") +@mock.patch("crewai.cli.create_crew.get_provider_data") +@mock.patch("crewai.cli.create_crew.select_provider") +@mock.patch("crewai.cli.create_crew.select_model") +@mock.patch("click.prompt") +def test_existing_provider_detected_without_shadowing_provider_param( + mock_prompt, + mock_select_model, + mock_select_provider, + mock_get_provider_data, + mock_load_env_vars, + mock_copy_template, + mock_create_folder_structure, + tmp_path, +): + """Test that existing_provider is correctly detected from env vars. + + Ensures the loop correctly identifies an existing provider configuration + without corrupting the provider function parameter. + """ + crew_path = tmp_path / "test_crew" + crew_path.mkdir() + mock_create_folder_structure.return_value = (crew_path, "test_crew", "TestCrew") + + # Simulate existing ANTHROPIC_API_KEY in env + mock_load_env_vars.return_value = {"ANTHROPIC_API_KEY": "sk-ant-existing"} + mock_get_provider_data.return_value = {"openai": ["gpt-4"]} + mock_select_provider.return_value = "openai" + mock_select_model.return_value = "gpt-4" + mock_prompt.return_value = "fake-api-key" + + # Patch click.confirm — user declines to override existing provider config + with mock.patch("click.confirm", return_value=False): + create_crew("Test Crew", provider="openai") + + # When user declines to override, the function should return early + # and NOT call select_provider + mock_select_provider.assert_not_called() + + @mock.patch("crewai.cli.create_crew.create_folder_structure") @mock.patch("crewai.cli.create_crew.copy_template") @mock.patch("crewai.cli.create_crew.load_env_vars")