From bb27456eab14ae3fffc9fd8a1c0f861ff8d83dde Mon Sep 17 00:00:00 2001 From: Fromsko <1614355756@qq.com> Date: Sat, 25 Jul 2026 00:10:44 +0800 Subject: [PATCH 01/15] code_executors: defer docker and magic imports to avoid hang on Windows without Docker/libmagic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows (and other platforms without Docker Desktop or libmagic installed), top-level `import docker` and `import magic` cause the interpreter to hang or segfault while probing for native daemons/libraries. This makes the entire trpc_agent_sdk package unusable — even a simple `from trpc_agent_sdk.agents import LlmAgent` never returns. Both imports are now deferred to call-time: - _container_cli.py: `import docker` moved into `_import_docker()`, called lazily from `_init_docker_client()` on first real use. - _files.py: `import magic` moved inside `detect_content_type()`, wrapped in try/except to gracefully degrade when python-magic is unavailable. Verified locally: `from trpc_agent_sdk.agents import LlmAgent` now completes instantly on Windows 11 without Docker Desktop, and the minimal example (weather agent with tool calling) runs end-to-end. Fixes #230 --- .../container/_container_cli.py | 36 ++++++++++++++++--- trpc_agent_sdk/code_executors/utils/_files.py | 18 ++++++---- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/trpc_agent_sdk/code_executors/container/_container_cli.py b/trpc_agent_sdk/code_executors/container/_container_cli.py index 39447ad6..7b2f2cb4 100644 --- a/trpc_agent_sdk/code_executors/container/_container_cli.py +++ b/trpc_agent_sdk/code_executors/container/_container_cli.py @@ -16,14 +16,38 @@ from dataclasses import dataclass from typing import Optional -import docker -from docker.models.containers import Container -from docker.utils.socket import consume_socket_output -from docker.utils.socket import demux_adaptor -from docker.utils.socket import frames_iter +# Note: Docker SDK imports (docker, docker.models.containers, docker.utils.socket) +# are deferred to runtime via _import_docker() to avoid hanging/crashing on +# systems without Docker installed (e.g. Windows without Docker Desktop). +# See: https://github.com/trpc-group/trpc-agent-python/issues/230 + from trpc_agent_sdk.log import logger from trpc_agent_sdk.utils import CommandExecResult + +def _import_docker(): + """Lazily import the Docker SDK only when actually needed. + + Importing ``docker`` at module top-level causes the Python Docker SDK + to probe for a Docker daemon on Windows (named pipe) and other platforms. + When Docker is not installed/running this probe hangs or segfaults, + making the entire ``trpc_agent_sdk`` package unusable. By deferring the + import to call-time we ensure that users who never touch + ``ContainerCodeExecutor`` are unaffected. + """ + import docker as _docker + from docker.models.containers import Container as _Container + from docker.utils.socket import consume_socket_output as _cso + from docker.utils.socket import demux_adaptor as _da + from docker.utils.socket import frames_iter as _fi + globals().update({ + 'docker': _docker, + 'Container': _Container, + 'consume_socket_output': _cso, + 'demux_adaptor': _da, + 'frames_iter': _fi, + }) + return _docker DEFAULT_IMAGE_TAG = 'python:3-slim' @@ -88,6 +112,8 @@ def _init_docker_client(self): - Remote Docker via DOCKER_HOST environment variable - Custom base_url if provided """ + # Lazily import the Docker SDK on first real use. + _import_docker() # Try to initialize Docker client # Let docker SDK handle connection detection (it supports various methods) try: diff --git a/trpc_agent_sdk/code_executors/utils/_files.py b/trpc_agent_sdk/code_executors/utils/_files.py index af79ab66..fc83a683 100644 --- a/trpc_agent_sdk/code_executors/utils/_files.py +++ b/trpc_agent_sdk/code_executors/utils/_files.py @@ -17,11 +17,12 @@ from pathlib import Path from typing import Optional -try: - import magic - HAS_MAGIC = True -except ImportError: - HAS_MAGIC = False +# NOTE: python-magic is NOT imported at module top level. +# On Windows (and other platforms without libmagic installed), the `magic` +# package searches for the native libmagic DLL at import time, which can hang +# or crash the interpreter — making the entire trpc_agent_sdk package unusable. +# Instead, we lazily import it inside detect_content_type() on first use. +# See: https://github.com/trpc-group/trpc-agent-python/issues/230 def path_join(base: str, path: str) -> str: @@ -228,9 +229,12 @@ def detect_content_type(filename: Path, data: bytes) -> str: if mime_type: return mime_type - # filename guess failed, use magic to guess - if HAS_MAGIC: + # filename guess failed, try python-magic (lazily imported) + try: + import magic return magic.from_buffer(data, mime=True) + except Exception: + pass # magic guess failed, use simple content-based detection if data.startswith(b'\x89PNG\r\n\x1a\n'): From 33aedbf7ed054cbe48d4ff667936482ab22585fd Mon Sep 17 00:00:00 2001 From: Fromsko <1614355756@qq.com> Date: Sat, 25 Jul 2026 00:23:26 +0800 Subject: [PATCH 02/15] tests: skip magic test on Windows; keep HAS_MAGIC for backward compat - detect_content_type now skips python-magic entirely on win32 to avoid native libmagic access violations (not catchable by try/except) - HAS_MAGIC retained as module-level False constant for backward compat with tests that mock it - test_detect_content_type_with_magic marked skipif(win32) - No new test failures: 19 pass, 1 skip, 11 pre-existing Windows path/glob failures unchanged from main branch --- tests/code_executors/test_utils_files.py | 4 ++++ trpc_agent_sdk/code_executors/utils/_files.py | 17 ++++++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/tests/code_executors/test_utils_files.py b/tests/code_executors/test_utils_files.py index 69e1c011..abcc3f18 100644 --- a/tests/code_executors/test_utils_files.py +++ b/tests/code_executors/test_utils_files.py @@ -5,10 +5,13 @@ # tRPC-Agent-Python is licensed under Apache-2.0. import os +import sys import tempfile from pathlib import Path from unittest.mock import patch +import pytest + from trpc_agent_sdk.code_executors.utils import collect_files_with_glob from trpc_agent_sdk.code_executors.utils import copy_dir from trpc_agent_sdk.code_executors.utils import copy_path @@ -328,6 +331,7 @@ def test_detect_content_type_text_utf8(self): @patch('trpc_agent_sdk.code_executors.utils._files.HAS_MAGIC', True) @patch('trpc_agent_sdk.code_executors.utils._files.magic', create=True) + @pytest.mark.skipif(sys.platform == 'win32', reason='python-magic crashes on Windows without libmagic DLL') def test_detect_content_type_with_magic(self, mock_magic): """Test detecting content type using magic library.""" filename = Path("test.unknown") diff --git a/trpc_agent_sdk/code_executors/utils/_files.py b/trpc_agent_sdk/code_executors/utils/_files.py index fc83a683..41e1b8ba 100644 --- a/trpc_agent_sdk/code_executors/utils/_files.py +++ b/trpc_agent_sdk/code_executors/utils/_files.py @@ -14,6 +14,7 @@ import mimetypes import os import shutil +import sys from pathlib import Path from typing import Optional @@ -23,6 +24,7 @@ # or crash the interpreter — making the entire trpc_agent_sdk package unusable. # Instead, we lazily import it inside detect_content_type() on first use. # See: https://github.com/trpc-group/trpc-agent-python/issues/230 +HAS_MAGIC = False # set to True by tests via mock; real check is lazy def path_join(base: str, path: str) -> str: @@ -230,11 +232,16 @@ def detect_content_type(filename: Path, data: bytes) -> str: return mime_type # filename guess failed, try python-magic (lazily imported) - try: - import magic - return magic.from_buffer(data, mime=True) - except Exception: - pass + # On Windows, python-magic requires a native libmagic DLL that, when + # missing, causes an access violation at import time (not catchable by + # try/except). We skip it entirely on win32 and fall through to the + # simple content-based detection below. + if HAS_MAGIC and sys.platform != 'win32': + try: + import magic + return magic.from_buffer(data, mime=True) + except Exception: + pass # magic guess failed, use simple content-based detection if data.startswith(b'\x89PNG\r\n\x1a\n'): From 7f60d9379328d05a9ea0637586ecf0576eeb6efa Mon Sep 17 00:00:00 2001 From: Fromsko <1614355756@qq.com> Date: Sat, 25 Jul 2026 00:27:16 +0800 Subject: [PATCH 03/15] code_executors: fix HAS_MAGIC dead code and test mock (address review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address AI code review feedback on PR #231: 1. Replace hardcoded HAS_MAGIC=False with cached _magic_module pattern: - On non-win32, detect_content_type lazily imports magic on first call and caches the module in _magic_module (checked once via _magic_checked) - This preserves the magic detection path on Linux/Mac where libmagic is properly installed, fixing the dead-code regression 2. Separate import failure from runtime failure: - ImportError → log debug, fall through to byte-signature detection - magic.from_buffer exception → log debug with exc_info, fall through - Previously both were silently swallowed 3. Fix test_detect_content_type_with_magic to inject mock directly into _magic_module instead of patching non-existent module attributes --- tests/code_executors/test_utils_files.py | 23 +++++++++++++------ trpc_agent_sdk/code_executors/utils/_files.py | 20 +++++++++++----- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/tests/code_executors/test_utils_files.py b/tests/code_executors/test_utils_files.py index abcc3f18..98548bab 100644 --- a/tests/code_executors/test_utils_files.py +++ b/tests/code_executors/test_utils_files.py @@ -329,19 +329,28 @@ def test_detect_content_type_text_utf8(self): assert "text" in mime_type.lower() or mime_type == "application/octet-stream" - @patch('trpc_agent_sdk.code_executors.utils._files.HAS_MAGIC', True) - @patch('trpc_agent_sdk.code_executors.utils._files.magic', create=True) @pytest.mark.skipif(sys.platform == 'win32', reason='python-magic crashes on Windows without libmagic DLL') - def test_detect_content_type_with_magic(self, mock_magic): + def test_detect_content_type_with_magic(self): """Test detecting content type using magic library.""" + import sys as _sys + from unittest.mock import MagicMock + filename = Path("test.unknown") data = b"some data" + mock_magic = MagicMock() mock_magic.from_buffer.return_value = "application/custom" - mime_type = detect_content_type(filename, data) - - assert mime_type == "application/custom" - mock_magic.from_buffer.assert_called_once_with(data, mime=True) + # Inject mock into the module's cached magic slot + import trpc_agent_sdk.code_executors.utils._files as _f + orig = (_f._magic_module, _f._magic_checked) + _f._magic_module = mock_magic + _f._magic_checked = True + try: + mime_type = detect_content_type(filename, data) + assert mime_type == "application/custom" + mock_magic.from_buffer.assert_called_once_with(data, mime=True) + finally: + _f._magic_module, _f._magic_checked = orig class TestGetRelPath: diff --git a/trpc_agent_sdk/code_executors/utils/_files.py b/trpc_agent_sdk/code_executors/utils/_files.py index 41e1b8ba..48320cd0 100644 --- a/trpc_agent_sdk/code_executors/utils/_files.py +++ b/trpc_agent_sdk/code_executors/utils/_files.py @@ -24,7 +24,8 @@ # or crash the interpreter — making the entire trpc_agent_sdk package unusable. # Instead, we lazily import it inside detect_content_type() on first use. # See: https://github.com/trpc-group/trpc-agent-python/issues/230 -HAS_MAGIC = False # set to True by tests via mock; real check is lazy +_magic_module = None # cached after first successful import on non-win32 +_magic_checked = False def path_join(base: str, path: str) -> str: @@ -231,17 +232,24 @@ def detect_content_type(filename: Path, data: bytes) -> str: if mime_type: return mime_type - # filename guess failed, try python-magic (lazily imported) + # filename guess failed, try python-magic (lazily imported). # On Windows, python-magic requires a native libmagic DLL that, when # missing, causes an access violation at import time (not catchable by # try/except). We skip it entirely on win32 and fall through to the # simple content-based detection below. - if HAS_MAGIC and sys.platform != 'win32': + global _magic_module, _magic_checked + if sys.platform != 'win32' and not _magic_checked: try: - import magic - return magic.from_buffer(data, mime=True) + import magic as _m + _magic_module = _m + except ImportError: + logger.debug("python-magic not available; falling back to byte-signature detection") + _magic_checked = True + if _magic_module is not None: + try: + return _magic_module.from_buffer(data, mime=True) except Exception: - pass + logger.debug("magic.from_buffer failed; falling back to byte-signature detection", exc_info=True) # magic guess failed, use simple content-based detection if data.startswith(b'\x89PNG\r\n\x1a\n'): From fffa3a554ac7680f4695d3b2a461ef4ca56a1aa0 Mon Sep 17 00:00:00 2001 From: Fromsko <1614355756@qq.com> Date: Sat, 25 Jul 2026 00:32:54 +0800 Subject: [PATCH 04/15] address review: TYPE_CHECKING guard, import short-circuit, yapf format 1. _container_cli.py: add TYPE_CHECKING guard for docker/Container type annotations so get_type_hints() won't NameError without Docker installed 2. _container_cli.py: add _docker_imported flag to short-circuit repeated _import_docker() calls 3. _files.py: only set _magic_checked=True after successful import or confirmed ImportError (clearer intent) 4. Run yapf -i on all changed files to satisfy CI format check --- .../code_executors/container/_container_cli.py | 14 ++++++++++++-- trpc_agent_sdk/code_executors/utils/_files.py | 3 ++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/trpc_agent_sdk/code_executors/container/_container_cli.py b/trpc_agent_sdk/code_executors/container/_container_cli.py index 7b2f2cb4..928d158a 100644 --- a/trpc_agent_sdk/code_executors/container/_container_cli.py +++ b/trpc_agent_sdk/code_executors/container/_container_cli.py @@ -14,16 +14,21 @@ import os import socket as pysocket from dataclasses import dataclass -from typing import Optional +from typing import TYPE_CHECKING, Optional # Note: Docker SDK imports (docker, docker.models.containers, docker.utils.socket) # are deferred to runtime via _import_docker() to avoid hanging/crashing on # systems without Docker installed (e.g. Windows without Docker Desktop). # See: https://github.com/trpc-group/trpc-agent-python/issues/230 +if TYPE_CHECKING: + import docker + from docker.models.containers import Container from trpc_agent_sdk.log import logger from trpc_agent_sdk.utils import CommandExecResult +_docker_imported = False + def _import_docker(): """Lazily import the Docker SDK only when actually needed. @@ -35,6 +40,9 @@ def _import_docker(): import to call-time we ensure that users who never touch ``ContainerCodeExecutor`` are unaffected. """ + global _docker_imported + if _docker_imported: + return import docker as _docker from docker.models.containers import Container as _Container from docker.utils.socket import consume_socket_output as _cso @@ -47,7 +55,9 @@ def _import_docker(): 'demux_adaptor': _da, 'frames_iter': _fi, }) - return _docker + _docker_imported = True + + DEFAULT_IMAGE_TAG = 'python:3-slim' diff --git a/trpc_agent_sdk/code_executors/utils/_files.py b/trpc_agent_sdk/code_executors/utils/_files.py index 48320cd0..65e811ec 100644 --- a/trpc_agent_sdk/code_executors/utils/_files.py +++ b/trpc_agent_sdk/code_executors/utils/_files.py @@ -242,9 +242,10 @@ def detect_content_type(filename: Path, data: bytes) -> str: try: import magic as _m _magic_module = _m + _magic_checked = True except ImportError: logger.debug("python-magic not available; falling back to byte-signature detection") - _magic_checked = True + _magic_checked = True # cache failure to avoid retrying every call if _magic_module is not None: try: return _magic_module.from_buffer(data, mime=True) From 7f2e29a2fdccb23762786db7398101ffa56736c9 Mon Sep 17 00:00:00 2001 From: Fromsko <1614355756@qq.com> Date: Sat, 25 Jul 2026 00:37:12 +0800 Subject: [PATCH 05/15] fix: flake8 F821 + missing logger import for CI lint - _container_cli.py: add # noqa: F821 for frames_iter/demux_adaptor/ consume_socket_output (injected at runtime by _import_docker) - _files.py: add missing `from trpc_agent_sdk.log import logger` - Both yapf and flake8 now pass clean on changed files --- trpc_agent_sdk/code_executors/container/_container_cli.py | 6 +++--- trpc_agent_sdk/code_executors/utils/_files.py | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/trpc_agent_sdk/code_executors/container/_container_cli.py b/trpc_agent_sdk/code_executors/container/_container_cli.py index 928d158a..dd10fb55 100644 --- a/trpc_agent_sdk/code_executors/container/_container_cli.py +++ b/trpc_agent_sdk/code_executors/container/_container_cli.py @@ -290,9 +290,9 @@ def _exec_run_with_stdin( if callable(close_write): close_write() - frames = frames_iter(sock, tty=False) - demux_frames = (demux_adaptor(*frame) for frame in frames) - output = consume_socket_output(demux_frames, demux=True) + frames = frames_iter(sock, tty=False) # noqa: F821 + demux_frames = (demux_adaptor(*frame) for frame in frames) # noqa: F821 + output = consume_socket_output(demux_frames, demux=True) # noqa: F821 stdout = output[0].decode("utf-8") if output and output[0] else "" stderr = output[1].decode("utf-8") if output and output[1] else "" finally: diff --git a/trpc_agent_sdk/code_executors/utils/_files.py b/trpc_agent_sdk/code_executors/utils/_files.py index 65e811ec..98d6da1b 100644 --- a/trpc_agent_sdk/code_executors/utils/_files.py +++ b/trpc_agent_sdk/code_executors/utils/_files.py @@ -16,6 +16,8 @@ import shutil import sys from pathlib import Path + +from trpc_agent_sdk.log import logger from typing import Optional # NOTE: python-magic is NOT imported at module top level. From f2ff4244ce1129c476d6037973a1e80c94643912 Mon Sep 17 00:00:00 2001 From: Fromsko <1614355756@qq.com> Date: Sat, 25 Jul 2026 00:48:53 +0800 Subject: [PATCH 06/15] tests: fix container_cli mock compatibility with lazy docker import CI test_container_cli.py uses @patch("..._container_cli.docker") which requires the docker name to exist at module level. Call _import_docker() at test import time to ensure the module attribute is present for patching. Fixes 12 CI failures (9405 passed, 0 regressions). --- .../container/test_container_cli.py | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/tests/code_executors/container/test_container_cli.py b/tests/code_executors/container/test_container_cli.py index 19c30aff..e8299e7b 100644 --- a/tests/code_executors/container/test_container_cli.py +++ b/tests/code_executors/container/test_container_cli.py @@ -27,8 +27,14 @@ ContainerClient, ContainerConfig, ) -from trpc_agent_sdk.utils import CommandExecResult +from trpc_agent_sdk.code_executors.container._container_cli import \ + _import_docker # ensure docker is injected for patch compatibility + +# Eagerly import docker into the module namespace so that +# @patch("..._container_cli.docker") works correctly. +_import_docker() +from trpc_agent_sdk.utils import CommandExecResult # --------------------------------------------------------------------------- # ContainerConfig @@ -140,7 +146,7 @@ def test_custom_base_url(self, mock_docker, mock_atexit): @patch("trpc_agent_sdk.code_executors.container._container_cli.atexit") @patch("trpc_agent_sdk.code_executors.container._container_cli.docker") def test_docker_exception_connection_error(self, mock_docker, mock_atexit): - exc_cls = type("DockerException", (Exception,), {}) + exc_cls = type("DockerException", (Exception, ), {}) mock_docker.errors.DockerException = exc_cls mock_docker.from_env.side_effect = exc_cls("Connection refused") @@ -150,7 +156,7 @@ def test_docker_exception_connection_error(self, mock_docker, mock_atexit): @patch("trpc_agent_sdk.code_executors.container._container_cli.atexit") @patch("trpc_agent_sdk.code_executors.container._container_cli.docker") def test_docker_exception_socket_error(self, mock_docker, mock_atexit): - exc_cls = type("DockerException", (Exception,), {}) + exc_cls = type("DockerException", (Exception, ), {}) mock_docker.errors.DockerException = exc_cls mock_docker.from_env.side_effect = exc_cls("No such file or directory") @@ -160,7 +166,7 @@ def test_docker_exception_socket_error(self, mock_docker, mock_atexit): @patch("trpc_agent_sdk.code_executors.container._container_cli.atexit") @patch("trpc_agent_sdk.code_executors.container._container_cli.docker") def test_docker_exception_generic(self, mock_docker, mock_atexit): - exc_cls = type("DockerException", (Exception,), {}) + exc_cls = type("DockerException", (Exception, ), {}) mock_docker.errors.DockerException = exc_cls mock_docker.from_env.side_effect = exc_cls("some other error") @@ -170,7 +176,7 @@ def test_docker_exception_generic(self, mock_docker, mock_atexit): @patch("trpc_agent_sdk.code_executors.container._container_cli.atexit") @patch("trpc_agent_sdk.code_executors.container._container_cli.docker") def test_unexpected_exception(self, mock_docker, mock_atexit): - mock_docker.errors.DockerException = type("DockerException", (Exception,), {}) + mock_docker.errors.DockerException = type("DockerException", (Exception, ), {}) mock_docker.from_env.side_effect = OSError("unexpected") with pytest.raises(RuntimeError, match="Unexpected error initializing Docker client"): @@ -287,8 +293,7 @@ def test_build_image_success(self, mock_docker, mock_atexit, tmp_path): cfg = ContainerConfig(docker_path=docker_dir, image="custom:latest") cc = ContainerClient(config=cfg) - mock_client.images.build.assert_called_once_with( - path=os.path.abspath(docker_dir), tag="custom:latest", rm=True) + mock_client.images.build.assert_called_once_with(path=os.path.abspath(docker_dir), tag="custom:latest", rm=True) @patch("trpc_agent_sdk.code_executors.container._container_cli.atexit") @patch("trpc_agent_sdk.code_executors.container._container_cli.docker") @@ -298,8 +303,7 @@ def test_build_image_path_not_exists(self, mock_docker, mock_atexit): mock_docker.errors.DockerException = Exception with pytest.raises(FileNotFoundError, match="Invalid Docker path"): - ContainerClient(config=ContainerConfig( - docker_path="/nonexistent/docker/path", image="custom:latest")) + ContainerClient(config=ContainerConfig(docker_path="/nonexistent/docker/path", image="custom:latest")) def test_build_image_no_docker_path(self): cc = ContainerClient.__new__(ContainerClient) From 23146805af28605ad7318acf25e871982ff25ef4 Mon Sep 17 00:00:00 2001 From: Fromsko <1614355756@qq.com> Date: Sat, 25 Jul 2026 01:19:57 +0800 Subject: [PATCH 07/15] address final review: restore HAS_MAGIC, add threading lock, cleanup 1. Restore HAS_MAGIC as backward-compatible public alias (updated lazily when magic is successfully imported on non-win32) 2. Add threading.Lock to _import_docker() for concurrent safety 3. Remove dead code: unused `import sys as _sys` in test method 4. All yapf + flake8 clean, 47 passed, 0 regressions --- tests/code_executors/test_utils_files.py | 1 - .../container/_container_cli.py | 32 +++++++++++-------- trpc_agent_sdk/code_executors/utils/_files.py | 18 ++++++++++- 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/tests/code_executors/test_utils_files.py b/tests/code_executors/test_utils_files.py index 98548bab..5c6d36d1 100644 --- a/tests/code_executors/test_utils_files.py +++ b/tests/code_executors/test_utils_files.py @@ -332,7 +332,6 @@ def test_detect_content_type_text_utf8(self): @pytest.mark.skipif(sys.platform == 'win32', reason='python-magic crashes on Windows without libmagic DLL') def test_detect_content_type_with_magic(self): """Test detecting content type using magic library.""" - import sys as _sys from unittest.mock import MagicMock filename = Path("test.unknown") diff --git a/trpc_agent_sdk/code_executors/container/_container_cli.py b/trpc_agent_sdk/code_executors/container/_container_cli.py index dd10fb55..74e6564c 100644 --- a/trpc_agent_sdk/code_executors/container/_container_cli.py +++ b/trpc_agent_sdk/code_executors/container/_container_cli.py @@ -27,7 +27,10 @@ from trpc_agent_sdk.log import logger from trpc_agent_sdk.utils import CommandExecResult +import threading + _docker_imported = False +_docker_lock = threading.Lock() def _import_docker(): @@ -43,19 +46,22 @@ def _import_docker(): global _docker_imported if _docker_imported: return - import docker as _docker - from docker.models.containers import Container as _Container - from docker.utils.socket import consume_socket_output as _cso - from docker.utils.socket import demux_adaptor as _da - from docker.utils.socket import frames_iter as _fi - globals().update({ - 'docker': _docker, - 'Container': _Container, - 'consume_socket_output': _cso, - 'demux_adaptor': _da, - 'frames_iter': _fi, - }) - _docker_imported = True + with _docker_lock: + if _docker_imported: + return + import docker as _docker + from docker.models.containers import Container as _Container + from docker.utils.socket import consume_socket_output as _cso + from docker.utils.socket import demux_adaptor as _da + from docker.utils.socket import frames_iter as _fi + globals().update({ + 'docker': _docker, + 'Container': _Container, + 'consume_socket_output': _cso, + 'demux_adaptor': _da, + 'frames_iter': _fi, + }) + _docker_imported = True DEFAULT_IMAGE_TAG = 'python:3-slim' diff --git a/trpc_agent_sdk/code_executors/utils/_files.py b/trpc_agent_sdk/code_executors/utils/_files.py index 98d6da1b..bac67cb4 100644 --- a/trpc_agent_sdk/code_executors/utils/_files.py +++ b/trpc_agent_sdk/code_executors/utils/_files.py @@ -30,6 +30,21 @@ _magic_checked = False +def _has_magic() -> bool: + """Backward-compatible indicator for whether python-magic is available. + + Mirrors the old module-level ``HAS_MAGIC`` boolean. Returns ``True`` only + when the magic module has been successfully imported (or explicitly + injected by tests). + """ + return _magic_module is not None + + +# Backward-compatible public alias (was a module-level bool before this PR). +# External code may reference it as ``from ..._files import HAS_MAGIC``. +HAS_MAGIC = False # updated lazily; see _has_magic() for the live value + + def path_join(base: str, path: str) -> str: """Join a base path and a path. @@ -239,11 +254,12 @@ def detect_content_type(filename: Path, data: bytes) -> str: # missing, causes an access violation at import time (not catchable by # try/except). We skip it entirely on win32 and fall through to the # simple content-based detection below. - global _magic_module, _magic_checked + global _magic_module, _magic_checked, HAS_MAGIC if sys.platform != 'win32' and not _magic_checked: try: import magic as _m _magic_module = _m + HAS_MAGIC = True _magic_checked = True except ImportError: logger.debug("python-magic not available; falling back to byte-signature detection") From 2e13abb4074f4de7d5190e0682bb991a63fe6771 Mon Sep 17 00:00:00 2001 From: Fromsko <1614355756@qq.com> Date: Sat, 25 Jul 2026 01:37:29 +0800 Subject: [PATCH 08/15] refactor: use explicit module-level placeholders instead of globals().update Address AI review Critical: eliminate # noqa: F821 and NameError risk by declaring docker/Container/consume_socket_output/demux_adaptor/ frames_iter as module-level None placeholders, populated by _import_docker() via direct assignment instead of globals().update. - Remove TYPE_CHECKING guard (placeholders serve the same purpose) - Remove all # noqa: F821 comments - _exec_run_with_stdin now references real module variables, not dynamically injected names - Tests @patch works naturally without _import_docker() side effects - yapf + flake8 clean, 47 passed, 0 regressions --- .../container/_container_cli.py | 45 ++++++++++++------- 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/trpc_agent_sdk/code_executors/container/_container_cli.py b/trpc_agent_sdk/code_executors/container/_container_cli.py index 74e6564c..0f04d15d 100644 --- a/trpc_agent_sdk/code_executors/container/_container_cli.py +++ b/trpc_agent_sdk/code_executors/container/_container_cli.py @@ -14,21 +14,36 @@ import os import socket as pysocket from dataclasses import dataclass -from typing import TYPE_CHECKING, Optional +from typing import Optional # Note: Docker SDK imports (docker, docker.models.containers, docker.utils.socket) # are deferred to runtime via _import_docker() to avoid hanging/crashing on # systems without Docker installed (e.g. Windows without Docker Desktop). # See: https://github.com/trpc-group/trpc-agent-python/issues/230 -if TYPE_CHECKING: - import docker - from docker.models.containers import Container +# +# Module-level placeholders are declared below (docker=None, Container=None, etc.) +# and populated by _import_docker() on first real use. This approach: +# - eliminates F821 flake8 errors (no # noqa needed) +# - provides clear NameError-free fallback if called before init +# - allows @patch("..._container_cli.docker") to work in tests +# Type annotations use string literals via `from __future__ import annotations`. from trpc_agent_sdk.log import logger from trpc_agent_sdk.utils import CommandExecResult import threading +# Module-level placeholders for Docker SDK symbols. +# These are populated by _import_docker() on first use, which avoids +# hanging/crashing on systems without Docker installed. +# Declaring them here eliminates F821 and ensures _exec_run_with_stdin +# gets a clear RuntimeError instead of NameError if called before init. +docker = None +Container = None +consume_socket_output = None +demux_adaptor = None +frames_iter = None + _docker_imported = False _docker_lock = threading.Lock() @@ -43,24 +58,22 @@ def _import_docker(): import to call-time we ensure that users who never touch ``ContainerCodeExecutor`` are unaffected. """ - global _docker_imported + global _docker_imported, docker, Container, consume_socket_output, demux_adaptor, frames_iter if _docker_imported: return with _docker_lock: if _docker_imported: return - import docker as _docker + import docker as _docker_mod from docker.models.containers import Container as _Container from docker.utils.socket import consume_socket_output as _cso from docker.utils.socket import demux_adaptor as _da from docker.utils.socket import frames_iter as _fi - globals().update({ - 'docker': _docker, - 'Container': _Container, - 'consume_socket_output': _cso, - 'demux_adaptor': _da, - 'frames_iter': _fi, - }) + docker = _docker_mod + Container = _Container + consume_socket_output = _cso + demux_adaptor = _da + frames_iter = _fi _docker_imported = True @@ -296,9 +309,9 @@ def _exec_run_with_stdin( if callable(close_write): close_write() - frames = frames_iter(sock, tty=False) # noqa: F821 - demux_frames = (demux_adaptor(*frame) for frame in frames) # noqa: F821 - output = consume_socket_output(demux_frames, demux=True) # noqa: F821 + frames = frames_iter(sock, tty=False) + demux_frames = (demux_adaptor(*frame) for frame in frames) + output = consume_socket_output(demux_frames, demux=True) stdout = output[0].decode("utf-8") if output and output[0] else "" stderr = output[1].decode("utf-8") if output and output[1] else "" finally: From 1e6e190f4c78805dd829d457be95d9af4cdcbefc Mon Sep 17 00:00:00 2001 From: Fromsko <1614355756@qq.com> Date: Sat, 25 Jul 2026 01:49:47 +0800 Subject: [PATCH 09/15] fix: widen magic import exception handling and restore HAS_MAGIC in tests Address latest AI review: 1. Change except ImportError to except Exception for magic import failure, catching OSError and other non-Import failures safely 2. Include HAS_MAGIC in test mock save/restore to prevent state leakage 3. Add exc_info=True to the debug log for better diagnostics --- tests/code_executors/test_utils_files.py | 5 +++-- trpc_agent_sdk/code_executors/utils/_files.py | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/code_executors/test_utils_files.py b/tests/code_executors/test_utils_files.py index 5c6d36d1..441fa3e1 100644 --- a/tests/code_executors/test_utils_files.py +++ b/tests/code_executors/test_utils_files.py @@ -341,15 +341,16 @@ def test_detect_content_type_with_magic(self): # Inject mock into the module's cached magic slot import trpc_agent_sdk.code_executors.utils._files as _f - orig = (_f._magic_module, _f._magic_checked) + orig = (_f._magic_module, _f._magic_checked, _f.HAS_MAGIC) _f._magic_module = mock_magic _f._magic_checked = True + _f.HAS_MAGIC = True try: mime_type = detect_content_type(filename, data) assert mime_type == "application/custom" mock_magic.from_buffer.assert_called_once_with(data, mime=True) finally: - _f._magic_module, _f._magic_checked = orig + _f._magic_module, _f._magic_checked, _f.HAS_MAGIC = orig class TestGetRelPath: diff --git a/trpc_agent_sdk/code_executors/utils/_files.py b/trpc_agent_sdk/code_executors/utils/_files.py index bac67cb4..903d9334 100644 --- a/trpc_agent_sdk/code_executors/utils/_files.py +++ b/trpc_agent_sdk/code_executors/utils/_files.py @@ -261,8 +261,8 @@ def detect_content_type(filename: Path, data: bytes) -> str: _magic_module = _m HAS_MAGIC = True _magic_checked = True - except ImportError: - logger.debug("python-magic not available; falling back to byte-signature detection") + except Exception: + logger.debug("python-magic import failed; falling back to byte-signature detection", exc_info=True) _magic_checked = True # cache failure to avoid retrying every call if _magic_module is not None: try: From 0949e38529b39ffe97bc03c37b2b7bf1628dc40e Mon Sep 17 00:00:00 2001 From: Fromsko <1614355756@qq.com> Date: Sat, 25 Jul 2026 02:07:04 +0800 Subject: [PATCH 10/15] fix: guard docker=None before except block to prevent AttributeError Address AI review Critical: if _import_docker() fails silently and docker remains None, the except docker.errors.DockerException line would throw AttributeError: NoneType has no attribute errors, masking the real error. Now explicitly checks docker is not None after import and raises a clear RuntimeError with install instructions. --- trpc_agent_sdk/code_executors/container/_container_cli.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/trpc_agent_sdk/code_executors/container/_container_cli.py b/trpc_agent_sdk/code_executors/container/_container_cli.py index 0f04d15d..a1c541e5 100644 --- a/trpc_agent_sdk/code_executors/container/_container_cli.py +++ b/trpc_agent_sdk/code_executors/container/_container_cli.py @@ -143,6 +143,10 @@ def _init_docker_client(self): """ # Lazily import the Docker SDK on first real use. _import_docker() + # Guard: if docker is still None after import attempt (e.g. SDK not installed), + # fail with a clear error rather than AttributeError later. + if docker is None: + raise RuntimeError("Docker SDK is not available. Install it with: pip install docker") # Try to initialize Docker client # Let docker SDK handle connection detection (it supports various methods) try: From ba298ca4d2d396e9138dee0014e083c44dc88bfc Mon Sep 17 00:00:00 2001 From: Fromsko <1614355756@qq.com> Date: Sat, 25 Jul 2026 02:25:00 +0800 Subject: [PATCH 11/15] fix: make docker=None guard reachable and restore HAS_MAGIC import-time semantics 1. _import_docker(): catch ImportError so docker stays None when SDK is not installed, making the `if docker is None` guard in _init_docker_client() actually reachable 2. _files.py: probe magic at module import time on non-win32 to set HAS_MAGIC immediately, preserving the original semantics where external code can check HAS_MAGIC right after import --- .../container/_container_cli.py | 26 ++++++++++++------- trpc_agent_sdk/code_executors/utils/_files.py | 11 +++++++- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/trpc_agent_sdk/code_executors/container/_container_cli.py b/trpc_agent_sdk/code_executors/container/_container_cli.py index a1c541e5..9b2bf284 100644 --- a/trpc_agent_sdk/code_executors/container/_container_cli.py +++ b/trpc_agent_sdk/code_executors/container/_container_cli.py @@ -57,6 +57,9 @@ def _import_docker(): making the entire ``trpc_agent_sdk`` package unusable. By deferring the import to call-time we ensure that users who never touch ``ContainerCodeExecutor`` are unaffected. + + If the docker package is not installed, ``docker`` stays ``None`` and + callers should check via the ``docker is None`` guard. """ global _docker_imported, docker, Container, consume_socket_output, demux_adaptor, frames_iter if _docker_imported: @@ -64,16 +67,19 @@ def _import_docker(): with _docker_lock: if _docker_imported: return - import docker as _docker_mod - from docker.models.containers import Container as _Container - from docker.utils.socket import consume_socket_output as _cso - from docker.utils.socket import demux_adaptor as _da - from docker.utils.socket import frames_iter as _fi - docker = _docker_mod - Container = _Container - consume_socket_output = _cso - demux_adaptor = _da - frames_iter = _fi + try: + import docker as _docker_mod + from docker.models.containers import Container as _Container + from docker.utils.socket import consume_socket_output as _cso + from docker.utils.socket import demux_adaptor as _da + from docker.utils.socket import frames_iter as _fi + docker = _docker_mod + Container = _Container + consume_socket_output = _cso + demux_adaptor = _da + frames_iter = _fi + except ImportError: + pass # docker stays None; caller checks `if docker is None` _docker_imported = True diff --git a/trpc_agent_sdk/code_executors/utils/_files.py b/trpc_agent_sdk/code_executors/utils/_files.py index 903d9334..9299fc33 100644 --- a/trpc_agent_sdk/code_executors/utils/_files.py +++ b/trpc_agent_sdk/code_executors/utils/_files.py @@ -42,7 +42,16 @@ def _has_magic() -> bool: # Backward-compatible public alias (was a module-level bool before this PR). # External code may reference it as ``from ..._files import HAS_MAGIC``. -HAS_MAGIC = False # updated lazily; see _has_magic() for the live value +# On non-win32, probe once at import time to preserve the original semantics +# (HAS_MAGIC reflects availability immediately, not deferred to first call). +if sys.platform != 'win32': + try: + import magic as _magic_mod + _magic_module = _magic_mod + _magic_checked = True + except Exception: + _magic_checked = True +HAS_MAGIC = _magic_module is not None def path_join(base: str, path: str) -> str: From d72f875f719625729902c1e585b8698c22f04956 Mon Sep 17 00:00:00 2001 From: Fromsko <1614355756@qq.com> Date: Sat, 25 Jul 2026 02:32:03 +0800 Subject: [PATCH 12/15] fix: move _import_docker to fixture, add magic thread lock 1. test_container_cli.py: replace module-level _import_docker() call with autouse module-scoped fixture, removing import-time global side effects and fixing import ordering (all imports at top) 2. _files.py: add threading.Lock for magic lazy import to prevent concurrent race condition on _magic_module/_magic_checked state 3. Consolidate _magic_checked=True inside lock for atomic state update --- .../container/test_container_cli.py | 16 ++++++++++----- trpc_agent_sdk/code_executors/utils/_files.py | 20 +++++++++++-------- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/tests/code_executors/container/test_container_cli.py b/tests/code_executors/container/test_container_cli.py index e8299e7b..92e42e4a 100644 --- a/tests/code_executors/container/test_container_cli.py +++ b/tests/code_executors/container/test_container_cli.py @@ -28,13 +28,19 @@ ContainerConfig, ) from trpc_agent_sdk.code_executors.container._container_cli import \ - _import_docker # ensure docker is injected for patch compatibility + _import_docker # noqa: F401 (used in fixture below) +from trpc_agent_sdk.utils import CommandExecResult -# Eagerly import docker into the module namespace so that -# @patch("..._container_cli.docker") works correctly. -_import_docker() -from trpc_agent_sdk.utils import CommandExecResult +@pytest.fixture(autouse=True, scope="module") +def _ensure_docker_imported(): + """Ensure docker SDK symbols exist at module level for @patch compatibility. + + Without this, @patch("..._container_cli.docker") fails because docker + is a None placeholder until _import_docker() runs. + """ + _import_docker() + # --------------------------------------------------------------------------- # ContainerConfig diff --git a/trpc_agent_sdk/code_executors/utils/_files.py b/trpc_agent_sdk/code_executors/utils/_files.py index 9299fc33..3b14a38b 100644 --- a/trpc_agent_sdk/code_executors/utils/_files.py +++ b/trpc_agent_sdk/code_executors/utils/_files.py @@ -26,8 +26,11 @@ # or crash the interpreter — making the entire trpc_agent_sdk package unusable. # Instead, we lazily import it inside detect_content_type() on first use. # See: https://github.com/trpc-group/trpc-agent-python/issues/230 +import threading as _threading + _magic_module = None # cached after first successful import on non-win32 _magic_checked = False +_magic_lock = _threading.Lock() def _has_magic() -> bool: @@ -265,14 +268,15 @@ def detect_content_type(filename: Path, data: bytes) -> str: # simple content-based detection below. global _magic_module, _magic_checked, HAS_MAGIC if sys.platform != 'win32' and not _magic_checked: - try: - import magic as _m - _magic_module = _m - HAS_MAGIC = True - _magic_checked = True - except Exception: - logger.debug("python-magic import failed; falling back to byte-signature detection", exc_info=True) - _magic_checked = True # cache failure to avoid retrying every call + with _magic_lock: + if not _magic_checked: + try: + import magic as _m + _magic_module = _m + HAS_MAGIC = True + except Exception: + logger.debug("python-magic import failed; falling back to byte-signature detection", exc_info=True) + _magic_checked = True # cache result (success or failure) if _magic_module is not None: try: return _magic_module.from_buffer(data, mime=True) From f493e18aef38d5bb90a3e73077a53d53393fcee8 Mon Sep 17 00:00:00 2001 From: Fromsko <1614355756@qq.com> Date: Sat, 25 Jul 2026 02:40:53 +0800 Subject: [PATCH 13/15] refactor: use TYPE_CHECKING + private _docker_* names, no public shadowing Fix AI review Critical: module-level docker=None/Container=None shadowed the TYPE_CHECKING type aliases, breaking external `from ..._container_cli import Container`. - Use TYPE_CHECKING guard for docker/Container type annotations (static only) - Store runtime symbols in private _docker_mod/_docker_container_cls/ _docker_consume_socket_output/_docker_demux_adaptor/_docker_frames_iter - Update all runtime references to use private names - Update test @patch targets to _docker_mod - No public module-level names are shadowed --- .../container/test_container_cli.py | 26 +++---- .../container/_container_cli.py | 68 +++++++++---------- 2 files changed, 46 insertions(+), 48 deletions(-) diff --git a/tests/code_executors/container/test_container_cli.py b/tests/code_executors/container/test_container_cli.py index 92e42e4a..9c41e38d 100644 --- a/tests/code_executors/container/test_container_cli.py +++ b/tests/code_executors/container/test_container_cli.py @@ -36,7 +36,7 @@ def _ensure_docker_imported(): """Ensure docker SDK symbols exist at module level for @patch compatibility. - Without this, @patch("..._container_cli.docker") fails because docker + Without this, @patch("..._container_cli._docker_mod") fails because docker is a None placeholder until _import_docker() runs. """ _import_docker() @@ -119,7 +119,7 @@ def _make_mock_container(exec_exit_code=0): class TestContainerClientInitDockerClient: @patch("trpc_agent_sdk.code_executors.container._container_cli.atexit") - @patch("trpc_agent_sdk.code_executors.container._container_cli.docker") + @patch("trpc_agent_sdk.code_executors.container._container_cli._docker_mod") def test_from_env_when_no_base_url(self, mock_docker, mock_atexit): mock_client = _make_mock_docker_client() mock_docker.from_env.return_value = mock_client @@ -135,7 +135,7 @@ def test_from_env_when_no_base_url(self, mock_docker, mock_atexit): assert cc.client is mock_client @patch("trpc_agent_sdk.code_executors.container._container_cli.atexit") - @patch("trpc_agent_sdk.code_executors.container._container_cli.docker") + @patch("trpc_agent_sdk.code_executors.container._container_cli._docker_mod") def test_custom_base_url(self, mock_docker, mock_atexit): mock_client = _make_mock_docker_client() mock_docker.DockerClient.return_value = mock_client @@ -150,7 +150,7 @@ def test_custom_base_url(self, mock_docker, mock_atexit): assert cc.base_url == "tcp://remote:2375" @patch("trpc_agent_sdk.code_executors.container._container_cli.atexit") - @patch("trpc_agent_sdk.code_executors.container._container_cli.docker") + @patch("trpc_agent_sdk.code_executors.container._container_cli._docker_mod") def test_docker_exception_connection_error(self, mock_docker, mock_atexit): exc_cls = type("DockerException", (Exception, ), {}) mock_docker.errors.DockerException = exc_cls @@ -160,7 +160,7 @@ def test_docker_exception_connection_error(self, mock_docker, mock_atexit): ContainerClient(config=ContainerConfig()) @patch("trpc_agent_sdk.code_executors.container._container_cli.atexit") - @patch("trpc_agent_sdk.code_executors.container._container_cli.docker") + @patch("trpc_agent_sdk.code_executors.container._container_cli._docker_mod") def test_docker_exception_socket_error(self, mock_docker, mock_atexit): exc_cls = type("DockerException", (Exception, ), {}) mock_docker.errors.DockerException = exc_cls @@ -170,7 +170,7 @@ def test_docker_exception_socket_error(self, mock_docker, mock_atexit): ContainerClient(config=ContainerConfig()) @patch("trpc_agent_sdk.code_executors.container._container_cli.atexit") - @patch("trpc_agent_sdk.code_executors.container._container_cli.docker") + @patch("trpc_agent_sdk.code_executors.container._container_cli._docker_mod") def test_docker_exception_generic(self, mock_docker, mock_atexit): exc_cls = type("DockerException", (Exception, ), {}) mock_docker.errors.DockerException = exc_cls @@ -180,7 +180,7 @@ def test_docker_exception_generic(self, mock_docker, mock_atexit): ContainerClient(config=ContainerConfig()) @patch("trpc_agent_sdk.code_executors.container._container_cli.atexit") - @patch("trpc_agent_sdk.code_executors.container._container_cli.docker") + @patch("trpc_agent_sdk.code_executors.container._container_cli._docker_mod") def test_unexpected_exception(self, mock_docker, mock_atexit): mock_docker.errors.DockerException = type("DockerException", (Exception, ), {}) mock_docker.from_env.side_effect = OSError("unexpected") @@ -197,7 +197,7 @@ def test_unexpected_exception(self, mock_docker, mock_atexit): class TestContainerClientInitContainer: @patch("trpc_agent_sdk.code_executors.container._container_cli.atexit") - @patch("trpc_agent_sdk.code_executors.container._container_cli.docker") + @patch("trpc_agent_sdk.code_executors.container._container_cli._docker_mod") def test_container_starts_and_verifies_python(self, mock_docker, mock_atexit): mock_client = _make_mock_docker_client() mock_docker.from_env.return_value = mock_client @@ -222,7 +222,7 @@ def test_container_starts_and_verifies_python(self, mock_docker, mock_atexit): assert cc.container is mock_container @patch("trpc_agent_sdk.code_executors.container._container_cli.atexit") - @patch("trpc_agent_sdk.code_executors.container._container_cli.docker") + @patch("trpc_agent_sdk.code_executors.container._container_cli._docker_mod") def test_container_with_bind_mounts(self, mock_docker, mock_atexit): mock_client = _make_mock_docker_client() mock_docker.from_env.return_value = mock_client @@ -247,7 +247,7 @@ def test_container_with_bind_mounts(self, mock_docker, mock_atexit): ) @patch("trpc_agent_sdk.code_executors.container._container_cli.atexit") - @patch("trpc_agent_sdk.code_executors.container._container_cli.docker") + @patch("trpc_agent_sdk.code_executors.container._container_cli._docker_mod") def test_python3_not_installed_raises(self, mock_docker, mock_atexit): mock_client = _make_mock_docker_client() mock_docker.from_env.return_value = mock_client @@ -260,7 +260,7 @@ def test_python3_not_installed_raises(self, mock_docker, mock_atexit): ContainerClient(config=ContainerConfig()) @patch("trpc_agent_sdk.code_executors.container._container_cli.atexit") - @patch("trpc_agent_sdk.code_executors.container._container_cli.docker") + @patch("trpc_agent_sdk.code_executors.container._container_cli._docker_mod") def test_init_container_client_not_initialized(self, mock_docker, mock_atexit): mock_client = _make_mock_docker_client() mock_docker.from_env.return_value = mock_client @@ -286,7 +286,7 @@ def test_init_container_client_not_initialized(self, mock_docker, mock_atexit): class TestContainerClientBuildDockerImage: @patch("trpc_agent_sdk.code_executors.container._container_cli.atexit") - @patch("trpc_agent_sdk.code_executors.container._container_cli.docker") + @patch("trpc_agent_sdk.code_executors.container._container_cli._docker_mod") def test_build_image_success(self, mock_docker, mock_atexit, tmp_path): mock_client = _make_mock_docker_client() mock_docker.from_env.return_value = mock_client @@ -302,7 +302,7 @@ def test_build_image_success(self, mock_docker, mock_atexit, tmp_path): mock_client.images.build.assert_called_once_with(path=os.path.abspath(docker_dir), tag="custom:latest", rm=True) @patch("trpc_agent_sdk.code_executors.container._container_cli.atexit") - @patch("trpc_agent_sdk.code_executors.container._container_cli.docker") + @patch("trpc_agent_sdk.code_executors.container._container_cli._docker_mod") def test_build_image_path_not_exists(self, mock_docker, mock_atexit): mock_client = _make_mock_docker_client() mock_docker.from_env.return_value = mock_client diff --git a/trpc_agent_sdk/code_executors/container/_container_cli.py b/trpc_agent_sdk/code_executors/container/_container_cli.py index 9b2bf284..d1fd60f4 100644 --- a/trpc_agent_sdk/code_executors/container/_container_cli.py +++ b/trpc_agent_sdk/code_executors/container/_container_cli.py @@ -14,35 +14,32 @@ import os import socket as pysocket from dataclasses import dataclass -from typing import Optional +from typing import TYPE_CHECKING, Optional # Note: Docker SDK imports (docker, docker.models.containers, docker.utils.socket) # are deferred to runtime via _import_docker() to avoid hanging/crashing on # systems without Docker installed (e.g. Windows without Docker Desktop). # See: https://github.com/trpc-group/trpc-agent-python/issues/230 # -# Module-level placeholders are declared below (docker=None, Container=None, etc.) -# and populated by _import_docker() on first real use. This approach: -# - eliminates F821 flake8 errors (no # noqa needed) -# - provides clear NameError-free fallback if called before init -# - allows @patch("..._container_cli.docker") to work in tests -# Type annotations use string literals via `from __future__ import annotations`. +# TYPE_CHECKING imports provide type info for static analysis without triggering +# a runtime import. _import_docker() populates runtime symbols on first use. +if TYPE_CHECKING: + import docker + from docker.models.containers import Container from trpc_agent_sdk.log import logger from trpc_agent_sdk.utils import CommandExecResult import threading -# Module-level placeholders for Docker SDK symbols. -# These are populated by _import_docker() on first use, which avoids -# hanging/crashing on systems without Docker installed. -# Declaring them here eliminates F821 and ensures _exec_run_with_stdin -# gets a clear RuntimeError instead of NameError if called before init. -docker = None -Container = None -consume_socket_output = None -demux_adaptor = None -frames_iter = None +# Runtime cache for Docker SDK symbols (populated by _import_docker()). +# Not exposed as module-level public names to avoid shadowing the +# TYPE_CHECKING type aliases above. +_docker_mod = None +_docker_container_cls = None +_docker_consume_socket_output = None +_docker_demux_adaptor = None +_docker_frames_iter = None _docker_imported = False _docker_lock = threading.Lock() @@ -58,28 +55,29 @@ def _import_docker(): import to call-time we ensure that users who never touch ``ContainerCodeExecutor`` are unaffected. - If the docker package is not installed, ``docker`` stays ``None`` and - callers should check via the ``docker is None`` guard. + If the docker package is not installed, ``_docker_mod`` stays ``None`` and + callers should check via the ``_docker_mod is None`` guard. """ - global _docker_imported, docker, Container, consume_socket_output, demux_adaptor, frames_iter + global _docker_imported, _docker_mod, _docker_container_cls + global _docker_consume_socket_output, _docker_demux_adaptor, _docker_frames_iter if _docker_imported: return with _docker_lock: if _docker_imported: return try: - import docker as _docker_mod - from docker.models.containers import Container as _Container + import docker as _d + from docker.models.containers import Container as _C from docker.utils.socket import consume_socket_output as _cso from docker.utils.socket import demux_adaptor as _da from docker.utils.socket import frames_iter as _fi - docker = _docker_mod - Container = _Container - consume_socket_output = _cso - demux_adaptor = _da - frames_iter = _fi + _docker_mod = _d + _docker_container_cls = _C + _docker_consume_socket_output = _cso + _docker_demux_adaptor = _da + _docker_frames_iter = _fi except ImportError: - pass # docker stays None; caller checks `if docker is None` + pass # _docker_mod stays None; caller checks guard _docker_imported = True @@ -151,26 +149,26 @@ def _init_docker_client(self): _import_docker() # Guard: if docker is still None after import attempt (e.g. SDK not installed), # fail with a clear error rather than AttributeError later. - if docker is None: + if _docker_mod is None: raise RuntimeError("Docker SDK is not available. Install it with: pip install docker") # Try to initialize Docker client # Let docker SDK handle connection detection (it supports various methods) try: if self.base_url: # Use custom base_url if provided - self._client = docker.DockerClient(base_url=self.base_url) + self._client = _docker_mod.DockerClient(base_url=self.base_url) else: # Use docker.from_env() which automatically detects: # - DOCKER_HOST environment variable # - Standard socket paths # - Docker Desktop configurations - self._client = docker.from_env() + self._client = _docker_mod.from_env() # Test connection by pinging Docker daemon # This will fail if Docker is not running or not accessible self._client.ping() logger.info("Docker client initialized successfully") - except docker.errors.DockerException as ex: + except _docker_mod.errors.DockerException as ex: # Extract more specific error information error_str = str(ex) @@ -319,9 +317,9 @@ def _exec_run_with_stdin( if callable(close_write): close_write() - frames = frames_iter(sock, tty=False) - demux_frames = (demux_adaptor(*frame) for frame in frames) - output = consume_socket_output(demux_frames, demux=True) + frames = _docker_frames_iter(sock, tty=False) + demux_frames = (_docker_demux_adaptor(*frame) for frame in frames) + output = _docker_consume_socket_output(demux_frames, demux=True) stdout = output[0].decode("utf-8") if output and output[0] else "" stderr = output[1].decode("utf-8") if output and output[1] else "" finally: From 8e676d242d37572747de9c2c094049a96c897bb5 Mon Sep 17 00:00:00 2001 From: Fromsko <1614355756@qq.com> Date: Sat, 25 Jul 2026 02:49:15 +0800 Subject: [PATCH 14/15] fix: fully lazy magic import, remove dead code, widen docker except 1. _files.py: remove module-level import magic on non-win32 entirely (was still triggering at import time). Now truly deferred to first detect_content_type() call. HAS_MAGIC defaults False, updated lazily. 2. Remove unused _has_magic() function (dead code per review) 3. _container_cli.py: remove unused _docker_container_cls 4. _container_cli.py: widen except ImportError to except Exception ensuring _docker_imported=True always set, preventing retry storms --- .../container/_container_cli.py | 7 ++--- trpc_agent_sdk/code_executors/utils/_files.py | 28 ++++--------------- 2 files changed, 7 insertions(+), 28 deletions(-) diff --git a/trpc_agent_sdk/code_executors/container/_container_cli.py b/trpc_agent_sdk/code_executors/container/_container_cli.py index d1fd60f4..6120b81e 100644 --- a/trpc_agent_sdk/code_executors/container/_container_cli.py +++ b/trpc_agent_sdk/code_executors/container/_container_cli.py @@ -36,7 +36,6 @@ # Not exposed as module-level public names to avoid shadowing the # TYPE_CHECKING type aliases above. _docker_mod = None -_docker_container_cls = None _docker_consume_socket_output = None _docker_demux_adaptor = None _docker_frames_iter = None @@ -58,7 +57,7 @@ def _import_docker(): If the docker package is not installed, ``_docker_mod`` stays ``None`` and callers should check via the ``_docker_mod is None`` guard. """ - global _docker_imported, _docker_mod, _docker_container_cls + global _docker_imported, _docker_mod global _docker_consume_socket_output, _docker_demux_adaptor, _docker_frames_iter if _docker_imported: return @@ -67,16 +66,14 @@ def _import_docker(): return try: import docker as _d - from docker.models.containers import Container as _C from docker.utils.socket import consume_socket_output as _cso from docker.utils.socket import demux_adaptor as _da from docker.utils.socket import frames_iter as _fi _docker_mod = _d - _docker_container_cls = _C _docker_consume_socket_output = _cso _docker_demux_adaptor = _da _docker_frames_iter = _fi - except ImportError: + except Exception: pass # _docker_mod stays None; caller checks guard _docker_imported = True diff --git a/trpc_agent_sdk/code_executors/utils/_files.py b/trpc_agent_sdk/code_executors/utils/_files.py index 3b14a38b..435f2a04 100644 --- a/trpc_agent_sdk/code_executors/utils/_files.py +++ b/trpc_agent_sdk/code_executors/utils/_files.py @@ -32,29 +32,11 @@ _magic_checked = False _magic_lock = _threading.Lock() - -def _has_magic() -> bool: - """Backward-compatible indicator for whether python-magic is available. - - Mirrors the old module-level ``HAS_MAGIC`` boolean. Returns ``True`` only - when the magic module has been successfully imported (or explicitly - injected by tests). - """ - return _magic_module is not None - - -# Backward-compatible public alias (was a module-level bool before this PR). -# External code may reference it as ``from ..._files import HAS_MAGIC``. -# On non-win32, probe once at import time to preserve the original semantics -# (HAS_MAGIC reflects availability immediately, not deferred to first call). -if sys.platform != 'win32': - try: - import magic as _magic_mod - _magic_module = _magic_mod - _magic_checked = True - except Exception: - _magic_checked = True -HAS_MAGIC = _magic_module is not None +# Backward-compatible public alias. Remains False until first +# detect_content_type() call triggers lazy import on non-win32. +# Note: on win32 this is always False (python-magic requires libmagic DLL +# which causes access violations at import time). +HAS_MAGIC = False def path_join(base: str, path: str) -> str: From 85b8deffab5cad725cb25ad3d9ffadfd2f06c72f Mon Sep 17 00:00:00 2001 From: Fromsko <1614355756@qq.com> Date: Sat, 25 Jul 2026 02:57:29 +0800 Subject: [PATCH 15/15] polish: add logger.debug for docker import failure, fix import ordering Address latest AI review warnings (no Critical this round): 1. Add logger.debug(exc_info=True) for _import_docker except path to aid debugging when docker import fails for non-obvious reasons 2. Move import threading to standard library group at top of file 3. Remove duplicate import threading below the third-party imports --- trpc_agent_sdk/code_executors/container/_container_cli.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/trpc_agent_sdk/code_executors/container/_container_cli.py b/trpc_agent_sdk/code_executors/container/_container_cli.py index 6120b81e..5e96114d 100644 --- a/trpc_agent_sdk/code_executors/container/_container_cli.py +++ b/trpc_agent_sdk/code_executors/container/_container_cli.py @@ -13,6 +13,7 @@ import atexit import os import socket as pysocket +import threading from dataclasses import dataclass from typing import TYPE_CHECKING, Optional @@ -30,9 +31,7 @@ from trpc_agent_sdk.log import logger from trpc_agent_sdk.utils import CommandExecResult -import threading - -# Runtime cache for Docker SDK symbols (populated by _import_docker()). +# Runtime cache # Not exposed as module-level public names to avoid shadowing the # TYPE_CHECKING type aliases above. _docker_mod = None @@ -74,7 +73,7 @@ def _import_docker(): _docker_demux_adaptor = _da _docker_frames_iter = _fi except Exception: - pass # _docker_mod stays None; caller checks guard + logger.debug("Docker SDK import failed; _docker_mod stays None", exc_info=True) _docker_imported = True