diff --git a/src/agent_tools.py b/src/agent_tools.py index 9a54ab8..2785623 100644 --- a/src/agent_tools.py +++ b/src/agent_tools.py @@ -80,6 +80,11 @@ def get_mcp_manager(): # Helpers (kept here — used by sub-modules) # --------------------------------------------------------------------------- def _truncate(text: str, limit: int = MAX_OUTPUT_CHARS) -> str: + # Callers treat the result as text, so always return a string: coerce a + # non-string (None -> "", otherwise str(...)) instead of returning it raw, + # which would just move the crash downstream. + if not isinstance(text, str): + text = "" if text is None else str(text) if len(text) > limit: return text[:limit] + f"\n... (truncated, {len(text)} chars total)" return text diff --git a/tests/test_agent_tools_truncate_nonstring.py b/tests/test_agent_tools_truncate_nonstring.py new file mode 100644 index 0000000..3963217 --- /dev/null +++ b/tests/test_agent_tools_truncate_nonstring.py @@ -0,0 +1,24 @@ +"""Regression: agent_tools._truncate must always return a string. + +It did `len(text)` directly, so `_truncate(None)` raised TypeError. Returning +the raw non-string just moves the crash downstream (callers treat it as text), +so non-strings are now coerced to a string and still truncated. +""" +from src.agent_tools import _truncate + + +def test_non_string_coerced_to_string(): + assert _truncate(None) == "" + assert _truncate(123) == "123" + assert isinstance(_truncate({"a": 1}), str) + + +def test_non_string_is_also_truncated(): + out = _truncate(12345, limit=3) + assert out.startswith("123") and "truncated" in out + + +def test_string_truncation_unchanged(): + assert _truncate("hello", limit=100) == "hello" + out = _truncate("x" * 50, limit=10) + assert out.startswith("x" * 10) and "truncated" in out