test(mcp): pin CancelledError short-circuits the retry loop

The retry branch is only reachable via `except Exception`, and
`CancelledError` inherits from `BaseException`, so today it naturally
bypasses the retry path and /stop still works.  Add one focused
regression test so any future refactor that widens the retry catch to
`BaseException`, re-orders the handlers, or adds `CancelledError` to
`_TRANSIENT_EXC_NAMES` fails CI instead of silently swallowing /stop.

Made-with: Cursor
This commit is contained in:
Xubin Ren
2026-04-21 13:24:40 +08:00
committed by Xubin Ren
parent 368752e707
commit 82aa9efc02
+24
View File
@@ -161,6 +161,30 @@ async def test_tool_success_on_first_try_no_retry():
assert session.call_tool.call_count == 1
@pytest.mark.asyncio
async def test_tool_does_not_retry_on_cancelled_error():
"""`asyncio.CancelledError` must short-circuit the retry loop.
Regression guard: the retry branch lives under ``except Exception``,
but ``CancelledError`` inherits from ``BaseException``, not
``Exception``, so it naturally bypasses the retry branch today. If a
future refactor ever widens the retry branch to ``BaseException`` (or
re-orders the handlers), ``/stop`` would start retrying instead of
cancelling — this test pins that invariant.
"""
session = AsyncMock()
session.call_tool = AsyncMock(side_effect=asyncio.CancelledError())
wrapper = MCPToolWrapper(session, "test_server", _make_tool_def(), tool_timeout=5)
with patch("nanobot.agent.tools.mcp.asyncio.sleep", new_callable=AsyncMock) as mock_sleep:
output = await wrapper.execute()
assert "cancelled" in output
assert session.call_tool.call_count == 1
mock_sleep.assert_not_called()
@pytest.mark.asyncio
async def test_tool_retry_on_connection_reset():
"""ConnectionResetError (a stdlib exception) should also trigger retry."""