From 08b4875f4a8af7a2162666fc0de0043b2db7ff5d Mon Sep 17 00:00:00 2001 From: nftpoetrist <264138787+nftpoetrist@users.noreply.github.com> Date: Wed, 26 Aug 2026 17:12:15 +0300 Subject: [PATCH] fix(deadline): remove the dead second SuspectableBackend class shadowing the Phase 3a Protocol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit agent/deadline.py defined SuspectableBackend twice: the Phase 3a Protocol (sync ensure_healthy(self) -> bool) and, further down the same module, an unrelated concrete class with the same name (async ensure_healthy(self, timeout=5.0)) added later by the MCP Phase 3b adopter. Since Python executes class statements top-to-bottom, the second definition silently shadowed the first at module scope. Nothing in the tree imports or subclasses either by name today — the MCP adopter duck-types the same-shaped contract directly on its own connection class rather than referencing agent.deadline.SuspectableBackend — so this caused no live behavior change. But it left the wrong (and differently shaped) class resolvable under that name for the next Phase 3b adopter that does import it for a type hint. --- agent/deadline.py | 29 ----------------------------- tests/agent/test_deadline.py | 20 ++++++++++++++++++++ 2 files changed, 20 insertions(+), 29 deletions(-) diff --git a/agent/deadline.py b/agent/deadline.py index 83d7f3cf2c..a2c5cbfe05 100644 --- a/agent/deadline.py +++ b/agent/deadline.py @@ -648,32 +648,3 @@ def kill_process_tree(pid: int, *, sig: Optional[int] = None) -> bool: except Exception: continue return signalled - - -class SuspectableBackend: - """Protocol for backends whose connection state can be *poisoned* by a - race (teardown-vs-keepalive, auth-lock corruption) without the backend - itself being dead. - - The contract is **cheap-mark, lazy-verify**: noticing a poisoned state - must never do I/O — ``mark_suspect`` just latches a reason string. The - NEXT caller pays for verification once, via ``ensure_healthy``: a cheap - health probe that either clears the suspicion (backend was fine) or - forces a reconnect/recycle before the call proceeds. This is what keeps - a single race from permanently parking a connection (#81051/#77765/ - #84132): instead of parking on the ambiguous event, the backend is - marked suspect and recycled exactly once on next use. - """ - - def mark_suspect(self, reason: str) -> None: - """Latch a suspicion about this backend. Must be cheap (no I/O).""" - raise NotImplementedError - - async def ensure_healthy(self, timeout: float = 5.0) -> bool: - """Verify a suspect backend before reuse. - - Returns True when the backend is healthy (clearing the suspicion); - returns False after forcing a reconnect/recycle so the caller's - normal no-session path handles the rebuild. Must not raise. - """ - raise NotImplementedError diff --git a/tests/agent/test_deadline.py b/tests/agent/test_deadline.py index e66ac19b2e..c87252e506 100644 --- a/tests/agent/test_deadline.py +++ b/tests/agent/test_deadline.py @@ -545,6 +545,26 @@ class TestSequentialToolTimeoutResolver: # --------------------------------------------------------------------------- +def test_suspectable_backend_name_is_not_shadowed(): + """`agent.deadline.SuspectableBackend` must resolve to the Phase 3a + Protocol (sync `ensure_healthy(self) -> bool`), not a second, unrelated + `class SuspectableBackend` defined later in the module. A later Phase 3b + adopter independently redefined the same name as a concrete async class + (`ensure_healthy(self, timeout=5.0)`), which silently shadowed the + Protocol at module scope — nothing subclasses or imports either by name + today, so the collision caused no runtime breakage yet, but it would + hand the wrong (and differently-shaped) type to the next `from + agent.deadline import SuspectableBackend` consumer. + """ + import inspect + + from agent.deadline import SuspectableBackend + + assert getattr(SuspectableBackend, "_is_protocol", False) is True + assert not inspect.iscoroutinefunction(SuspectableBackend.ensure_healthy) + assert "timeout" not in inspect.signature(SuspectableBackend.ensure_healthy).parameters + + class _RecordingBackend: """Minimal SuspectableBackend: records mark_suspect calls."""