mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-23 16:36:23 +00:00
* fix(gateway): serialize concurrent turns per resolved session_id with a turn lease Closes the serialization half of #64934. The busy guards are keyed by routing key, but the durable transcript is owned by session_id — and switch_session() makes the key→id mapping many-to-one (/resume from a second chat/topic, CLI-continuity rebinding, async-delegation pinning, topic-binding tip-walks). Two routing keys mapped to one session_id ran concurrent turns on two different agent objects, invisible to every per-key guard: flushes persisted in completion order, the identity-marker dedup swallowed rows, and the second turn ran on a stale history base — leaving a permanent user;user alternation wedge. The fix: an asyncio lease keyed by RESOLVED session_id (gateway/turn_lease.py), acquired in _handle_message_with_agent after session resolution is final (post switch_session/tip-walk), immediately before the transcript load, and released in _handle_message's finally on every exit path. Tokens are granted per (routing key, run generation) so a stale unwind can never release a newer turn's lease (#28686 ownership lesson). Same-key messages never reach the acquisition point mid-turn (both routing-key guards hold them), so the lock is uncontended outside the alias-key route — where the second turn now waits for the first turn's flush and logs one WARNING naming the session and both routing keys (pairs with the #67371 tripwire). Fail-open: a stuck holder degrades to today's unserialized behavior with a loud ERROR after agent.gateway_timeout — never a wedged session; a degraded token holds nothing and can't steal the lease. Registry is size-capped and never evicts a live lease. Persist-disabled review forks never dispatch through _handle_message, so they cannot contend. Known limits (tracked on #64934): CLI-continuity cross-process pairs need a DB-level lease; mid-turn compression rotation leaves a small alias window for a follow-up at the binding-sync sites. Validation: 8 behavior tests (alias-key wait + flush order, no cross-session contention, generation-scoped idempotent release, timeout fail-open without lease theft, bounded registry, bare-runner-safe release wiring) + E2E against a real SessionStore reproducing the issue's switch_session alias route — strict alternation and arrival order preserved. * refactor(gateway): conversation-scope funnel + mid-turn lease rebind Completes the #64934 system beyond the point fix. Two structural changes, both eliminating whole bug classes rather than instances: 1. _clear_conversation_scope — THE single conversation-boundary funnel. /new, /resume, auto-reset, expiry finalization, and the compression-exhausted reset each carried a hand-copied pop-list of the per-session dicts, and the lists drifted every time a new dict was added (#48031, #58403, #10702, #35809 were all 'boundary X forgot dict Y' bugs). All five sites now make one funnel call driven by the _CONVERSATION_SCOPED_STATE registry; adding a new conversation-scoped dict means adding one name to the registry, and every boundary picks it up automatically. Scope rules documented at the registry: turn-scoped state, the monotonic generation counter, and the agent cache are deliberately excluded (different lifecycles). 2. SessionTurnLeaseRegistry.rebind — the held turn lease now FOLLOWS mid-turn compression rotation. Both rotation sites (session-hygiene pre-compression, agent-result session_id swap) alias the same _SessionLease object under the new id, so an alias routing key resolving the fresh child (topic tip-walk) still serializes against the in-flight turn. Closes the rotation-alias window flagged as a known limit on #64934. Ownership-checked like release; when the target id already has a live lease the rebind fails open with a loud WARNING (never a mid-turn deadlock). Tests: 3 new rebind behavior tests + 5 funnel behavior tests (including a real-setter drift guard); the two AST change-detector pins in test_10710/test_48031 were re-pointed at the funnel and the #58403 pin converted to a behavioral test. E2E: rotation-alias scenario against a real SessionStore + SessionDB — turn B on the fresh child waits behind the rotated holder, sees its rows, alternation intact.
302 lines
12 KiB
Python
302 lines
12 KiB
Python
"""Per-session turn lease — serializes the [load history → run → flush] region.
|
|
|
|
Why this exists (#64934): the gateway's busy guards are keyed by ROUTING KEY
|
|
(``_active_sessions`` in the adapter, ``_running_agents`` in the runner), but
|
|
the durable transcript is owned by SESSION_ID — and ``switch_session()`` makes
|
|
the key→id mapping many-to-one (``/resume`` of a named session from a second
|
|
chat/topic, CLI-continuity rebinding, async-delegation completion pinning,
|
|
Telegram topic-binding tip-walks). Two routing keys mapped to one session_id
|
|
run concurrent turns on two different agent objects, so no per-key guard ever
|
|
sees the collision. The two turns then interleave their flushes on one
|
|
transcript: rows persist in completion order instead of arrival order, the
|
|
identity-marker dedup over shared history dicts can swallow a row outright,
|
|
and the second turn runs on a history base that never saw the first turn's
|
|
exchange — leaving a permanent ``user;user`` alternation wedge that
|
|
``repair_message_sequence`` re-repairs on every request forever.
|
|
|
|
The lease closes that route by serializing per RESOLVED session_id: it is
|
|
acquired after session resolution is final (post ``switch_session``/tip-walk),
|
|
immediately before the transcript load, and released in the dispatch layer's
|
|
``finally`` on every exit path. Same-key messages never reach the acquisition
|
|
point while a turn runs (both routing-key guards hold them), so the lock is
|
|
uncontended everywhere except the alias-key route — where the second turn now
|
|
waits for the first turn's flush and logs one WARNING naming the session and
|
|
both routing keys (pairing with the cross-agent tripwire in
|
|
``agent/agent_runtime_helpers.note_turn_start``).
|
|
|
|
Safety properties:
|
|
|
|
- **Generation-scoped, identity-checked release.** A token records its owner
|
|
(routing key, run generation) and release only frees the lease when that
|
|
exact token is the current holder — a stale unwind can never release a
|
|
newer turn's lease (the #28686 ownership lesson applied). Release is
|
|
idempotent.
|
|
- **Fail-open on timeout.** A stuck holder degrades to today's unserialized
|
|
behavior with a loud ERROR after the configured wait — never a wedged
|
|
session. A degraded token holds nothing and releases nothing.
|
|
- **Bounded registry.** The per-session lease map is size-capped; eviction
|
|
only ever removes idle (unheld, uncontended) entries, never a live lease.
|
|
|
|
Known limits (deliberate, flagged on #64934):
|
|
|
|
- A CLI process sharing the session via CLI-continuity is outside any
|
|
in-process lock — that pair needs a DB-level lease (separate design).
|
|
- Mid-turn compression rotation leaves a small alias window: the tip-walk can
|
|
resolve a fresh child id while the parent-holding turn is still in flight.
|
|
The mid-turn binding-sync sites are the right place to alias the lease in a
|
|
follow-up.
|
|
"""
|
|
|
|
import asyncio
|
|
import logging
|
|
import time
|
|
from typing import Dict, Optional
|
|
|
|
logger = logging.getLogger(__name__)
|
|
|
|
# Upper bound on tracked per-session leases. Idle entries (no holder, no
|
|
# waiter) are evicted oldest-first once the cap is reached; live leases are
|
|
# never evicted, so a burst of distinct sessions can transiently exceed the
|
|
# cap rather than break serialization.
|
|
DEFAULT_MAX_LEASES = 512
|
|
|
|
# Fallback wait (seconds) when the caller passes no positive timeout. Matches
|
|
# the gateway's default agent inactivity timeout so a stuck holder fails open
|
|
# on the same clock the turn itself would be declared stuck on.
|
|
DEFAULT_LEASE_WAIT = 1800.0
|
|
|
|
|
|
class TurnLeaseToken:
|
|
"""Handle returned by :meth:`SessionTurnLeaseRegistry.acquire`.
|
|
|
|
``degraded`` means the acquire timed out and the turn is proceeding
|
|
UNSERIALIZED (fail-open); such a token holds nothing and its release is a
|
|
no-op. ``released`` makes release idempotent.
|
|
"""
|
|
|
|
__slots__ = ("session_id", "owner_key", "generation", "degraded", "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})"
|
|
)
|
|
|
|
|
|
class _SessionLease:
|
|
__slots__ = ("lock", "holder", "acquired_at", "last_used")
|
|
|
|
def __init__(self) -> None:
|
|
self.lock = asyncio.Lock()
|
|
self.holder: Optional[TurnLeaseToken] = None
|
|
self.acquired_at = 0.0
|
|
self.last_used = time.time()
|
|
|
|
@property
|
|
def idle(self) -> bool:
|
|
"""True when this lease can be evicted: nobody holds or awaits it."""
|
|
return self.holder is None and not self.lock.locked()
|
|
|
|
|
|
class SessionTurnLeaseRegistry:
|
|
"""Asyncio lease per resolved session_id serializing transcript turns.
|
|
|
|
Process-local and single-event-loop by design — the same visibility scope
|
|
as the routing-key guards it extends. All methods must be called from the
|
|
gateway's event loop.
|
|
"""
|
|
|
|
def __init__(self, max_entries: int = DEFAULT_MAX_LEASES) -> None:
|
|
self._leases: Dict[str, _SessionLease] = {}
|
|
self._max_entries = max(1, int(max_entries))
|
|
|
|
def __len__(self) -> int:
|
|
return len(self._leases)
|
|
|
|
def _get_or_create(self, session_id: str) -> _SessionLease:
|
|
lease = self._leases.get(session_id)
|
|
if lease is None:
|
|
self._evict_idle()
|
|
lease = _SessionLease()
|
|
self._leases[session_id] = lease
|
|
lease.last_used = time.time()
|
|
return lease
|
|
|
|
def _evict_idle(self) -> None:
|
|
"""Drop oldest idle entries so a new lease fits under the cap.
|
|
|
|
Never evicts a held or contended lease — correctness beats the cap.
|
|
"""
|
|
overflow = len(self._leases) - self._max_entries + 1
|
|
if overflow <= 0:
|
|
return
|
|
idle_ids = sorted(
|
|
(sid for sid, lease in self._leases.items() if lease.idle),
|
|
key=lambda sid: self._leases[sid].last_used,
|
|
)
|
|
for sid in idle_ids[:overflow]:
|
|
self._leases.pop(sid, None)
|
|
|
|
async def acquire(
|
|
self,
|
|
session_id: str,
|
|
*,
|
|
owner_key: str,
|
|
generation: int,
|
|
timeout: Optional[float] = None,
|
|
) -> Optional[TurnLeaseToken]:
|
|
"""Acquire the turn lease for ``session_id``, waiting if held.
|
|
|
|
Returns a :class:`TurnLeaseToken` — degraded when the wait timed out
|
|
(fail-open: caller proceeds unserialized). Returns ``None`` for a
|
|
falsy ``session_id``.
|
|
"""
|
|
if not session_id:
|
|
return None
|
|
wait = float(timeout) if timeout and timeout > 0 else DEFAULT_LEASE_WAIT
|
|
token = TurnLeaseToken(session_id, owner_key, int(generation))
|
|
lease = self._get_or_create(session_id)
|
|
|
|
if lease.lock.locked():
|
|
holder = lease.holder
|
|
logger.warning(
|
|
"turn lease contention on session %s: routing key %s (gen %s) "
|
|
"waiting behind in-flight turn held by routing key %s (gen %s, "
|
|
"held %.0fs) — two routing keys are mapped to one session_id "
|
|
"(#64934); serializing this turn behind the previous turn's "
|
|
"flush",
|
|
session_id,
|
|
owner_key,
|
|
generation,
|
|
holder.owner_key if holder else "?",
|
|
holder.generation if holder else "?",
|
|
time.time() - lease.acquired_at if lease.acquired_at else -1.0,
|
|
)
|
|
|
|
try:
|
|
await asyncio.wait_for(lease.lock.acquire(), timeout=wait)
|
|
except asyncio.TimeoutError:
|
|
holder = lease.holder
|
|
logger.error(
|
|
"turn lease wait timed out after %.0fs on session %s "
|
|
"(waiter: routing key %s gen %s; holder: routing key %s "
|
|
"gen %s) — failing open: this turn runs UNSERIALIZED against "
|
|
"the stuck holder rather than wedging the session; transcript "
|
|
"writes may interleave",
|
|
wait,
|
|
session_id,
|
|
owner_key,
|
|
generation,
|
|
holder.owner_key if holder else "?",
|
|
holder.generation if holder else "?",
|
|
)
|
|
token.degraded = True
|
|
return token
|
|
|
|
lease.holder = token
|
|
lease.acquired_at = time.time()
|
|
lease.last_used = lease.acquired_at
|
|
return token
|
|
|
|
def rebind(self, token: Optional[TurnLeaseToken], new_session_id: str) -> bool:
|
|
"""Alias a HELD lease onto ``new_session_id`` after mid-turn rotation.
|
|
|
|
Compression can rotate the durable session_id while a turn is in
|
|
flight (session-hygiene pre-compression, in-agent compression). The
|
|
turn's flush then targets the NEW id — so the serialization boundary
|
|
must follow it, or an alias routing key resolving the new id (e.g. a
|
|
topic tip-walk landing on the fresh child) could start a concurrent
|
|
turn the lease never sees. This closes the rotation-alias window
|
|
flagged on #64934.
|
|
|
|
Mechanism: the SAME ``_SessionLease`` object is registered under the
|
|
new id (the old mapping stays until it goes idle and is evicted), so
|
|
acquirers on either id serialize against one lock — no lock state is
|
|
moved, no asyncio internals are touched. Only the current holder can
|
|
rebind (identity-checked like release), and the token follows to the
|
|
new id so release frees the shared object.
|
|
|
|
Edge: if the new id already has a live lease of its own (another
|
|
turn is running on the target session), the two serialization
|
|
domains cannot be merged mid-wait — log loudly and keep the token on
|
|
the old id. Fail-open, never deadlock: a holder cannot wait mid-turn.
|
|
"""
|
|
if (
|
|
token is None
|
|
or token.degraded
|
|
or token.released
|
|
or not new_session_id
|
|
or new_session_id == token.session_id
|
|
):
|
|
return False
|
|
lease = self._leases.get(token.session_id)
|
|
if lease is None or lease.holder is not token:
|
|
return False
|
|
|
|
existing = self._leases.get(new_session_id)
|
|
if existing is not None and existing is not lease and not existing.idle:
|
|
holder = existing.holder
|
|
logger.warning(
|
|
"turn lease rebind blocked: session %s rotated to %s mid-turn "
|
|
"(holder: routing key %s gen %s) but the target session's "
|
|
"lease is already live (holder: routing key %s gen %s) — "
|
|
"keeping the lease on the old id; transcript writes on %s "
|
|
"may interleave (#64934 rotation-alias edge)",
|
|
token.session_id,
|
|
new_session_id,
|
|
token.owner_key,
|
|
token.generation,
|
|
holder.owner_key if holder else "?",
|
|
holder.generation if holder else "?",
|
|
new_session_id,
|
|
)
|
|
return False
|
|
|
|
self._leases[new_session_id] = lease
|
|
lease.last_used = time.time()
|
|
token.session_id = new_session_id
|
|
return True
|
|
|
|
def release(self, token: Optional[TurnLeaseToken]) -> bool:
|
|
"""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.
|
|
"""
|
|
if token is None or token.degraded or token.released:
|
|
return False
|
|
token.released = True
|
|
lease = self._leases.get(token.session_id)
|
|
if lease is None:
|
|
return False
|
|
if lease.holder is not token:
|
|
logger.debug(
|
|
"turn lease release skipped on session %s: token (key %s "
|
|
"gen %s) is not the current holder",
|
|
token.session_id,
|
|
token.owner_key,
|
|
token.generation,
|
|
)
|
|
return False
|
|
lease.holder = None
|
|
lease.acquired_at = 0.0
|
|
lease.last_used = time.time()
|
|
if lease.lock.locked():
|
|
lease.lock.release()
|
|
return True
|