diff --git a/gateway/run.py b/gateway/run.py index 940a9c58b05d0..f33e43dd1cfa5 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -3487,8 +3487,33 @@ def _normalize_empty_agent_response( return response if agent_result.get("failed"): - error_detail = agent_result.get("error", "unknown error") + # None-safe: the gateway result dict is built with + # ``'error': holder.get('error')`` and can carry an EXPLICIT None, + # which bypasses dict.get's default and would render + # "The request failed: None". + error_detail = agent_result.get("error") or "unknown error" error_str = str(error_detail).lower() + # Session-persistence failures get a dedicated recovery message. + # Suggesting /reset here would be actively harmful: it destroys the + # user's conversation context and does nothing to fix the underlying + # storage problem (lock contention, disk exhaustion, ...). + failure_reason = str(agent_result.get("failure_reason") or "") + if failure_reason.startswith("session_persistence_failed") or ( + "session storage" in error_str + ): + if failure_reason.endswith(":disk") or "disk" in error_str: + return ( + "⚠️ Session storage was temporarily unavailable, so this " + "turn was stopped to protect your conversation history. " + "Please check available disk space, then send your " + "message again." + ) + return ( + "⚠️ Session storage was temporarily unavailable, so this " + "turn was stopped to protect your conversation history. " + "Your message was recorded — please send it again in a " + "moment." + ) is_context_failure = any( p in error_str for p in ("context", "token", "too large", "too long", "exceed", "payload") diff --git a/tests/gateway/test_normalize_empty_agent_response.py b/tests/gateway/test_normalize_empty_agent_response.py new file mode 100644 index 0000000000000..287012a4c444a --- /dev/null +++ b/tests/gateway/test_normalize_empty_agent_response.py @@ -0,0 +1,128 @@ +"""Unit tests for persistence-failure-aware messaging in +``_normalize_empty_agent_response``. + +When a turn is stopped because session persistence failed (SQLite lock +contention, disk exhaustion, ...), the user must NOT be told to /reset — +that destroys their conversation context and does nothing to fix storage. +They must also never see 'The request failed: None' when the gateway result +dict carries an explicit ``error: None``. +""" + +import pytest + +from gateway.run import _normalize_empty_agent_response + + +class TestPersistenceFailureRecoveryMessage: + """Failed turns whose failure_reason marks a session-persistence + failure get a dedicated recovery message: reassure the user their + history is protected, tell them to resend — never suggest /reset.""" + + def test_locked_persistence_failure_gets_recovery_message(self): + agent_result = { + "final_response": "", + "failed": True, + "failure_reason": "session_persistence_failed:locked", + "error": "session storage was locked by another writer", + "api_calls": 2, + } + + response = _normalize_empty_agent_response(agent_result, "", history_len=10) + + assert "send it again" in response.lower() + assert "/reset" not in response + assert "unknown error" not in response.lower() + + def test_disk_persistence_failure_mentions_disk(self): + agent_result = { + "final_response": "", + "failed": True, + "failure_reason": "session_persistence_failed:disk", + "error": "session storage write failed: disk full", + "api_calls": 1, + } + + response = _normalize_empty_agent_response(agent_result, "", history_len=10) + + assert "disk" in response.lower() + assert "/reset" not in response + assert "unknown error" not in response.lower() + + def test_unknown_cause_persistence_failure_still_avoids_reset(self): + agent_result = { + "final_response": "", + "failed": True, + "failure_reason": "session_persistence_failed:unknown", + "error": "session storage failure", + "api_calls": 1, + } + + response = _normalize_empty_agent_response(agent_result, "", history_len=10) + + assert "/reset" not in response + assert "send it again" in response.lower() + + def test_legacy_shape_error_text_mentioning_session_storage(self): + """Legacy failed results carry no failure_reason but an error text + naming session storage — they must get the same recovery message.""" + agent_result = { + "final_response": "", + "failed": True, + "error": "turn stopped: session storage unavailable", + "api_calls": 1, + } + + response = _normalize_empty_agent_response(agent_result, "", history_len=10) + + assert "/reset" not in response + assert "send it again" in response.lower() + + +class TestExplicitNoneErrorIsNoneSafe: + """The gateway result dict is built with ``'error': holder.get('error')`` + and can carry an EXPLICIT None, which bypasses dict.get defaults.""" + + def test_explicit_none_error_never_renders_none(self): + agent_result = { + "final_response": "", + "failed": True, + "error": None, + "api_calls": 1, + } + + response = _normalize_empty_agent_response(agent_result, "", history_len=10) + + assert "None" not in response + # Non-persistence generic failures may legitimately say + # 'unknown error' — the defect is rendering the literal None. + assert "unknown error" in response.lower() + + +class TestGenericFailureRegression: + """Non-persistence failures keep the existing byte-identical message.""" + + def test_provider_error_still_formats_request_failed(self): + agent_result = { + "final_response": "", + "failed": True, + "error": "provider exploded", + "api_calls": 1, + } + + response = _normalize_empty_agent_response(agent_result, "", history_len=10) + + assert "The request failed: provider exploded" in response + assert "/reset" in response + + def test_context_failure_branch_unchanged(self): + agent_result = { + "final_response": "", + "failed": True, + "error": "prompt exceeds context window", + "api_calls": 1, + } + + response = _normalize_empty_agent_response(agent_result, "", history_len=60) + + assert "context window" in response + assert "/compact" in response