From 82aa9efc028b268aa774dfe43e3a16567ca92672 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Tue, 21 Apr 2026 05:21:39 +0000 Subject: [PATCH] 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 --- tests/agent/test_mcp_transient_retry.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/agent/test_mcp_transient_retry.py b/tests/agent/test_mcp_transient_retry.py index 823bb1e8..c7ef0c4a 100644 --- a/tests/agent/test_mcp_transient_retry.py +++ b/tests/agent/test_mcp_transient_retry.py @@ -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."""