Skip to content

Commit 0da8811

Browse files
leeyyicursoragent
andcommitted
refactor: 按 code review 重构 code_executors,强化 cube 子包的依赖契约
主要变更(按评审意见整理): - code_executors 父包不再 lazy 重导出 Cube 符号;业务方需显式 `from trpc_agent_sdk.code_executors.cube import ...`,让 [cube] 依赖在调用点显性暴露,避免错把可选依赖伪装成核心依赖。 - 移除 cube 子包"未装 [cube] 也能 import"的隐含契约: `cube/_e2b.py` 删除,`_sandbox.py` / `_code_executor.py` 改为顶层 `import e2b_code_interpreter as e2b`;缺失 extra 时直接以 ImportError 在 import 处失败,而非延迟到 sandbox 打开时。 - 工作空间收集流水线:删除 utils/_collect.py,将 `build_code_files` / `build_manifest_output` 上提为 `BaseWorkspaceFS._build_code_files` / `._build_manifest_output` 保护静态方法;local/container/cube 三个后端通过 self.* 调用, 子类可直接复用或重写。`max_read_size` 由 `Optional[int]=None` 改为 `int = MAX_READ_SIZE_BYTES`,签名更直接。 - `CubeCodeExecutor.code_block_delimiters` 改用 `default_factory` 复用基类默认值并追加 bash fence,避免重复声明 tool_code/python; 父类未来扩展默认列表时自动跟随。 - `cube/_runtime.py::_copy_remote`:rm -rf 调用加 Safety 注释, 记录 normalize_remote_relative + join_remote + shell_quote + GNU rm `--preserve-root` 的多层校验不变量,作为后续维护合同。 测试同步: - 新增 tests/code_executors/test_base_workspace_fs_collect.py,针对 BaseWorkspaceFS 上的保护方法测试,并新增子类覆写用例。 - tests/code_executors/cube/conftest.py 重写 `fake_e2b` fixture, patch `_sandbox.e2b` / `_code_executor.e2b` 模块级符号。 - 删除 test_e2b.py、test_package_lazy_import.py,新增 test_package_imports.py(保留父包 non-reexport + 父包 import 不触发 e2b 的两条契约)。 - 其余测试更新 monkeypatch 目标到新的实现位置。 Assisted-by: Cursor:claude-opus-4.7 Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 50d8995 commit 0da8811

23 files changed

Lines changed: 741 additions & 703 deletions

examples/code_executors/cube_demo.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,9 @@
3434
import os
3535
import sys
3636

37-
from trpc_agent_sdk.code_executors import CubeCodeExecutor
38-
from trpc_agent_sdk.code_executors import CubeCodeExecutorConfig
39-
from trpc_agent_sdk.code_executors import create_cube_workspace_runtime
37+
from trpc_agent_sdk.code_executors.cube import CubeCodeExecutor
38+
from trpc_agent_sdk.code_executors.cube import CubeCodeExecutorConfig
39+
from trpc_agent_sdk.code_executors.cube import create_cube_workspace_runtime
4040
from trpc_agent_sdk.code_executors._types import CodeBlock
4141
from trpc_agent_sdk.code_executors._types import CodeExecutionInput
4242
from trpc_agent_sdk.code_executors._types import WorkspaceOutputSpec

tests/code_executors/cube/conftest.py

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,18 @@
55
# tRPC-Agent-Python is licensed under Apache-2.0.
66
"""Shared fixtures for the cube/ test suite.
77
8-
Exposes a ``fake_e2b`` fixture that patches
9-
:func:`trpc_agent_sdk.code_executors.cube._e2b._import_e2b` to return a
10-
fake vendor module with stub classes / enums that match the surface the
11-
production code consults. This keeps the whole test suite independent
12-
of the real ``e2b-code-interpreter`` dependency.
8+
Production code does ``import e2b_code_interpreter as e2b`` at module
9+
top-level in :mod:`trpc_agent_sdk.code_executors.cube._sandbox` and
10+
:mod:`trpc_agent_sdk.code_executors.cube._code_executor`. Tests still
11+
need to swap that vendor surface out for a fake — both to avoid talking
12+
to a real Cube server and to inject precise exception types — so the
13+
``fake_e2b`` fixture monkeypatches the ``e2b`` symbol in *both* importer
14+
modules' globals (NOT ``sys.modules['e2b_code_interpreter']``, which
15+
would only affect callers that re-import the package after the patch).
16+
17+
The fake mirrors just the surface the production code actually
18+
consults: a handful of vendor exceptions, the ``SandboxState`` /
19+
``FileType`` enums, and a placeholder ``AsyncSandbox``.
1320
"""
1421

1522
from __future__ import annotations
@@ -44,11 +51,25 @@ def __init__(self, stdout: str = "", stderr: str = "", exit_code: int = 1):
4451
self.exit_code = exit_code
4552

4653

54+
class _FakeTimeoutException(Exception):
55+
"""Mirrors e2b_code_interpreter.TimeoutException.
56+
57+
The real vendor message is long and prescriptive ("passing 'timeout'
58+
when making the request", "Use '0' to disable"). Production code
59+
catches this type in :meth:`CubeSandboxClient.commands_run` and
60+
rewrites it into a structured ``CubeCommandResult(timed_out=True)``,
61+
so what actually gets surfaced to callers never contains this
62+
message. The fake keeps the *type* precise while leaving the message
63+
empty so tests can assert on the translated shape.
64+
"""
65+
66+
4767
def _make_fake_e2b() -> SimpleNamespace:
4868
ns = SimpleNamespace()
4969
ns.SandboxException = _FakeSandboxException
5070
ns.SandboxNotFoundException = _FakeSandboxNotFoundException
5171
ns.CommandExitException = _FakeCommandExitException
72+
ns.TimeoutException = _FakeTimeoutException
5273
ns.SandboxState = SimpleNamespace(
5374
RUNNING=SimpleNamespace(value="running"),
5475
PAUSED=SimpleNamespace(value="paused"),
@@ -61,23 +82,22 @@ def _make_fake_e2b() -> SimpleNamespace:
6182

6283
@pytest.fixture
6384
def fake_e2b(monkeypatch):
64-
"""Patch ``_import_e2b`` everywhere the cube package imports it."""
85+
"""Swap ``e2b_code_interpreter`` for a fake in every cube import site.
86+
87+
Production code does ``import e2b_code_interpreter as e2b`` at the
88+
top of ``_sandbox.py`` and ``_code_executor.py``, which binds an
89+
``e2b`` name in those modules' globals. We patch each of those
90+
bindings independently (rather than ``sys.modules``) so already-
91+
executed ``from … import e2b`` statements see the fake.
92+
"""
6593
ns = _make_fake_e2b()
66-
# The production code does ``from ._e2b import _import_e2b`` in
67-
# _sandbox.py and _code_executor.py, which rebinds the symbol in
68-
# those modules' globals — so we must patch every import site, not
69-
# just the original definition.
70-
monkeypatch.setattr(
71-
"trpc_agent_sdk.code_executors.cube._e2b._import_e2b",
72-
lambda: ns,
73-
)
7494
monkeypatch.setattr(
75-
"trpc_agent_sdk.code_executors.cube._sandbox._import_e2b",
76-
lambda: ns,
95+
"trpc_agent_sdk.code_executors.cube._sandbox.e2b",
96+
ns,
7797
)
7898
monkeypatch.setattr(
79-
"trpc_agent_sdk.code_executors.cube._code_executor._import_e2b",
80-
lambda: ns,
99+
"trpc_agent_sdk.code_executors.cube._code_executor.e2b",
100+
ns,
81101
)
82102
return ns
83103

@@ -92,7 +112,6 @@ def _make_fake_async_sandbox(sandbox_id: str = "sbx-1"):
92112
sbx.sandbox_id = sandbox_id
93113
sbx.kill = AsyncMock(return_value=None)
94114
sbx.set_timeout = AsyncMock(return_value=None)
95-
# get_info returns a state holder by default; tests override.
96115
info = SimpleNamespace(state=SimpleNamespace(value="running"))
97116
sbx.get_info = AsyncMock(return_value=info)
98117
sbx.commands = MagicMock()

tests/code_executors/cube/test_bug_hunt.py

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,3 +556,77 @@ async def test_bug11_copy_remote_issues_rm_before_cp(mock_client):
556556
cp_idx = next(i for i, c in enumerate(cmds) if c.startswith("cp -a"))
557557
assert rm_idx < cp_idx, "rm must precede cp"
558558

559+
560+
561+
562+
563+
@pytest.mark.asyncio
564+
async def test_bug12_commands_run_translates_timeout_to_structured_result(
565+
fake_e2b, fake_async_sandbox,
566+
):
567+
"""TimeoutException at the e2b boundary must become a CubeCommandResult."""
568+
fake_async_sandbox.commands.run = AsyncMock(
569+
side_effect=fake_e2b.TimeoutException()
570+
)
571+
client = CubeSandboxClient(
572+
fake_async_sandbox, idle_timeout=60, execute_timeout=30.0,
573+
)
574+
result = await client.commands_run("sleep 9999", timeout=1.5)
575+
assert isinstance(result, CubeCommandResult)
576+
assert result.timed_out is True, "timed_out flag must be set"
577+
assert result.exit_code == -1, (
578+
f"exit_code on timeout must be -1 (matches local/container "
579+
f"executors); got {result.exit_code}"
580+
)
581+
assert result.stdout == "", f"stdout must be empty on timeout: {result.stdout!r}"
582+
# The rewritten stderr is short and hand-written. Importantly, it does
583+
# NOT contain the e2b vendor boilerplate.
584+
assert "timed out" in result.stderr.lower(), (
585+
f"stderr must describe the timeout: {result.stderr!r}"
586+
)
587+
assert "1.5" in result.stderr, (
588+
f"stderr must mention the configured timeout value: {result.stderr!r}"
589+
)
590+
for leaked in ("passing 'timeout'", "context deadline exceeded", "Use '0'"):
591+
assert leaked not in result.stderr, (
592+
f"vendor message leaked into stderr: {leaked!r} in {result.stderr!r}"
593+
)
594+
595+
596+
@pytest.mark.asyncio
597+
async def test_bug12_execute_code_surfaces_deadline_exceeded_outcome(
598+
fake_e2b, fake_async_sandbox,
599+
):
600+
"""Timeout must appear as OUTCOME_DEADLINE_EXCEEDED, not a raised exception."""
601+
from trpc_agent_sdk.code_executors._types import (
602+
CodeBlock,
603+
CodeExecutionInput,
604+
Outcome,
605+
)
606+
from trpc_agent_sdk.code_executors.cube._code_executor import CubeCodeExecutor
607+
608+
fake_async_sandbox.commands.run = AsyncMock(
609+
side_effect=fake_e2b.TimeoutException()
610+
)
611+
client = CubeSandboxClient(
612+
fake_async_sandbox, idle_timeout=60, execute_timeout=2.0,
613+
)
614+
cfg = CubeCodeExecutorConfig(
615+
template="t", api_url="u", api_key="k",
616+
idle_timeout=60, execute_timeout=2.0,
617+
)
618+
executor = CubeCodeExecutor(client, cfg)
619+
620+
# execute_code MUST return a result, not raise.
621+
result = await executor.execute_code(
622+
invocation_context=None, # type: ignore[arg-type]
623+
code_execution_input=CodeExecutionInput(
624+
code_blocks=[CodeBlock(code="import time; time.sleep(9999)", language="python")],
625+
),
626+
)
627+
assert result.outcome == Outcome.OUTCOME_DEADLINE_EXCEEDED, (
628+
f"expected OUTCOME_DEADLINE_EXCEEDED, got {result.outcome}"
629+
)
630+
assert "timed out" in result.output.lower(), (
631+
f"output must mention the timeout: {result.output!r}"
632+
)

tests/code_executors/cube/test_e2b.py

Lines changed: 0 additions & 54 deletions
This file was deleted.
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
# Tencent is pleased to support the open source community by making tRPC-Agent-Python available.
2+
#
3+
# Copyright (C) 2026 Tencent. All rights reserved.
4+
#
5+
# tRPC-Agent-Python is licensed under Apache-2.0.
6+
"""Unit tests for the cube subpackage import surface.
7+
8+
The Cube/E2B backend is shipped as the optional ``[cube]`` extra and
9+
requires ``e2b-code-interpreter`` at import time. Two contracts are
10+
pinned here:
11+
12+
1. The parent ``trpc_agent_sdk.code_executors`` package intentionally
13+
does NOT re-export Cube symbols. Re-exporting optional-dependency
14+
symbols would silently force every importer of the parent package
15+
to install ``[cube]``; instead, business code that genuinely needs
16+
the Cube backend imports the subpackage directly:
17+
18+
from trpc_agent_sdk.code_executors.cube import CubeCodeExecutor
19+
20+
2. As a corollary, importing the parent package alone does NOT pull in
21+
``e2b_code_interpreter`` — the parent's ``__init__.py`` deliberately
22+
does not reference ``trpc_agent_sdk.code_executors.cube``. (Importing
23+
the cube subpackage itself, by contrast, does eagerly import the
24+
vendor SDK; that is the explicit cost of opting into the [cube]
25+
backend.)
26+
27+
Tests that need a **cold** ``sys.modules`` state are run in a subprocess
28+
so they never corrupt the in-process module cache (which is shared
29+
across the whole test session).
30+
"""
31+
32+
from __future__ import annotations
33+
34+
import subprocess
35+
import sys
36+
import textwrap
37+
38+
import pytest
39+
40+
41+
def _run_isolated(script: str) -> subprocess.CompletedProcess:
42+
return subprocess.run(
43+
[sys.executable, "-c", textwrap.dedent(script)],
44+
capture_output=True,
45+
text=True,
46+
check=False,
47+
)
48+
49+
50+
def test_parent_package_does_not_reexport_cube_symbols():
51+
"""Cube symbols must NOT be reachable from the parent package.
52+
53+
Re-exporting optional-dependency symbols from the eager package
54+
would silently make ``[cube]`` mandatory for everyone who imports
55+
``code_executors``. Force callers to make the dependency explicit
56+
by importing from the subpackage.
57+
"""
58+
from trpc_agent_sdk import code_executors as ce
59+
cube_symbols = (
60+
"CubeCodeExecutor",
61+
"CubeCodeExecutorConfig",
62+
"CubeCommandResult",
63+
"CubeProgramRunner",
64+
"CubeSandboxClient",
65+
"CubeWorkspaceFS",
66+
"CubeWorkspaceManager",
67+
"CubeWorkspaceRuntime",
68+
"CubeWorkspaceRuntimeConfig",
69+
"OnExisting",
70+
"create_cube_workspace_runtime",
71+
)
72+
for name in cube_symbols:
73+
assert name not in ce.__all__, f"{name!r} leaked into parent __all__"
74+
with pytest.raises(AttributeError):
75+
getattr(ce, name)
76+
77+
78+
def test_parent_package_import_does_not_touch_e2b():
79+
"""Plain ``import code_executors`` does NOT import e2b_code_interpreter.
80+
81+
The parent package never references the cube subpackage, so
82+
importing it must stay cheap and dependency-free even when the
83+
[cube] extra is installed in the same environment.
84+
85+
Run in a subprocess so the main test session's module cache cannot
86+
mask the behaviour.
87+
"""
88+
result = _run_isolated("""
89+
import sys
90+
import trpc_agent_sdk.code_executors # noqa: F401
91+
assert "e2b_code_interpreter" not in sys.modules, \
92+
"bare import of code_executors pulled in e2b_code_interpreter"
93+
print("OK")
94+
""")
95+
assert result.returncode == 0, result.stderr
96+
assert "OK" in result.stdout
97+
98+
99+
def test_cube_subpackage_reexports_public_api():
100+
"""Every entry on the cube subpackage's ``__all__`` must resolve."""
101+
from trpc_agent_sdk.code_executors import cube
102+
for name in cube.__all__:
103+
assert hasattr(cube, name), f"{name} missing from cube/__init__.py"

0 commit comments

Comments
 (0)