From 65cb70b8d09c2ae2a87dea754c429687d69f2252 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Fri, 3 Jul 2026 03:17:07 +0530 Subject: [PATCH] refactor(gateway): add SessionStore.peek_session_id public accessor for webhook close Replace the webhook delivery-close path's direct reach into private SessionStore._entries (which also bypassed the store lock) with a public, lock-held peek_session_id(session_key) accessor. Mirrors the existing lookup_by_session_id inverse helper. Keeps a getattr fallback for older stores / test doubles. Adds a unit test for the accessor. --- gateway/platforms/webhook.py | 25 +++++++++----- gateway/session.py | 16 +++++++++ tests/gateway/test_webhook_session_close.py | 38 +++++++++++++++++++++ 3 files changed, 71 insertions(+), 8 deletions(-) diff --git a/gateway/platforms/webhook.py b/gateway/platforms/webhook.py index bb1fefb673e..8327213a056 100644 --- a/gateway/platforms/webhook.py +++ b/gateway/platforms/webhook.py @@ -776,14 +776,23 @@ class WebhookAdapter(BasePlatformAdapter): if key_fn is None: return session_key = key_fn(event.source) - if hasattr(store, "_ensure_loaded"): - try: - store._ensure_loaded() - except Exception: - pass - entries = getattr(store, "_entries", {}) or {} - entry = entries.get(session_key) - session_id = getattr(entry, "session_id", None) if entry else None + # Resolve the persisted session_id via the store's public, + # lock-held accessor (peek_session_id) rather than reaching into + # the private _entries dict without the store lock. Fall back to + # the private path only for older stores / test doubles that + # predate the accessor. + peek = getattr(store, "peek_session_id", None) + if callable(peek): + session_id = peek(session_key) + else: + if hasattr(store, "_ensure_loaded"): + try: + store._ensure_loaded() + except Exception: + pass + entries = getattr(store, "_entries", {}) or {} + entry = entries.get(session_key) + session_id = getattr(entry, "session_id", None) if entry else None if not session_id: logger.debug( "[webhook] No session_id to close for %s (key=%s)", diff --git a/gateway/session.py b/gateway/session.py index fd2fae87f38..2a75aa16d7f 100644 --- a/gateway/session.py +++ b/gateway/session.py @@ -1887,6 +1887,22 @@ class SessionStore: if entry.session_id == session_id: return entry return None + + def peek_session_id(self, session_key: str) -> Optional[str]: + """Return the persisted session_id currently bound to a session key. + + Public, lock-held accessor for the key→session_id mapping. Callers that + need to resolve the session row for a source (e.g. the webhook + delivery-close path) should use this rather than reaching into the + private ``_entries`` dict without holding ``self._lock``. Returns None + when the key is unknown or has no session_id yet. + """ + if not session_key: + return None + with self._lock: + self._ensure_loaded_locked() + entry = self._entries.get(session_key) + return getattr(entry, "session_id", None) if entry else None def append_to_transcript(self, session_id: str, message: Dict[str, Any], skip_db: bool = False) -> None: """Append a message to a session's transcript (SQLite). diff --git a/tests/gateway/test_webhook_session_close.py b/tests/gateway/test_webhook_session_close.py index e91f02f73d7..804bed07bf6 100644 --- a/tests/gateway/test_webhook_session_close.py +++ b/tests/gateway/test_webhook_session_close.py @@ -176,3 +176,41 @@ async def test_webhook_session_closed_even_when_agent_run_raises(tmp_path): ) assert row["end_reason"] == "webhook_complete" store._db.close() + + +def test_peek_session_id_resolves_bound_key(tmp_path): + """SessionStore.peek_session_id returns the session_id bound to a key. + + This is the public, lock-held accessor the webhook close path uses to + resolve a session row from its key without reaching into the private + ``_entries`` dict. A missing/unknown key returns None (so the close path + debug-logs and no-ops rather than closing the wrong row). + """ + sessions_dir = tmp_path / "sessions" + sessions_dir.mkdir() + config = GatewayConfig( + platforms={Platform.WEBHOOK: PlatformConfig(enabled=True)} + ) + store = SessionStore(sessions_dir=sessions_dir, config=config) + + adapter = _make_adapter( + {"alerts": {"secret": _INSECURE_NO_AUTH, "prompt": "x", "deliver": "log"}} + ) + source = adapter.build_source( + chat_id="webhook:alerts:peek-001", + chat_name="webhook/alerts", + chat_type="webhook", + user_id="webhook:alerts", + user_name="alerts", + ) + entry = store.get_or_create_session(source) + key = store._generate_session_key(source) + + # Known key → the bound session_id. + assert store.peek_session_id(key) == entry.session_id + # Unknown key and empty key → None (never a wrong-row close). + assert store.peek_session_id("no:such:key") is None + assert store.peek_session_id("") is None + if store._db is not None: + store._db.close() +