From c46d37d8760eabbe8a5921d50f104ea5468a83cc Mon Sep 17 00:00:00 2001
From: RosenTomov <32323783+RosenTomov@users.noreply.github.com>
Date: Tue, 9 Jun 2026 18:35:10 +0300
Subject: [PATCH] test(tool_execution): stop two tests leaking
 src.tool_execution into the suite (#2686)

* Make in-venv pip-fallback test independent of the runner's environment

test_pip_install_fallback_chain_propagates_failure_in_venv simulated the in-venv case by probing the real interpreter (sys.prefix != sys.base_prefix). That assumes the test runner is itself inside a venv. CI runs pytest with no venv, so venv_check reported not-in-venv, the negated guard flipped, the --user branch fired, and the assertion failed. Make venv_check exit 0 directly to simulate the in-venv condition deterministically, mirroring the outside-venv companion test.

* Stop agent-tool import shims from leaking into the admin-gate test

test_function_call_non_object_args and test_unknown_tool_calls stub heavy DB/auth deps at import time to load the real agent-tool stack, but they popped src.tool_execution and left core.auth stubbed without restoring. Popping and re-importing src.tool_execution rebinds the src package's tool_execution attribute, so test_edit_file's later 'import src.tool_execution as te' resolved to a different module object than the one execute_tool_block lives in. The monkeypatch on _owner_is_admin then missed, the non-admin edit_file gate never fired, and the edit went through (exit_code 0). Stop touching src.tool_execution and restore the heavy stubs after import. Verified the full suite is green on Linux (Python 3.11, matching CI).

---------

Co-authored-by: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com>
---
 tests/test_function_call_non_object_args.py | 44 +++++++++++++------
 tests/test_unknown_tool_calls.py            | 48 +++++++++++++--------
 2 files changed, 61 insertions(+), 31 deletions(-)

diff --git a/tests/test_function_call_non_object_args.py b/tests/test_function_call_non_object_args.py
index 5e8cf4675..f96e0cb61 100644
--- a/tests/test_function_call_non_object_args.py
+++ b/tests/test_function_call_non_object_args.py
@@ -1,22 +1,38 @@
 import sys
 from unittest.mock import MagicMock
 
-# Clean up any mocks from previous tests to ensure we load real modules
-for mod in ['src.agent_tools', 'src.tool_parsing', 'src.tool_schemas', 'src.tool_execution']:
-    sys.modules.pop(mod, None)
+# This module needs the real agent-tool stack; importing it pulls in heavy
+# DB/auth deps, so we stub those just long enough to import, then restore them.
+# We deliberately do NOT pop src.tool_execution: popping and re-importing it
+# rebinds the `src` package's `tool_execution` attribute, so a later
+# `import src.tool_execution as te` resolves to a different module object than
+# the one its functions live in - which silently breaks tests that monkeypatch
+# it (e.g. test_edit_file's admin gate).
+_ABSENT = object()
+_AGENT_MODULES = ["src.agent_tools", "src.tool_parsing", "src.tool_schemas"]
+_STUBBED = [
+    "sqlalchemy", "sqlalchemy.orm", "sqlalchemy.ext", "sqlalchemy.ext.declarative",
+    "sqlalchemy.ext.hybrid", "sqlalchemy.sql", "sqlalchemy.sql.expression",
+    "src.database", "core.models", "core.database", "core.auth",
+]
+_saved_stubs = {name: sys.modules.get(name, _ABSENT) for name in _STUBBED}
 
-# Mock heavy database/model dependencies before importing
-for mod in [
-    'sqlalchemy', 'sqlalchemy.orm', 'sqlalchemy.ext', 'sqlalchemy.ext.declarative',
-    'sqlalchemy.ext.hybrid', 'sqlalchemy.sql', 'sqlalchemy.sql.expression',
-    'src.database', 'core.models', 'core.database', 'core.auth'
-]:
-    if mod not in sys.modules:
-        sys.modules[mod] = MagicMock()
+for _mod in _AGENT_MODULES:
+    sys.modules.pop(_mod, None)
+for _mod in _STUBBED:
+    if _mod not in sys.modules:
+        sys.modules[_mod] = MagicMock()
 
-import pytest
-import src.agent_tools  # noqa: F401
-from src.tool_schemas import function_call_to_tool_block
+import pytest  # noqa: E402
+import src.agent_tools  # noqa: E402,F401
+from src.tool_schemas import function_call_to_tool_block  # noqa: E402
+
+# Drop the stubs we installed so they do not leak into later tests.
+for _name, _original in _saved_stubs.items():
+    if _original is _ABSENT:
+        sys.modules.pop(_name, None)
+    else:
+        sys.modules[_name] = _original
 
 
 @pytest.mark.parametrize("arguments", [
diff --git a/tests/test_unknown_tool_calls.py b/tests/test_unknown_tool_calls.py
index bf6e4b64c..9911d61fb 100644
--- a/tests/test_unknown_tool_calls.py
+++ b/tests/test_unknown_tool_calls.py
@@ -1,25 +1,39 @@
 import sys
 from unittest.mock import MagicMock
 
-# Clean up any mocks from previous tests to ensure we load real modules
-for mod in ['src.agent_tools', 'src.tool_parsing', 'src.tool_schemas', 'src.tool_execution']:
-    sys.modules.pop(mod, None)
+# This module needs the real agent-tool stack; importing it pulls in heavy
+# DB/auth deps, so we stub those just long enough to import, then restore them.
+# We deliberately do NOT pop src.tool_execution: popping and re-importing it
+# rebinds the `src` package's `tool_execution` attribute, so a later
+# `import src.tool_execution as te` resolves to a different module object than
+# the one its functions live in - which silently breaks tests that monkeypatch
+# it (e.g. test_edit_file's admin gate).
+_ABSENT = object()
+_AGENT_MODULES = ["src.agent_tools", "src.tool_parsing", "src.tool_schemas"]
+_STUBBED = [
+    "sqlalchemy", "sqlalchemy.orm", "sqlalchemy.ext", "sqlalchemy.ext.declarative",
+    "sqlalchemy.ext.hybrid", "sqlalchemy.sql", "sqlalchemy.sql.expression",
+    "src.database", "core.models", "core.database", "core.auth",
+]
+_saved_stubs = {name: sys.modules.get(name, _ABSENT) for name in _STUBBED}
 
-# Mock heavy database/model dependencies before importing
-for mod in [
-    'sqlalchemy', 'sqlalchemy.orm', 'sqlalchemy.ext', 'sqlalchemy.ext.declarative',
-    'sqlalchemy.ext.hybrid', 'sqlalchemy.sql', 'sqlalchemy.sql.expression',
-    'src.database', 'core.models', 'core.database', 'core.auth'
-]:
-    if mod not in sys.modules:
-        sys.modules[mod] = MagicMock()
+for _mod in _AGENT_MODULES:
+    sys.modules.pop(_mod, None)
+for _mod in _STUBBED:
+    if _mod not in sys.modules:
+        sys.modules[_mod] = MagicMock()
 
-import pytest
-import src.agent_tools
-from src.tool_parsing import parse_tool_blocks
-from src.tool_schemas import function_call_to_tool_block
-from src.tool_execution import execute_tool_block
-from types import SimpleNamespace
+import pytest  # noqa: E402
+import src.agent_tools  # noqa: E402,F401
+from src.tool_parsing import parse_tool_blocks  # noqa: E402
+from src.tool_schemas import function_call_to_tool_block  # noqa: E402
+
+# Drop the stubs we installed so they do not leak into later tests.
+for _name, _original in _saved_stubs.items():
+    if _original is _ABSENT:
+        sys.modules.pop(_name, None)
+    else:
+        sys.modules[_name] = _original
 
 
 def test_parse_xml_unknown_tool_returns_none():