Skip to content

Commit 80c424a

Browse files
kingpanther13claude
andcommitted
fix(test): rewrite memory test for Monty, skip unreachable recursion test
The two failing E2Es from 3f508f0 were both written against a Monty sandbox that doesn't expose the primitives I assumed: * ``test_memory_limit_enforced`` used ``bytearray(N)`` — Monty raises ``LookupError: Unable to find 'bytearray' in external functions dict`` because ``bytearray`` isn't an injected helper, so the test hit ``SANDBOX_RUNTIME_ERROR`` instead of the intended ``SANDBOX_LIMIT_EXCEEDED``. Replaced with a string-multiplication allocation (``'x' * (12 * 1024 * 1024)``) which uses only the string-multiply operator — no builtin lookup — and produces a 12 MB allocation that exceeds the 10 MB ``CODE_MODE_MAX_MEMORY`` default. * ``test_recursion_limit_enforced`` used an assigned recursive lambda (``f = lambda n: 1 if n <= 0 else 1 + f(n - 1); f(500)``). Monty doesn't resolve the lambda's binding name from inside its own body (``LookupError: Unable to find 'f' in external functions dict``) so the recursion never actually starts. The test was structurally incapable of triggering the limit. Renamed to ``test_recursion_limit_unreachable_from_user_code`` and replaced with a documenting ``pytest.skip`` that explains why no E2E variant is possible — Monty doesn't allow ``def`` and assigned lambdas can't recurse — and points at the unit-level coverage that locks in the classifier mapping. Also added ``tests/src/unit/test_classify_sandbox_error.py`` (10 tests) covering ``_classify_sandbox_error`` directly: each of the three buckets (LIMIT_EXCEEDED, SYNTAX_UNSUPPORTED, RUNTIME_ERROR) gets its trigger exception types exercised, including the ``RecursionError`` path the E2E can't reach and the ``LookupError: Unable to find 'X' in external functions dict`` that Monty produces for missing builtins (it falls into the default ``SANDBOX_RUNTIME_ERROR`` bucket — the right call because the "use the injected helpers" advice is already in the default suggestions). The other 5 new E2E tests from 3f508f0 (invocation cap, traversal, proxy laundering blocked × 4) all passed in CI; only the two above were broken. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent b3f2089 commit 80c424a

2 files changed

Lines changed: 143 additions & 25 deletions

File tree

tests/src/e2e/tools/test_create_custom_tool.py

Lines changed: 37 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1575,16 +1575,22 @@ class TestCodeModeAdditionalResourceLimits:
15751575
async def test_memory_limit_enforced(self, mcp_client_with_code_mode):
15761576
"""Allocating more than ``CODE_MODE_MAX_MEMORY`` must raise
15771577
``SANDBOX_LIMIT_EXCEEDED``, not silently succeed.
1578+
1579+
Uses string multiplication rather than ``bytearray`` because
1580+
Monty's sandbox doesn't expose ``bytearray`` as a builtin
1581+
(``LookupError: Unable to find 'bytearray' in external functions
1582+
dict``); ``str * int`` is a pure operator with no builtin
1583+
lookup.
15781584
"""
15791585
check = await _check_tool_available(mcp_client_with_code_mode)
15801586
_skip_if_unavailable(check, "Memory limit enforcement")
15811587

1582-
# Default limit is 10 MB; allocating 20 MB must trip it.
1588+
# Default limit is 10 MB; allocating ~12 MB must trip it.
15831589
data = await safe_call_tool(
15841590
mcp_client_with_code_mode,
15851591
TOOL_NAME,
15861592
{
1587-
"code": "x = bytearray(20 * 1024 * 1024)\nlen(x)",
1593+
"code": "x = 'x' * (12 * 1024 * 1024)\nlen(x)",
15881594
"justification": "E2E test: memory limit enforcement",
15891595
},
15901596
)
@@ -1596,31 +1602,37 @@ async def test_memory_limit_enforced(self, mcp_client_with_code_mode):
15961602
)
15971603
logger.info("Memory limit correctly classified")
15981604

1599-
async def test_recursion_limit_enforced(self, mcp_client_with_code_mode):
1600-
"""Recursion deeper than ``CODE_MODE_MAX_RECURSION`` must raise
1601-
``SANDBOX_LIMIT_EXCEEDED``.
1605+
async def test_recursion_limit_unreachable_from_user_code(
1606+
self, mcp_client_with_code_mode
1607+
):
1608+
"""Document that ``CODE_MODE_MAX_RECURSION`` is not directly
1609+
exercisable from sandbox-supplied code.
1610+
1611+
Monty doesn't allow ``def`` (`SANDBOX_SYNTAX_UNSUPPORTED` —
1612+
see ``test_no_class_definitions``-adjacent coverage), and an
1613+
assigned lambda can't reference its own binding name from
1614+
inside its own body (``LookupError: Unable to find 'f' in
1615+
external functions dict``). User code therefore has no way to
1616+
construct a recursive call deep enough to trip the
1617+
``ResourceLimits.max_recursion_depth`` cap.
1618+
1619+
The setting still flows through to the sandbox runtime — see
1620+
``_run_sandboxed_code``, where ``ResourceLimits(...,
1621+
max_recursion_depth=settings.code_mode_max_recursion)`` is
1622+
constructed — and the classifier maps a ``RecursionError`` to
1623+
``SANDBOX_LIMIT_EXCEEDED`` (covered by the unit-level
1624+
``test_classify_recursion_error`` in
1625+
``tests/src/unit/test_saved_tools_persistence.py``-adjacent
1626+
suite). This test is a deliberate skip so a future maintainer
1627+
sees the gap and the rationale rather than rediscovering the
1628+
Monty constraint from scratch.
16021629
"""
1603-
check = await _check_tool_available(mcp_client_with_code_mode)
1604-
_skip_if_unavailable(check, "Recursion limit enforcement")
1605-
1606-
# Direct recursion is hard in Monty (no def). Use a recursive
1607-
# lambda via assignment — Monty supports lambda binding.
1608-
code = (
1609-
"f = lambda n: 1 if n <= 0 else 1 + f(n - 1)\n"
1610-
"f(500)"
1611-
)
1612-
data = await safe_call_tool(
1613-
mcp_client_with_code_mode,
1614-
TOOL_NAME,
1615-
{"code": code, "justification": "E2E test: recursion limit"},
1616-
)
1617-
assert data.get("success") is False, f"Should fail: {data}"
1618-
err = data.get("error", {})
1619-
err_code = err.get("code") if isinstance(err, dict) else ""
1620-
assert err_code == "SANDBOX_LIMIT_EXCEEDED", (
1621-
f"Expected SANDBOX_LIMIT_EXCEEDED, got {err_code}: {data}"
1630+
pytest.skip(
1631+
"Monty doesn't support recursive user-defined functions; "
1632+
"the recursion limit fires only on Monty's internal AST "
1633+
"evaluation depth, which sandbox-supplied code can't reach. "
1634+
"Classifier mapping is unit-tested directly."
16221635
)
1623-
logger.info("Recursion limit correctly classified")
16241636

16251637
async def test_invocation_cap_enforced(self, mcp_client_with_code_mode):
16261638
"""Looping past ``code_mode_max_invocations`` must trip the cap
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
"""Unit tests for ``_classify_sandbox_error`` in ``ha_mcp.tools.tools_code``.
2+
3+
The classifier maps Monty / sandbox runtime exceptions to one of three
4+
buckets — ``SANDBOX_LIMIT_EXCEEDED`` / ``SANDBOX_SYNTAX_UNSUPPORTED`` /
5+
``SANDBOX_RUNTIME_ERROR`` — with category-tailored suggestions. Some
6+
mappings (notably ``RecursionError``) aren't directly exercisable from
7+
sandbox-supplied code because Monty doesn't allow user-defined recursive
8+
functions; this suite covers them at the helper level so a regression
9+
in the type-name match doesn't slip through CI.
10+
"""
11+
12+
from ha_mcp.errors import ErrorCode
13+
from ha_mcp.tools.tools_code import _classify_sandbox_error
14+
15+
16+
class TestLimitExceededBucket:
17+
def test_memory_error(self):
18+
code, message, suggestions = _classify_sandbox_error(
19+
MemoryError("10485779 bytes > 10485760")
20+
)
21+
assert code == ErrorCode.SANDBOX_LIMIT_EXCEEDED
22+
assert "memory" in message.lower()
23+
joined = " ".join(suggestions).lower()
24+
assert "memory" in joined
25+
assert "code_mode_max_memory" in joined.lower()
26+
27+
def test_recursion_error(self):
28+
code, message, suggestions = _classify_sandbox_error(
29+
RecursionError("maximum recursion depth exceeded")
30+
)
31+
assert code == ErrorCode.SANDBOX_LIMIT_EXCEEDED
32+
assert "recursion" in message.lower()
33+
joined = " ".join(suggestions).lower()
34+
assert "recursion" in joined
35+
assert "code_mode_max_recursion" in joined.lower()
36+
37+
def test_timeout_error(self):
38+
code, message, suggestions = _classify_sandbox_error(
39+
TimeoutError("operation timed out after 30s")
40+
)
41+
assert code == ErrorCode.SANDBOX_LIMIT_EXCEEDED
42+
assert "time" in message.lower() or "wall" in message.lower()
43+
joined = " ".join(suggestions).lower()
44+
assert "code_mode_max_duration" in joined.lower()
45+
46+
47+
class TestSyntaxUnsupportedBucket:
48+
def test_module_not_found_error(self):
49+
code, message, suggestions = _classify_sandbox_error(
50+
ModuleNotFoundError("No module named 'time'")
51+
)
52+
assert code == ErrorCode.SANDBOX_SYNTAX_UNSUPPORTED
53+
assert "import" in message.lower()
54+
joined = " ".join(suggestions).lower()
55+
assert "import" in joined
56+
assert "api_get" in joined or "helper" in joined
57+
58+
def test_not_implemented_error(self):
59+
code, message, suggestions = _classify_sandbox_error(
60+
NotImplementedError(
61+
"Monty does not yet support context managers (with statements)"
62+
)
63+
)
64+
assert code == ErrorCode.SANDBOX_SYNTAX_UNSUPPORTED
65+
66+
def test_syntax_error(self):
67+
code, _message, _suggestions = _classify_sandbox_error(
68+
SyntaxError("invalid syntax")
69+
)
70+
assert code == ErrorCode.SANDBOX_SYNTAX_UNSUPPORTED
71+
72+
73+
class TestRuntimeErrorBucket:
74+
def test_type_error(self):
75+
code, _message, suggestions = _classify_sandbox_error(
76+
TypeError("'list' object is not an iterator")
77+
)
78+
assert code == ErrorCode.SANDBOX_RUNTIME_ERROR
79+
# Default-bucket suggestions name the actual exception type.
80+
joined = " ".join(suggestions)
81+
assert "TypeError" in joined
82+
83+
def test_attribute_error(self):
84+
code, _message, _suggestions = _classify_sandbox_error(
85+
AttributeError("'tuple' object has no attribute '__class__'")
86+
)
87+
assert code == ErrorCode.SANDBOX_RUNTIME_ERROR
88+
89+
def test_value_error(self):
90+
code, _message, _suggestions = _classify_sandbox_error(
91+
ValueError("something went wrong")
92+
)
93+
assert code == ErrorCode.SANDBOX_RUNTIME_ERROR
94+
95+
def test_lookup_error_for_missing_builtin(self):
96+
"""``LookupError: Unable to find 'X' in external functions dict``
97+
is what Monty raises when sandbox code references a name that's
98+
not in the injected helpers (e.g. ``bytearray``). Classifier
99+
falls into the default bucket — that's the right call because
100+
the user-actionable advice is "use the injected helpers" which
101+
is already in the default suggestions.
102+
"""
103+
code, _message, _suggestions = _classify_sandbox_error(
104+
LookupError("Unable to find 'bytearray' in external functions dict")
105+
)
106+
assert code == ErrorCode.SANDBOX_RUNTIME_ERROR

0 commit comments

Comments
 (0)