Repository: huggingface/smolagents

Issue:
[BUG] CI is broken: AssertionError with message role in test_action_step_to_messages
**Describe the bug**
CI is broken: https://github.com/huggingface/smolagents/actions/runs/13647944821/job/38150144811?pr=861
```
FAILED tests/test_memory.py::test_action_step_to_messages - AssertionError: assert <MessageRole....: 'tool-call'> == <MessageRole....: 'assistant'>
  
  - assistant
  + tool-call
```

I think the CI was broken with the merge of:
- #779

Discussion:

Patch:
diff --git a/src/smolagents/cli.py b/src/smolagents/cli.py
index 6d5c070..7d93a3b 100644
--- a/src/smolagents/cli.py
+++ b/src/smolagents/cli.py
@@ -79,7 +79,7 @@ def parse_arguments(description):
     return parser.parse_args()
 
 
-def load_model(model_type: str, model_id: str, api_base: str | None, api_key: str | None) -> Model:
+def load_model(model_type: str, model_id: str, api_base: str | None = None, api_key: str | None = None) -> Model:
     if model_type == "OpenAIServerModel":
         return OpenAIServerModel(
             api_key=api_key or os.getenv("FIREWORKS_API_KEY"),
diff --git a/src/smolagents/local_python_executor.py b/src/smolagents/local_python_executor.py
index 90ee7f0..f657387 100644
--- a/src/smolagents/local_python_executor.py
+++ b/src/smolagents/local_python_executor.py
@@ -136,6 +136,18 @@ DANGEROUS_PATTERNS = (
     "multiprocessing",
 )
 
+DANGEROUS_MODULES = [
+    "os",
+    "subprocess",
+    "pty",
+    "shutil",
+    "sys",
+    "pathlib",
+    "io",
+    "socket",
+    "multiprocessing",
+]
+
 
 class PrintContainer:
     def __init__(self):
@@ -225,12 +237,23 @@ def safer_eval(func: Callable):
     """
 
     @wraps(func)
-    def _check_return(*args, **kwargs):
-        result = func(*args, **kwargs)
+    def _check_return(
+        expression,
+        state,
+        static_tools,
+        custom_tools,
+        authorized_imports=BASE_BUILTIN_MODULES,
+    ):
+        result = func(expression, state, static_tools, custom_tools, authorized_imports=authorized_imports)
         if (isinstance(result, ModuleType) and result is builtins) or (
             isinstance(result, dict) and result == vars(builtins)
         ):
             raise InterpreterError("Forbidden return value: builtins")
+        if isinstance(result, ModuleType):
+            if "*" not in authorized_imports:
+                for module in DANGEROUS_MODULES:
+                    if module not in authorized_imports and result is import_module(module):
+                        raise InterpreterError(f"Forbidden return value: {module}")
         return result
 
     return _check_return
diff --git a/src/smolagents/memory.py b/src/smolagents/memory.py
index b7a9c5b..7eabbea 100644
--- a/src/smolagents/memory.py
+++ b/src/smolagents/memory.py
@@ -106,7 +106,8 @@ class ActionStep(MemoryStep):
                     content=[
                         {
                             "type": "text",
-                            "text": f"Call id: {self.tool_calls[0].id}\nObservation:\n{self.observations}",
+                            "text": (f"Call id: {self.tool_calls[0].id}\n" if self.tool_calls else "")
+                            + f"Observation:\n{self.observations}",
                         }
                     ],
                 )
diff --git a/src/smolagents/models.py b/src/smolagents/models.py
index 2a586ed..38561c5 100644
--- a/src/smolagents/models.py
+++ b/src/smolagents/models.py
@@ -24,7 +24,6 @@ from dataclasses import asdict, dataclass
 from enum import Enum
 from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union
 
-from huggingface_hub import InferenceClient
 from huggingface_hub.utils import is_torch_available
 from PIL import Image
 
@@ -429,6 +428,8 @@ class HfApiModel(Model):
         custom_role_conversions: Optional[Dict[str, str]] = None,
         **kwargs,
     ):
+        from huggingface_hub import InferenceClient
+
         super().__init__(**kwargs)
         self.model_id = model_id
         self.provider = provider


Test patch:
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
index e3592cc..9ac958d 100644
--- a/.github/workflows/tests.yml
+++ b/.github/workflows/tests.yml
@@ -53,6 +53,11 @@ jobs:
       #   run: |
       #     uv run pytest ./tests/test_all_docs.py
 
+      - name: CLI tests
+        run: |
+          uv run pytest ./tests/test_cli.py
+        if: ${{ success() || failure() }}
+
       - name: Final answer tests
         run: |
           uv run pytest ./tests/test_final_answer.py
diff --git a/tests/test_cli.py b/tests/test_cli.py
new file mode 100644
index 0000000..69832f0
--- /dev/null
+++ b/tests/test_cli.py
@@ -0,0 +1,54 @@
+from unittest.mock import patch
+
+import pytest
+
+from smolagents.cli import load_model
+from smolagents.models import HfApiModel, LiteLLMModel, OpenAIServerModel, TransformersModel
+
+
+@pytest.fixture
+def set_env_vars(monkeypatch):
+    monkeypatch.setenv("FIREWORKS_API_KEY", "test_fireworks_api_key")
+    monkeypatch.setenv("HF_TOKEN", "test_hf_api_key")
+
+
+def test_load_model_openai_server_model(set_env_vars):
+    with patch("openai.OpenAI") as MockOpenAI:
+        model = load_model("OpenAIServerModel", "test_model_id")
+    assert isinstance(model, OpenAIServerModel)
+    assert model.model_id == "test_model_id"
+    assert MockOpenAI.call_count == 1
+    assert MockOpenAI.call_args.kwargs["base_url"] == "https://api.fireworks.ai/inference/v1"
+    assert MockOpenAI.call_args.kwargs["api_key"] == "test_fireworks_api_key"
+
+
+def test_load_model_litellm_model():
+    model = load_model("LiteLLMModel", "test_model_id", api_key="test_api_key", api_base="https://api.test.com")
+    assert isinstance(model, LiteLLMModel)
+    assert model.api_key == "test_api_key"
+    assert model.api_base == "https://api.test.com"
+    assert model.model_id == "test_model_id"
+
+
+def test_load_model_transformers_model():
+    with (
+        patch("transformers.AutoModelForCausalLM.from_pretrained"),
+        patch("transformers.AutoTokenizer.from_pretrained"),
+    ):
+        model = load_model("TransformersModel", "test_model_id")
+    assert isinstance(model, TransformersModel)
+    assert model.model_id == "test_model_id"
+
+
+def test_load_model_hf_api_model(set_env_vars):
+    with patch("huggingface_hub.InferenceClient") as huggingface_hub_InferenceClient:
+        model = load_model("HfApiModel", "test_model_id")
+    assert isinstance(model, HfApiModel)
+    assert model.model_id == "test_model_id"
+    assert huggingface_hub_InferenceClient.call_count == 1
+    assert huggingface_hub_InferenceClient.call_args.kwargs["token"] == "test_hf_api_key"
+
+
+def test_load_model_invalid_model_type():
+    with pytest.raises(ValueError, match="Unsupported model type: InvalidModel"):
+        load_model("InvalidModel", "test_model_id")
diff --git a/tests/test_local_python_executor.py b/tests/test_local_python_executor.py
index d0f294a..592b7f0 100644
--- a/tests/test_local_python_executor.py
+++ b/tests/test_local_python_executor.py
@@ -1436,7 +1436,7 @@ class TestLocalPythonExecutorSecurity:
 
     @pytest.mark.parametrize(
         "additional_authorized_imports, expectation",
-        [([], pytest.raises(InterpreterError)), (["sys"], does_not_raise())],
+        [([], pytest.raises(InterpreterError)), (["os", "sys"], does_not_raise())],
     )
     def test_vulnerability_via_sys(self, additional_authorized_imports, expectation):
         executor = LocalPythonExecutor(additional_authorized_imports)
@@ -1450,6 +1450,22 @@ class TestLocalPythonExecutorSecurity:
                 )
             )
 
+    @pytest.mark.parametrize(
+        "additional_authorized_imports, expectation",
+        [(["importlib"], pytest.raises(InterpreterError)), (["importlib", "os"], does_not_raise())],
+    )
+    def test_vulnerability_via_importlib(self, additional_authorized_imports, expectation):
+        executor = LocalPythonExecutor(additional_authorized_imports)
+        with expectation:
+            executor(
+                dedent(
+                    """
+                    import importlib
+                    importlib.import_module("os").system(":")
+                    """
+                )
+            )
+
     @pytest.mark.parametrize(
         "code",
         [
@@ -1500,3 +1516,23 @@ class TestLocalPythonExecutorSecurity:
                     """
                 )
             )
+
+    @pytest.mark.parametrize(
+        "additional_authorized_imports, expectation",
+        [([], pytest.raises(InterpreterError)), (["os"], does_not_raise())],
+    )
+    def test_vulnerability_load_module_via_builtin_importer(self, additional_authorized_imports, expectation):
+        executor = LocalPythonExecutor(additional_authorized_imports)
+        with expectation:
+            executor(
+                dedent(
+                    """
+                    classes = {}.__class__.__base__.__subclasses__()
+                    for cls in classes:
+                        if cls.__name__ == "BuiltinImporter":
+                            break
+                    os_module = cls().load_module("os")
+                    os_module.system(":")
+                    """
+                )
+            )
diff --git a/tests/test_memory.py b/tests/test_memory.py
index c007a18..c435177 100644
--- a/tests/test_memory.py
+++ b/tests/test_memory.py
@@ -70,7 +70,7 @@ def test_action_step_to_messages():
         assert "type" in content
         assert "text" in content
     message = messages[1]
-    assert message["role"] == MessageRole.ASSISTANT
+    assert message["role"] == MessageRole.TOOL_CALL
 
     assert len(message["content"]) == 1
     text_content = message["content"][0]
@@ -89,6 +89,28 @@ def test_action_step_to_messages():
     assert "image" in image_content
 
 
+def test_action_step_to_messages_no_tool_calls_with_observations():
+    action_step = ActionStep(
+        model_input_messages=None,
+        tool_calls=None,
+        start_time=None,
+        end_time=None,
+        step_number=None,
+        error=None,
+        duration=None,
+        model_output_message=None,
+        model_output=None,
+        observations="This is an observation.",
+        observations_images=None,
+        action_output=None,
+    )
+    messages = action_step.to_messages()
+    assert len(messages) == 1
+    observation_message = messages[0]
+    assert observation_message["role"] == MessageRole.TOOL_RESPONSE
+    assert "Observation:\nThis is an observation." in observation_message["content"][0]["text"]
+
+
 def test_planning_step_to_messages():
     planning_step = PlanningStep(
         model_input_messages=[Message(role=MessageRole.USER, content="Hello")],