|
mcp: every wait on the MCP path gets a bound, and a failed reload keeps what it had
reload_tools kept dying on prod, and only a restart cured it. Nothing on the MCP path was bounded: ClientSession was built without read_timeout_seconds, so the SDK waited forever (anyio.fail_after(None)); _send awaited a future with no timeout; disconnect() awaited the shielded runner with no timeout; and disconnect_all() walks the pool one client at a time, so a single wedged client blocked every reload that came after it. A cancelled caller only cancelled itself — the runner stayed wedged, which is why it was every reload, not one. Bounds, in two deliberate layers. The SDK read timeout ends a request nobody answers *inside* the runner, so the client stays usable; the caller-side bound is the backstop for an op queued behind a stuck one, and it aborts the runner rather than leaving it for the next command to queue behind. connect 60s, list_tools 45s, call_tool 300s (SDK 240s), session read 30s, disconnect 15s, each overridable, each logging the culprit. disconnect() and disconnect_all() no longer swallow CancelledError — that is exactly how the reload budget stopped being enforceable. The registry is now rebuilt in one step: collect every server's tools first, then commit the whole set in a single synchronous swap. clear_external_tools() ran *before* the rebuild, so a rebuild that failed halfway left the model holding MCP tool names that resolved to nothing. A failed pass now leaves the previous registrations untouched, and the reload phase carries its own budget so the tool always returns a report. Two more defects fell out of the audit. The streamable_http transport was handed a bare httpx.AsyncClient, whose 5s default read killed the GET stream of an idle server — six of the nine prod servers are streamable_http. And register_mcp_tools called log.warning() in a module where log was never defined, a NameError on precisely the failure path it was meant to report. |
|---|
|
|
| navi/api/deps.py |
|---|
| navi/api/routes/admin.py |
|---|
| navi/core/registry.py |
|---|
| navi/core/reload.py |
|---|
| navi/mcp/client.py |
|---|
| navi/mcp/manager.py |
|---|
| navi/mcp/tools.py |
|---|
| tests/integration/test_mcp_integration.py |
|---|
| tests/unit/core/test_registry.py |
|---|
| tests/unit/core/test_reload.py |
|---|
| tests/unit/mcp/test_client_bounds.py 0 → 100644 |
|---|