fix(deadline): remove the dead second SuspectableBackend class shadowing the Phase 3a Protocol

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.
This commit is contained in:
nftpoetrist
2026-08-26 17:12:15 +03:00
committed by kshitij
parent 6cbb7b6115
commit 08b4875f4a
2 changed files with 20 additions and 29 deletions

View File

@@ -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

View File

@@ -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."""