fix: prevent CLI argument parser defaults from overriding config file values (#10140)

openhands/cli/settings.py

@@ -27,7 +27,7 @@ from openhands.core.config.condenser_config import (
     CondenserPipelineConfig,
     ConversationWindowCondenserConfig,
 )
-from openhands.core.config.utils import OH_DEFAULT_AGENT
+from openhands.core.config.config_utils import OH_DEFAULT_AGENT
 from openhands.memory.condenser.impl.llm_summarizing_condenser import (
     LLMSummarizingCondenserConfig,
 )

openhands/core/config/utils.py

@@ -20,10 +20,6 @@ from openhands.core.config.condenser_config import (
     condenser_config_from_toml_section,
     create_condenser_config,
 )
-from openhands.core.config.config_utils import (
-    OH_DEFAULT_AGENT,
-    OH_MAX_ITERATIONS,
-)
 from openhands.core.config.extended_config import ExtendedConfig
 from openhands.core.config.kubernetes_config import KubernetesConfig
 from openhands.core.config.llm_config import LLMConfig
@@ -712,14 +708,14 @@ def get_parser() -> argparse.ArgumentParser:
     parser.add_argument(
         '-c',
         '--agent-cls',
-        default=OH_DEFAULT_AGENT,
+        default=None,
         type=str,
         help='Name of the default agent to use',
     )
     parser.add_argument(
         '-i',
         '--max-iterations',
-        default=OH_MAX_ITERATIONS,
+        default=None,
         type=int,
         help='The maximum number of iterations to run the agent',
     )

tests/unit/test_arg_parser.py

@@ -1,6 +1,6 @@
 import pytest
 
-from openhands.core.config import OH_DEFAULT_AGENT, OH_MAX_ITERATIONS, get_parser
+from openhands.core.config import get_parser
 
 
 def test_parser_default_values():
@@ -10,8 +10,8 @@ def test_parser_default_values():
     assert args.directory is None
     assert args.task == ''
     assert args.file is None
-    assert args.agent_cls == OH_DEFAULT_AGENT
-    assert args.max_iterations == OH_MAX_ITERATIONS
+    assert args.agent_cls is None
+    assert args.max_iterations is None
     assert args.max_budget_per_task is None
     assert args.eval_output_dir == 'evaluation/evaluation_outputs/outputs'
     assert args.eval_n_limit is None

tests/unit/test_config_precedence.py

@@ -3,6 +3,8 @@ from unittest.mock import MagicMock, patch
 import pytest
 
 from openhands.core.config import (
+    OH_DEFAULT_AGENT,
+    OH_MAX_ITERATIONS,
     OpenHandsConfig,
     get_llm_config_arg,
     setup_config_from_args,
@@ -308,3 +310,74 @@ def test_cli_settings_json_not_override_config_toml(
     # Verify that settings.json did not override config.toml
     assert test_llm_config.model == 'config-toml-model'
     assert test_llm_config.api_key == 'config-toml-api-key'
+
+
+def test_default_values_applied_when_none():
+    """Test that default values are applied when config values are None."""
+
+    # Create mock args with None values for agent_cls and max_iterations
+    mock_args = MagicMock()
+    mock_args.config_file = None
+    mock_args.llm_config = None
+    mock_args.agent_cls = None
+    mock_args.max_iterations = None
+
+    # Load config
+    with patch(
+        'openhands.core.config.utils.load_openhands_config',
+        return_value=OpenHandsConfig(),
+    ):
+        config = setup_config_from_args(mock_args)
+
+    # Verify they match the expected defaults
+    assert config.default_agent == OH_DEFAULT_AGENT
+    assert config.max_iterations == OH_MAX_ITERATIONS
+
+
+def test_cli_args_override_defaults():
+    """Test that CLI arguments override default values."""
+
+    # Create mock args with custom values
+    mock_args = MagicMock()
+    mock_args.config_file = None
+    mock_args.llm_config = None
+    mock_args.agent_cls = 'CustomAgent'
+    mock_args.max_iterations = 50
+
+    # Load config
+    with patch(
+        'openhands.core.config.utils.load_openhands_config',
+        return_value=OpenHandsConfig(),
+    ):
+        config = setup_config_from_args(mock_args)
+
+    # Verify custom values are used instead of defaults
+    assert config.default_agent == 'CustomAgent'
+    assert config.max_iterations == 50
+
+
+def test_cli_args_none_uses_config_toml_values():
+    """Test that when CLI args agent_cls and max_iterations are None, config.toml values are used."""
+
+    # Create mock args with None values for agent_cls and max_iterations
+    mock_args = MagicMock()
+    mock_args.config_file = None
+    mock_args.llm_config = None
+    mock_args.agent_cls = None
+    mock_args.max_iterations = None
+
+    # Create a config with specific values from config.toml
+    config_from_toml = OpenHandsConfig()
+    config_from_toml.default_agent = 'ConfigTomlAgent'
+    config_from_toml.max_iterations = 100
+
+    # Load config
+    with patch(
+        'openhands.core.config.utils.load_openhands_config',
+        return_value=config_from_toml,
+    ):
+        config = setup_config_from_args(mock_args)
+
+    # Verify config.toml values are preserved when CLI args are None
+    assert config.default_agent == 'ConfigTomlAgent'
+    assert config.max_iterations == 100