refactor(gateway): drop dead degraded token field; de-churn salvage diff
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.
This commit is contained in:
parent
3a3aed3c1f
commit
2a0d0bc698
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)."""
|
||||
|
|
|
|||
|
|
@ -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())
|
||||
|
|
|
|||
Loading…
Reference in New Issue