From 9c3c6833779522c8b7abd161f44a065471b91932 Mon Sep 17 00:00:00 2001 From: Amir Fathi Date: Wed, 5 Aug 2026 00:17:59 +0000 Subject: [PATCH 1/3] fix(mcp): track ping error responses as connection failures, not successes Fixes #607 --- src/fast_agent/mcp/transport_tracking.py | 25 ++++++++++- .../fast_agent/mcp/test_transport_tracking.py | 45 +++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/src/fast_agent/mcp/transport_tracking.py b/src/fast_agent/mcp/transport_tracking.py index c5bc04c83..aaf51a5bb 100644 --- a/src/fast_agent/mcp/transport_tracking.py +++ b/src/fast_agent/mcp/transport_tracking.py @@ -335,6 +335,11 @@ def _handle_post_event(self, event: ChannelEvent, now: datetime) -> None: self._post_last_at = now self._record_response_channel(event) + if classification is ActivityState.ERROR: + detail = self._ping_failure_detail(event.message) + if detail is not None: + self._post_last_error = detail + mode_stats.last_error = detail if classification is not ActivityState.PING: self._record_history(event.channel, classification, now) elif event.event_type == "error": @@ -372,7 +377,15 @@ def _handle_get_event(self, event: ChannelEvent, now: datetime) -> None: summary = _summarise_classified_message(classification, event.message) self._get_last_summary = summary self._get_last_at = now - self._get_last_event = "ping" if classification is ActivityState.PING else "message" + if classification is ActivityState.PING: + self._get_last_event = "ping" + elif classification is ActivityState.ERROR: + self._get_last_event = "error" + detail = self._ping_failure_detail(event.message) + if detail is not None: + self._get_last_error = detail + else: + self._get_last_event = "message" self._get_last_event_at = now self._record_response_channel(event) @@ -547,9 +560,19 @@ def _classify_ping_exchange( return classification if classification is ActivityState.RESPONSE and request_id in self._ping_request_ids: self._ping_request_ids.discard(request_id) + if isinstance(root, JSONRPCError): + return ActivityState.ERROR return ActivityState.PING return classification + @staticmethod + def _ping_failure_detail(message: JSONRPCMessage) -> str | None: + if isinstance(message, JSONRPCError): + code = message.error.code + text = message.error.message or "ping failed" + return f"{text} ({code})" if code is not None else text + return None + def _tally_classification( self, channel_key: str, diff --git a/tests/unit/fast_agent/mcp/test_transport_tracking.py b/tests/unit/fast_agent/mcp/test_transport_tracking.py index fd58290b0..1a5552470 100644 --- a/tests/unit/fast_agent/mcp/test_transport_tracking.py +++ b/tests/unit/fast_agent/mcp/test_transport_tracking.py @@ -288,6 +288,51 @@ def test_response_channel_records_error_response() -> None: assert metrics.consume_response_channel(7) == "get" +def test_ping_error_response_is_recorded_as_a_connection_failure() -> None: + metrics = TransportChannelMetrics() + metrics.register_ping_request(1) + + metrics.record_event( + ChannelEvent( + channel="get", + event_type="message", + message=JSONRPCError( + jsonrpc="2.0", + id=1, + error=ErrorData(code=-32603, message="ping timeout"), + ), + ) + ) + + snapshot = metrics.snapshot() + assert snapshot.get is not None + assert snapshot.get.state == "error" + assert snapshot.get.last_error == "ping timeout (-32603)" + assert snapshot.get.last_event == "error" + + +def test_ping_error_response_on_post_channel_is_recorded_as_a_connection_failure() -> None: + metrics = TransportChannelMetrics() + metrics.register_ping_request(1) + + metrics.record_event( + ChannelEvent( + channel="post-json", + event_type="message", + message=JSONRPCError( + jsonrpc="2.0", + id=1, + error=ErrorData(code=-32603, message="ping timeout"), + ), + ) + ) + + snapshot = metrics.snapshot() + assert snapshot.post is not None + assert snapshot.post.state == "error" + assert snapshot.post.last_error == "ping timeout (-32603)" + + @pytest.mark.parametrize( "method", ["ping", "PING", " notifications/PING ", "mcp.ping"], From 2becdb888cf25eecc28a3f2af139f8cac637b8b8 Mon Sep 17 00:00:00 2001 From: Amir Fathi Date: Wed, 5 Aug 2026 12:14:03 +0000 Subject: [PATCH 2/3] fix(mcp): surface ping errors on a connected GET channel and clear them on recovery _get_state() checked _get_connected before _get_last_error, so a ping failure recorded on a live GET/SSE channel never changed the reported state away from "open" and the UI (gated on state == "error") never rendered it. Neither _get_last_error nor _post_last_error was ever reset on a successful ping, so once one failed the channel stayed red permanently, without a decreasing signal to answer. Check last_error first in _get_state() regardless of connected, and clear the stored error on the next successful ping response on both channels. --- src/fast_agent/mcp/transport_tracking.py | 11 +++- .../fast_agent/mcp/test_transport_tracking.py | 56 +++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/src/fast_agent/mcp/transport_tracking.py b/src/fast_agent/mcp/transport_tracking.py index aaf51a5bb..1e5429090 100644 --- a/src/fast_agent/mcp/transport_tracking.py +++ b/src/fast_agent/mcp/transport_tracking.py @@ -340,6 +340,11 @@ def _handle_post_event(self, event: ChannelEvent, now: datetime) -> None: if detail is not None: self._post_last_error = detail mode_stats.last_error = detail + elif classification is ActivityState.PING and isinstance( + event.message, JSONRPCResponse + ): + self._post_last_error = None + mode_stats.last_error = None if classification is not ActivityState.PING: self._record_history(event.channel, classification, now) elif event.event_type == "error": @@ -379,6 +384,8 @@ def _handle_get_event(self, event: ChannelEvent, now: datetime) -> None: self._get_last_at = now if classification is ActivityState.PING: self._get_last_event = "ping" + if isinstance(event.message, JSONRPCResponse): + self._get_last_error = None elif classification is ActivityState.ERROR: self._get_last_event = "error" detail = self._ping_failure_detail(event.message) @@ -872,10 +879,10 @@ def _has_get_snapshot_activity(self) -> bool: ) def _get_state(self) -> str: - if self._get_connected: - return "open" if self._get_last_error is not None: return "disabled" if self._get_last_status_code == 405 else "error" + if self._get_connected: + return "open" if self._get_had_connection: return "off" return "idle" diff --git a/tests/unit/fast_agent/mcp/test_transport_tracking.py b/tests/unit/fast_agent/mcp/test_transport_tracking.py index 1a5552470..0136f9325 100644 --- a/tests/unit/fast_agent/mcp/test_transport_tracking.py +++ b/tests/unit/fast_agent/mcp/test_transport_tracking.py @@ -333,6 +333,62 @@ def test_ping_error_response_on_post_channel_is_recorded_as_a_connection_failure assert snapshot.post.last_error == "ping timeout (-32603)" +def test_get_ping_error_is_visible_while_the_channel_stays_connected() -> None: + metrics = TransportChannelMetrics() + metrics.record_event(ChannelEvent(channel="get", event_type="connect")) + metrics.register_ping_request(1) + + metrics.record_event( + ChannelEvent( + channel="get", + event_type="message", + message=JSONRPCError( + jsonrpc="2.0", + id=1, + error=ErrorData(code=-32603, message="ping timeout"), + ), + ) + ) + + snapshot = metrics.snapshot() + assert snapshot.get is not None + assert snapshot.get.connected is True + assert snapshot.get.state == "error" + assert snapshot.get.last_error == "ping timeout (-32603)" + + +def test_ping_error_clears_on_the_next_successful_ping() -> None: + metrics = TransportChannelMetrics() + metrics.record_event(ChannelEvent(channel="get", event_type="connect")) + metrics.register_ping_request(1) + metrics.record_event( + ChannelEvent( + channel="get", + event_type="message", + message=JSONRPCError( + jsonrpc="2.0", + id=1, + error=ErrorData(code=-32603, message="ping timeout"), + ), + ) + ) + assert metrics.snapshot().get.state == "error" + + metrics.register_ping_request(2) + metrics.record_event( + ChannelEvent( + channel="get", + event_type="message", + message=JSONRPCResponse(jsonrpc="2.0", id=2, result={}), + ) + ) + + snapshot = metrics.snapshot() + assert snapshot.get is not None + assert snapshot.get.state == "open" + assert snapshot.get.last_error is None + + @pytest.mark.parametrize( "method", ["ping", "PING", " notifications/PING ", "mcp.ping"], From 0b613705eeda17f8691b28e2631347dd8432910e Mon Sep 17 00:00:00 2001 From: Amir Fathi Date: Wed, 5 Aug 2026 12:16:58 +0000 Subject: [PATCH 3/3] fix(mcp): narrow snapshot.get before reading .state in the new regression test ty flagged the direct chained access as unresolved-attribute on the ChannelSnapshot | None union; assert not-None first, same pattern already used by the neighboring tests in this file. --- tests/unit/fast_agent/mcp/test_transport_tracking.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/fast_agent/mcp/test_transport_tracking.py b/tests/unit/fast_agent/mcp/test_transport_tracking.py index 0136f9325..372e41df8 100644 --- a/tests/unit/fast_agent/mcp/test_transport_tracking.py +++ b/tests/unit/fast_agent/mcp/test_transport_tracking.py @@ -372,7 +372,9 @@ def test_ping_error_clears_on_the_next_successful_ping() -> None: ), ) ) - assert metrics.snapshot().get.state == "error" + first_snapshot = metrics.snapshot() + assert first_snapshot.get is not None + assert first_snapshot.get.state == "error" metrics.register_ping_request(2) metrics.record_event(