fix(gateway): slash commands report an unreadable transcript instead of replying nothing
Partial salvage of #100887 by @jwilson411. The restore-path half (raise TranscriptReadError from load_transcript, fail the turn closed) already landed via #100910; this keeps the complementary half: every slash-command handler and platform helper that reads the transcript now catches TranscriptReadError and tells the user the history exists but is unreadable, instead of letting the exception reach the dispatch wrapper, which logs it and sends no reply. (cherry picked from commit 2a132c903f51c8d21f6eea64ddeef688c9619a18, run.py/session.py hunks dropped as already on main; notice-path tests replaced accordingly)
This commit is contained in:
@@ -655,7 +655,7 @@ from pathlib import Path as _Path
|
||||
sys.path.insert(0, str(_Path(__file__).resolve().parents[2]))
|
||||
|
||||
from gateway.config import Platform, PlatformConfig
|
||||
from gateway.session import SessionSource, build_session_key
|
||||
from gateway.session import SessionSource, TranscriptReadError, build_session_key
|
||||
from hermes_constants import get_default_hermes_root, get_hermes_dir, get_hermes_home
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@@ -4160,6 +4160,12 @@ class BasePlatformAdapter(ABC):
|
||||
if callable(peek):
|
||||
session_id = peek(session_key)
|
||||
transcript = store.load_transcript(session_id or session_key)
|
||||
except TranscriptReadError:
|
||||
logger.warning(
|
||||
"Transcript read failed for session %s; media dedup runs "
|
||||
"with no history this turn (#100788)", session_key,
|
||||
)
|
||||
return None
|
||||
except Exception:
|
||||
return None
|
||||
if not transcript:
|
||||
|
||||
@@ -96,7 +96,7 @@ from gateway.platforms.yuanbao_proto import (
|
||||
encode_get_group_member_list,
|
||||
next_seq_no,
|
||||
)
|
||||
from gateway.session import build_session_key
|
||||
from gateway.session import TranscriptReadError, build_session_key
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -1144,6 +1144,14 @@ class RecallGuardMiddleware(InboundMiddleware):
|
||||
await asyncio.sleep(0.5)
|
||||
try:
|
||||
transcript = store.load_transcript(sid)
|
||||
except TranscriptReadError as exc:
|
||||
# No readable rows means nothing to redact; polling on
|
||||
# would just re-log the same failure (#100788).
|
||||
logger.warning(
|
||||
"[%s] Recall redact: transcript unreadable for "
|
||||
"session %s: %s", adapter.name, sid, exc,
|
||||
)
|
||||
return
|
||||
except Exception:
|
||||
continue
|
||||
for entry in transcript:
|
||||
@@ -1183,6 +1191,11 @@ class RecallGuardMiddleware(InboundMiddleware):
|
||||
# match) is the canonical path again.
|
||||
try:
|
||||
transcript = store.load_transcript(sid)
|
||||
except TranscriptReadError as exc:
|
||||
# Not an empty transcript — the rows are unreadable, so recall has
|
||||
# nothing to match against (#100788).
|
||||
logger.warning("[%s] Recall: transcript unreadable: %s", adapter.name, exc)
|
||||
return
|
||||
except Exception as exc:
|
||||
logger.warning("[%s] Recall: failed to load transcript: %s", adapter.name, exc)
|
||||
return
|
||||
@@ -2145,6 +2158,13 @@ class QuoteContextMiddleware(InboundMiddleware):
|
||||
if kind in _RESOLVABLE_MEDIA_KINDS:
|
||||
media_refs.append((rid, kind, filename.strip()))
|
||||
break
|
||||
except TranscriptReadError as exc:
|
||||
# Quote resolution degrades to "no refs" rather than pretending
|
||||
# the quoted message was never seen (#100788).
|
||||
logger.warning(
|
||||
"[%s] quote transcript lookup: transcript unreadable: %s",
|
||||
getattr(adapter, "name", "yuanbao"), exc,
|
||||
)
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"[%s] quote transcript lookup failed: %s",
|
||||
@@ -2747,6 +2767,14 @@ class MediaResolveMiddleware(InboundMiddleware):
|
||||
try:
|
||||
session_entry = store.get_or_create_session(source)
|
||||
history = store.load_transcript(session_entry.session_id)
|
||||
except TranscriptReadError as exc:
|
||||
# Hydrate nothing rather than silently acting as if the session
|
||||
# had no observed media (#100788).
|
||||
logger.warning(
|
||||
"[%s] Observed-media hydration: transcript unreadable: %s",
|
||||
adapter.name, exc,
|
||||
)
|
||||
return [], []
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"[%s] Observed-media hydration setup failed: %s",
|
||||
|
||||
@@ -37,6 +37,7 @@ from gateway.platforms.base import EphemeralReply, MessageEvent, MessageType
|
||||
from gateway.session import (
|
||||
AsyncSessionStore,
|
||||
SessionSource,
|
||||
TranscriptReadError,
|
||||
build_session_key,
|
||||
is_shared_multi_user_session,
|
||||
)
|
||||
@@ -49,6 +50,11 @@ from utils import (
|
||||
|
||||
logger = logging.getLogger("gateway.run")
|
||||
|
||||
HISTORY_UNREADABLE = (
|
||||
"⚠️ Conversation history is unreadable (state.db). "
|
||||
"This is not a new conversation — earlier messages exist but cannot be loaded."
|
||||
)
|
||||
|
||||
# Upper bound on the off-loop agent-resource cleanup during a /new or /reset
|
||||
# (see _handle_reset_command). A stuck teardown must not block the event loop;
|
||||
# past this the reset proceeds and the cleanup is left to finish (or leak) in
|
||||
@@ -963,7 +969,10 @@ class GatewaySlashCommandsMixin:
|
||||
return "\n".join(lines)
|
||||
|
||||
# Last resort: rough estimate from transcript
|
||||
history = await self.async_session_store.load_transcript(session_entry.session_id)
|
||||
try:
|
||||
history = await self.async_session_store.load_transcript(session_entry.session_id)
|
||||
except TranscriptReadError:
|
||||
return HISTORY_UNREADABLE
|
||||
if history:
|
||||
from agent.model_metadata import estimate_messages_tokens_rough
|
||||
|
||||
@@ -2641,8 +2650,11 @@ class GatewaySlashCommandsMixin:
|
||||
"""Handle /retry command - re-send the last user message."""
|
||||
source = event.source
|
||||
session_entry = await self.async_session_store.get_or_create_session(source)
|
||||
history = await self.async_session_store.load_transcript(session_entry.session_id)
|
||||
|
||||
try:
|
||||
history = await self.async_session_store.load_transcript(session_entry.session_id)
|
||||
except TranscriptReadError:
|
||||
return HISTORY_UNREADABLE
|
||||
|
||||
# Find the last *real* user message. Timeline bookkeeping rows carry
|
||||
# role=user + display_kind (model_switch / async_delegation_complete /
|
||||
# auto_continue / hidden); clients never count them as user turns.
|
||||
@@ -3693,7 +3705,10 @@ class GatewaySlashCommandsMixin:
|
||||
|
||||
source = event.source
|
||||
session_entry = await self.async_session_store.get_or_create_session(source)
|
||||
history = await self.async_session_store.load_transcript(session_entry.session_id)
|
||||
try:
|
||||
history = await self.async_session_store.load_transcript(session_entry.session_id)
|
||||
except TranscriptReadError:
|
||||
return HISTORY_UNREADABLE
|
||||
if not history:
|
||||
return t("gateway.btw.no_history")
|
||||
|
||||
@@ -4561,7 +4576,10 @@ class GatewaySlashCommandsMixin:
|
||||
"""
|
||||
source = event.source
|
||||
session_entry = await self.async_session_store.get_or_create_session(source)
|
||||
history = await self.async_session_store.load_transcript(session_entry.session_id)
|
||||
try:
|
||||
history = await self.async_session_store.load_transcript(session_entry.session_id)
|
||||
except TranscriptReadError:
|
||||
return HISTORY_UNREADABLE
|
||||
|
||||
if not history or len(history) < 4:
|
||||
return t("gateway.compress.not_enough")
|
||||
@@ -5352,7 +5370,17 @@ class GatewaySlashCommandsMixin:
|
||||
title = await self._session_db.get_session_title(target_id) or name
|
||||
|
||||
# Count messages for context
|
||||
history = await self.async_session_store.load_transcript(target_id)
|
||||
try:
|
||||
history = await self.async_session_store.load_transcript(target_id)
|
||||
except TranscriptReadError:
|
||||
# The resume itself succeeded; only the count is missing. Say the
|
||||
# history is unreadable rather than reporting an empty session
|
||||
# (#100788).
|
||||
return (
|
||||
t("gateway.resume.resumed_no_count", title=title)
|
||||
+ "\n"
|
||||
+ HISTORY_UNREADABLE
|
||||
)
|
||||
msg_count = len([m for m in history if m.get("role") == "user"]) if history else 0
|
||||
msg_part = f" ({msg_count} message{'s' if msg_count != 1 else ''})" if msg_count else ""
|
||||
|
||||
@@ -5469,7 +5497,10 @@ class GatewaySlashCommandsMixin:
|
||||
|
||||
# Load the current session and its transcript
|
||||
current_entry = await self.async_session_store.get_or_create_session(source)
|
||||
history = await self.async_session_store.load_transcript(current_entry.session_id)
|
||||
try:
|
||||
history = await self.async_session_store.load_transcript(current_entry.session_id)
|
||||
except TranscriptReadError:
|
||||
return HISTORY_UNREADABLE
|
||||
if not history:
|
||||
return t("gateway.branch.no_conversation")
|
||||
|
||||
@@ -5654,6 +5685,10 @@ class GatewaySlashCommandsMixin:
|
||||
try:
|
||||
entry = self.session_store.get_or_create_session(source)
|
||||
history = self.session_store.load_transcript(entry.session_id) or []
|
||||
except TranscriptReadError:
|
||||
# A read failure is not an empty transcript (#100788): the
|
||||
# breakdown would understate the context by the whole chat.
|
||||
return [HISTORY_UNREADABLE]
|
||||
except Exception:
|
||||
history = []
|
||||
|
||||
@@ -5685,6 +5720,10 @@ class GatewaySlashCommandsMixin:
|
||||
try:
|
||||
entry = self.session_store.get_or_create_session(source)
|
||||
history = self.session_store.load_transcript(entry.session_id) or []
|
||||
except TranscriptReadError:
|
||||
# See _context_breakdown_block: don't pass a read failure off
|
||||
# as an empty transcript (#100788).
|
||||
return [HISTORY_UNREADABLE]
|
||||
except Exception:
|
||||
history = []
|
||||
|
||||
@@ -5868,7 +5907,10 @@ class GatewaySlashCommandsMixin:
|
||||
|
||||
# No agent at all -- check session history for a rough count
|
||||
session_entry = await self.async_session_store.get_or_create_session(source)
|
||||
history = await self.async_session_store.load_transcript(session_entry.session_id)
|
||||
try:
|
||||
history = await self.async_session_store.load_transcript(session_entry.session_id)
|
||||
except TranscriptReadError:
|
||||
return HISTORY_UNREADABLE
|
||||
if history:
|
||||
from agent.model_metadata import estimate_messages_tokens_rough
|
||||
msgs = [m for m in history if m.get("role") in {"user", "assistant"} and m.get("content")]
|
||||
|
||||
@@ -0,0 +1,108 @@
|
||||
"""A failed transcript read must not masquerade as an empty history (#100788).
|
||||
|
||||
The gateway restore path (``_handle_message``) already fails closed on
|
||||
current main (#100910). This file covers the surviving half of PR #100887:
|
||||
the slash-command handlers, which used to let ``TranscriptReadError``
|
||||
propagate into the dispatch wrapper and reply with nothing at all.
|
||||
|
||||
Incident shape: a malformed ``state.db`` made every
|
||||
``SessionStore.load_transcript`` raise; the except-block swallowed it and
|
||||
returned ``[]``. Restore then rebuilt the turn from "no history", so a
|
||||
long-running chat silently restarted as a brand-new conversation and the
|
||||
model happily answered as if nothing had ever been discussed.
|
||||
|
||||
Two guarantees under test:
|
||||
A. ``load_transcript`` raises ``TranscriptReadError`` on a read failure,
|
||||
while a genuinely empty session still returns ``[]``.
|
||||
B. The gateway restore path degrades loudly: history stays empty, and a
|
||||
per-turn ephemeral notice is queued telling the model the history
|
||||
exists but is unreadable.
|
||||
|
||||
Offline: SQLite on tmp_path only, no network.
|
||||
"""
|
||||
|
||||
import sqlite3
|
||||
|
||||
import pytest
|
||||
|
||||
from gateway.config import GatewayConfig
|
||||
from gateway.session import SessionStore, TranscriptReadError
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def store(tmp_path):
|
||||
return SessionStore(sessions_dir=tmp_path / "gw", config=GatewayConfig())
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# A. read failure != empty transcript (landed on main via #100910; kept as
|
||||
# the contract the slash-command handlers below rely on)
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestLoadTranscriptReadFailure:
|
||||
def test_read_failure_raises_instead_of_returning_empty(self, store, monkeypatch):
|
||||
db = store._db
|
||||
assert db is not None
|
||||
db.create_session("s1", "telegram", session_key="telegram:1")
|
||||
db.append_message("s1", "user", "the conversation we must not forget")
|
||||
|
||||
boom = sqlite3.DatabaseError("database disk image is malformed")
|
||||
|
||||
def _raise(*_args, **_kwargs):
|
||||
raise boom
|
||||
|
||||
monkeypatch.setattr(db, "get_messages_as_conversation", _raise)
|
||||
|
||||
with pytest.raises(TranscriptReadError) as excinfo:
|
||||
store.load_transcript("s1")
|
||||
|
||||
assert excinfo.value.session_id == "s1"
|
||||
assert excinfo.value.__cause__ is boom
|
||||
|
||||
def test_genuinely_empty_session_still_returns_empty_list(self, store):
|
||||
db = store._db
|
||||
assert db is not None
|
||||
db.create_session("s2", "telegram", session_key="telegram:2")
|
||||
|
||||
assert store.load_transcript("s2") == []
|
||||
|
||||
def test_no_db_still_returns_empty_list(self, store):
|
||||
# "No DB for this session" really is an empty transcript, not a
|
||||
# failure — that path must keep its [] contract.
|
||||
store._db = None
|
||||
assert store.load_transcript("nope") == []
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# B. restore path: empty history + a degraded-history notice
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# B. slash-command handlers surface the failure instead of dying silently.
|
||||
# Before: the handler raised, base.py's dispatch wrapper logged
|
||||
# "Command '/x' dispatch failed" and the user got NO reply at all.
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestSlashCommandsOnUnreadableTranscript:
|
||||
def test_history_unreadable_text_is_explicit(self):
|
||||
from gateway.slash_commands import HISTORY_UNREADABLE
|
||||
|
||||
assert "unreadable" in HISTORY_UNREADABLE
|
||||
assert "not a new conversation" in HISTORY_UNREADABLE
|
||||
|
||||
def test_every_transcript_reading_handler_catches_the_error(self):
|
||||
"""No `await ...load_transcript(` in the mixin may be left uncaught."""
|
||||
import inspect
|
||||
import re
|
||||
|
||||
from gateway import slash_commands as sc
|
||||
|
||||
src = inspect.getsource(sc)
|
||||
# Each awaited load_transcript must sit inside a try: whose handlers
|
||||
# include TranscriptReadError within the following ~6 lines.
|
||||
for m in re.finditer(r"await self\.async_session_store\.load_transcript\(", src):
|
||||
window = src[m.end() : m.end() + 400]
|
||||
assert "except TranscriptReadError" in window, src[m.start() - 200 : m.end() + 100]
|
||||
Reference in New Issue
Block a user