From 37d5be1b401102dbbc3bc784a4eedd02cd11c2ca Mon Sep 17 00:00:00 2001 From: Danelegend <43459662+Danelegend@users.noreply.github.com> Date: Wed, 25 Feb 2026 11:17:42 -0800 Subject: [PATCH] feat: python tool not added when no code interpretter server (#8749) --- .../python/python_tool.py | 7 +- .../tools/test_python_tool_server_enabled.py | 53 +++++++++++ .../tests/code_interpreter/conftest.py | 32 +++++++ .../test_code_interpreter_api.py | 1 + .../tools/test_python_tool_availability.py | 88 +++++++++++++++++++ 5 files changed, 179 insertions(+), 2 deletions(-) create mode 100644 backend/tests/external_dependency_unit/tools/test_python_tool_server_enabled.py create mode 100644 backend/tests/integration/tests/code_interpreter/conftest.py create mode 100644 backend/tests/unit/onyx/tools/test_python_tool_availability.py diff --git a/backend/onyx/tools/tool_implementations/python/python_tool.py b/backend/onyx/tools/tool_implementations/python/python_tool.py index d3cb3d723d..5f0fafb562 100644 --- a/backend/onyx/tools/tool_implementations/python/python_tool.py +++ b/backend/onyx/tools/tool_implementations/python/python_tool.py @@ -12,6 +12,7 @@ from onyx.configs.app_configs import CODE_INTERPRETER_BASE_URL from onyx.configs.app_configs import CODE_INTERPRETER_DEFAULT_TIMEOUT_MS from onyx.configs.app_configs import CODE_INTERPRETER_MAX_OUTPUT_LENGTH from onyx.configs.constants import FileOrigin +from onyx.db.code_interpreter import fetch_code_interpreter_server from onyx.file_store.utils import build_full_frontend_file_url from onyx.file_store.utils import get_default_file_store from onyx.server.query_and_chat.placement import Placement @@ -103,8 +104,10 @@ class PythonTool(Tool[PythonToolOverrideKwargs]): @override @classmethod def is_available(cls, db_session: Session) -> bool: - is_available = bool(CODE_INTERPRETER_BASE_URL) - return is_available + if not CODE_INTERPRETER_BASE_URL: + return False + server = fetch_code_interpreter_server(db_session) + return server.server_enabled def tool_definition(self) -> dict: return { diff --git a/backend/tests/external_dependency_unit/tools/test_python_tool_server_enabled.py b/backend/tests/external_dependency_unit/tools/test_python_tool_server_enabled.py new file mode 100644 index 0000000000..8296067806 --- /dev/null +++ b/backend/tests/external_dependency_unit/tools/test_python_tool_server_enabled.py @@ -0,0 +1,53 @@ +"""Tests that PythonTool.is_available() respects the server_enabled DB flag. + +Uses a real DB session with CODE_INTERPRETER_BASE_URL mocked so the +environment-variable check passes and the DB flag is the deciding factor. +""" + +from unittest.mock import patch + +from sqlalchemy.orm import Session + +from onyx.db.code_interpreter import fetch_code_interpreter_server +from onyx.db.code_interpreter import update_code_interpreter_server_enabled +from onyx.tools.tool_implementations.python.python_tool import PythonTool + + +def test_python_tool_unavailable_when_server_disabled( + db_session: Session, +) -> None: + """With a valid base URL, the tool should be unavailable when + server_enabled is False in the DB.""" + server = fetch_code_interpreter_server(db_session) + initial_enabled = server.server_enabled + + try: + update_code_interpreter_server_enabled(db_session, enabled=False) + + with patch( + "onyx.tools.tool_implementations.python.python_tool.CODE_INTERPRETER_BASE_URL", + "http://fake:8888", + ): + assert PythonTool.is_available(db_session) is False + finally: + update_code_interpreter_server_enabled(db_session, enabled=initial_enabled) + + +def test_python_tool_available_when_server_enabled( + db_session: Session, +) -> None: + """With a valid base URL, the tool should be available when + server_enabled is True in the DB.""" + server = fetch_code_interpreter_server(db_session) + initial_enabled = server.server_enabled + + try: + update_code_interpreter_server_enabled(db_session, enabled=True) + + with patch( + "onyx.tools.tool_implementations.python.python_tool.CODE_INTERPRETER_BASE_URL", + "http://fake:8888", + ): + assert PythonTool.is_available(db_session) is True + finally: + update_code_interpreter_server_enabled(db_session, enabled=initial_enabled) diff --git a/backend/tests/integration/tests/code_interpreter/conftest.py b/backend/tests/integration/tests/code_interpreter/conftest.py new file mode 100644 index 0000000000..0f695537b5 --- /dev/null +++ b/backend/tests/integration/tests/code_interpreter/conftest.py @@ -0,0 +1,32 @@ +from collections.abc import Generator + +import pytest +import requests + +from tests.integration.common_utils.constants import API_SERVER_URL +from tests.integration.common_utils.test_models import DATestUser + +CODE_INTERPRETER_URL = f"{API_SERVER_URL}/admin/code-interpreter" + + +@pytest.fixture +def preserve_code_interpreter_state( + admin_user: DATestUser, +) -> Generator[None, None, None]: + """Capture the code interpreter enabled state before a test and restore it + afterwards, so that tests that toggle the setting cannot leak state.""" + response = requests.get( + CODE_INTERPRETER_URL, + headers=admin_user.headers, + ) + response.raise_for_status() + initial_enabled = response.json()["enabled"] + + yield + + restore = requests.put( + CODE_INTERPRETER_URL, + json={"enabled": initial_enabled}, + headers=admin_user.headers, + ) + restore.raise_for_status() diff --git a/backend/tests/integration/tests/code_interpreter/test_code_interpreter_api.py b/backend/tests/integration/tests/code_interpreter/test_code_interpreter_api.py index 9f69330060..74a6d202c0 100644 --- a/backend/tests/integration/tests/code_interpreter/test_code_interpreter_api.py +++ b/backend/tests/integration/tests/code_interpreter/test_code_interpreter_api.py @@ -37,6 +37,7 @@ def test_get_code_interpreter_status_as_admin( def test_update_code_interpreter_disable_and_enable( admin_user: DATestUser, + preserve_code_interpreter_state: None, # noqa: ARG001 ) -> None: """PUT endpoint should update the enabled flag and persist across reads.""" # Disable diff --git a/backend/tests/unit/onyx/tools/test_python_tool_availability.py b/backend/tests/unit/onyx/tools/test_python_tool_availability.py new file mode 100644 index 0000000000..a1b371bede --- /dev/null +++ b/backend/tests/unit/onyx/tools/test_python_tool_availability.py @@ -0,0 +1,88 @@ +"""Tests for PythonTool availability based on server_enabled flag. + +Verifies that PythonTool reports itself as unavailable when either: +- CODE_INTERPRETER_BASE_URL is not set, or +- CodeInterpreterServer.server_enabled is False in the database. +""" + +from unittest.mock import MagicMock +from unittest.mock import patch + +from sqlalchemy.orm import Session + + +# ------------------------------------------------------------------ +# Unavailable when CODE_INTERPRETER_BASE_URL is not set +# ------------------------------------------------------------------ + + +@patch( + "onyx.tools.tool_implementations.python.python_tool.CODE_INTERPRETER_BASE_URL", + None, +) +def test_python_tool_unavailable_without_base_url() -> None: + from onyx.tools.tool_implementations.python.python_tool import PythonTool + + db_session = MagicMock(spec=Session) + assert PythonTool.is_available(db_session) is False + + +@patch( + "onyx.tools.tool_implementations.python.python_tool.CODE_INTERPRETER_BASE_URL", + "", +) +def test_python_tool_unavailable_with_empty_base_url() -> None: + from onyx.tools.tool_implementations.python.python_tool import PythonTool + + db_session = MagicMock(spec=Session) + assert PythonTool.is_available(db_session) is False + + +# ------------------------------------------------------------------ +# Unavailable when server_enabled is False +# ------------------------------------------------------------------ + + +@patch( + "onyx.tools.tool_implementations.python.python_tool.CODE_INTERPRETER_BASE_URL", + "http://localhost:8000", +) +@patch( + "onyx.tools.tool_implementations.python.python_tool.fetch_code_interpreter_server", +) +def test_python_tool_unavailable_when_server_disabled( + mock_fetch: MagicMock, +) -> None: + from onyx.tools.tool_implementations.python.python_tool import PythonTool + + mock_server = MagicMock() + mock_server.server_enabled = False + mock_fetch.return_value = mock_server + + db_session = MagicMock(spec=Session) + assert PythonTool.is_available(db_session) is False + + +# ------------------------------------------------------------------ +# Available when both conditions are met +# ------------------------------------------------------------------ + + +@patch( + "onyx.tools.tool_implementations.python.python_tool.CODE_INTERPRETER_BASE_URL", + "http://localhost:8000", +) +@patch( + "onyx.tools.tool_implementations.python.python_tool.fetch_code_interpreter_server", +) +def test_python_tool_available_when_server_enabled( + mock_fetch: MagicMock, +) -> None: + from onyx.tools.tool_implementations.python.python_tool import PythonTool + + mock_server = MagicMock() + mock_server.server_enabled = True + mock_fetch.return_value = mock_server + + db_session = MagicMock(spec=Session) + assert PythonTool.is_available(db_session) is True