From 2a0d0bc698a54ce34b275a6dda224e5d26e9eff5 Mon Sep 17 00:00:00 2001 From: kshitij <82637225+kshitijk4poor@users.noreply.github.com> Date: Fri, 7 Aug 2026 18:17:37 +0530 Subject: [PATCH] refactor(gateway): drop dead degraded token field; de-churn salvage diff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the #80376 salvage: - TurnLeaseToken.degraded is dead code since acquire() started raising TurnLeaseTimeoutError: the only constructor site passes the default and repo-wide there are zero external readers. Remove the field, its ctor param, the repr segment, and the always-False guards in rebind()/release(); the class docstring's 'retained for compatibility' claim described consumers that do not exist. - Revert pure black-rewrap churn in test_config_env_bridge_authority.py and test_turn_lease.py (hunks on functions this change does not touch), keeping the functional Windows-env/encoding additions. - Turn the default-value test into an invariant: config default must equal gateway.turn_lease.DEFAULT_LEASE_WAIT instead of pinning the 1800 literal. - Rewrap the dangling 'Released' comment line in gateway/run.py. Verified: 20/20 targeted gateway tests, ruff clean; mutation check — with gateway/turn_lease.py reverted to pre-fix main the module fails, restored it passes. --- gateway/run.py | 7 +-- gateway/turn_lease.py | 22 +++----- .../test_config_env_bridge_authority.py | 56 ++++++++----------- tests/gateway/test_turn_lease.py | 8 +-- 4 files changed, 39 insertions(+), 54 deletions(-) diff --git a/gateway/run.py b/gateway/run.py index 9064ed001f9c2..5cced736f0c5c 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -16663,10 +16663,9 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew # Fail-closed on timeout: never enter the transcript region without a # lease. Outer dispatch returns a bounded rejection/resend notice rather # than recreating the exact concurrent-turn corruption this lease exists - # to prevent. Released - # in _handle_message's finally via _release_turn_lease — granted per - # (routing key, run generation) so a stale unwind can't release a - # newer turn's lease. + # to prevent. Released in _handle_message's finally via + # _release_turn_lease — granted per (routing key, run generation) so a + # stale unwind can't release a newer turn's lease. _lease_registry = getattr(self, "_turn_leases", None) if _lease_registry is not None: try: diff --git a/gateway/turn_lease.py b/gateway/turn_lease.py index 012b7c93ebffb..a47ad65a43dee 100644 --- a/gateway/turn_lease.py +++ b/gateway/turn_lease.py @@ -97,32 +97,29 @@ class TurnLeaseTimeoutError(TimeoutError): class TurnLeaseToken: """Handle returned by :meth:`SessionTurnLeaseRegistry.acquire`. - ``degraded`` is retained for compatibility with older callers and test - doubles, but :meth:`acquire` no longer returns degraded tokens: a timeout - raises :class:`TurnLeaseTimeoutError` instead. ``released`` makes release - idempotent. + A timeout raises :class:`TurnLeaseTimeoutError` instead of returning a + token, so every token handed out is a held lease. ``released`` makes + release idempotent. """ - __slots__ = ("session_id", "owner_key", "generation", "degraded", "released") + __slots__ = ("session_id", "owner_key", "generation", "released") def __init__( self, session_id: str, owner_key: str, generation: int, - degraded: bool = False, ) -> None: self.session_id = session_id self.owner_key = owner_key self.generation = generation - self.degraded = degraded self.released = False def __repr__(self) -> str: # pragma: no cover - debug aid return ( f"TurnLeaseToken(session_id={self.session_id!r}, " f"owner_key={self.owner_key!r}, generation={self.generation}, " - f"degraded={self.degraded}, released={self.released})" + f"released={self.released})" ) @@ -291,7 +288,6 @@ class SessionTurnLeaseRegistry: """ if ( token is None - or token.degraded or token.released or not new_session_id or new_session_id == token.session_id @@ -329,11 +325,11 @@ class SessionTurnLeaseRegistry: """Release ``token``'s lease. Idempotent; ownership-checked. Returns True only when this exact token was the current holder and - the lock was freed. A degraded token, a re-release, or a stale token - whose slot has since been granted to a newer turn are all safe - no-ops — a stale unwind can never release a newer turn's lease. + the lock was freed. A re-release or a stale token whose slot has + since been granted to a newer turn are both safe no-ops — a stale + unwind can never release a newer turn's lease. """ - if token is None or token.degraded or token.released: + if token is None or token.released: return False token.released = True lease = self._leases.get(token.session_id) diff --git a/tests/gateway/test_config_env_bridge_authority.py b/tests/gateway/test_config_env_bridge_authority.py index 20a30246410c4..23824ba69e62b 100644 --- a/tests/gateway/test_config_env_bridge_authority.py +++ b/tests/gateway/test_config_env_bridge_authority.py @@ -21,9 +21,7 @@ import pytest PROJECT_ROOT = Path(__file__).resolve().parents[2] -def _run_gateway_import( - hermes_home: Path, initial_env: dict[str, str] -) -> dict[str, str]: +def _run_gateway_import(hermes_home: Path, initial_env: dict[str, str]) -> dict[str, str]: """Import gateway.run in a clean subprocess and return the post-import env. The bridge runs at module-import time, so simply importing is enough @@ -100,15 +98,9 @@ def _run_gateway_import( return out -def _write_config( - home: Path, - agent_cfg: dict | None = None, - display_cfg: dict | None = None, - timezone: str | None = None, - gateway_cfg: dict | None = None, -) -> None: +def _write_config(home: Path, agent_cfg: dict | None = None, display_cfg: dict | None = None, + timezone: str | None = None, gateway_cfg: dict | None = None) -> None: import yaml - cfg: dict = {} if agent_cfg: cfg["agent"] = agent_cfg @@ -135,22 +127,16 @@ def hermes_home(tmp_path: Path) -> Path: def test_config_gateway_timeout_wins_over_stale_env(hermes_home: Path) -> None: """Every agent.* bridge key must be config-authoritative, not .env-authoritative.""" - _write_config( - hermes_home, - agent_cfg={ - "gateway_timeout": 1800, - "gateway_timeout_warning": 900, - "session_stall_timeout": 300, - }, - ) - _write_env( - hermes_home, - { - "HERMES_AGENT_TIMEOUT": "60", - "HERMES_AGENT_TIMEOUT_WARNING": "30", - "HERMES_SESSION_STALL_TIMEOUT": "15", - }, - ) + _write_config(hermes_home, agent_cfg={ + "gateway_timeout": 1800, + "gateway_timeout_warning": 900, + "session_stall_timeout": 300, + }) + _write_env(hermes_home, { + "HERMES_AGENT_TIMEOUT": "60", + "HERMES_AGENT_TIMEOUT_WARNING": "30", + "HERMES_SESSION_STALL_TIMEOUT": "15", + }) env = _run_gateway_import(hermes_home, initial_env={}) @@ -190,15 +176,21 @@ def test_default_turn_lease_timeout_overrides_stale_env_when_key_is_omitted( def test_default_turn_lease_timeout_matches_the_runtime_fallback() -> None: - """The advertised config default must match the fail-closed runtime.""" + """The advertised config default must match the fail-closed runtime fallback. + + Invariant, not a snapshot: whatever the default becomes, config and the + lease registry's DEFAULT_LEASE_WAIT must move together. + """ + from gateway.turn_lease import DEFAULT_LEASE_WAIT from hermes_cli.config import DEFAULT_CONFIG - assert DEFAULT_CONFIG["agent"]["gateway_turn_lease_timeout"] == 1800 + assert ( + float(DEFAULT_CONFIG["agent"]["gateway_turn_lease_timeout"]) + == DEFAULT_LEASE_WAIT + ) -def test_config_platform_connect_timeout_supplies_env_when_unset( - hermes_home: Path, -) -> None: +def test_config_platform_connect_timeout_supplies_env_when_unset(hermes_home: Path) -> None: """config.yaml:gateway.platform_connect_timeout supplies the env var when it isn't already set (#19776 — config surface for the Discord connect timeout, replacing the undocumented env-var-only workaround).""" diff --git a/tests/gateway/test_turn_lease.py b/tests/gateway/test_turn_lease.py index bd03d807e0817..552fc98b2d2a4 100644 --- a/tests/gateway/test_turn_lease.py +++ b/tests/gateway/test_turn_lease.py @@ -48,7 +48,7 @@ def test_alias_key_turn_waits_and_order_is_preserved(): token = await registry.acquire( "sess-1", owner_key=owner_key, generation=generation, timeout=5 ) - assert token is not None and not token.degraded + assert token is not None events.append(f"load:{owner_key}") await asyncio.sleep(hold) # simulate run + flush events.append(f"flush:{owner_key}") @@ -394,9 +394,7 @@ def test_rebind_moves_serialization_to_new_session_id(): async def scenario(): registry = SessionTurnLeaseRegistry() - token = await registry.acquire( - "parent", owner_key="key-a", generation=1, timeout=5 - ) + token = await registry.acquire("parent", owner_key="key-a", generation=1, timeout=5) assert registry.rebind(token, "child") is True assert token is not None and token.session_id == "child" @@ -409,7 +407,7 @@ def test_rebind_moves_serialization_to_new_session_id(): assert registry.release(token) is True t2 = await waiter - assert t2 is not None and not t2.degraded + assert t2 is not None registry.release(t2) _run(scenario())