diff --git a/gateway/run.py b/gateway/run.py index aac6a192555f9..4d08c3c90b241 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -534,6 +534,32 @@ def _looks_like_gateway_provider_error(text: str) -> bool: return bool(_GATEWAY_PROVIDER_ERROR_SHAPE_RE.search(body)) +# Issue #72131: track provider-error status text sent via the plain-send fallback +# so that _sanitize_gateway_final_response can dedup identical final responses. +_last_sent_provider_error_status: Optional[str] = None +# Set to True when _sanitize_gateway_final_response suppresses output due to dedup. +_deduped_provider_error: bool = False + + +def _record_provider_error_status(text: str) -> None: + """Record a provider-error status that was sent as a persistent message. + + Called when _send_or_update_status_coro falls back to plain send() (adapter + lacks send_or_update_status) and the content is a provider-error reply. + """ + global _last_sent_provider_error_status, _deduped_provider_error + _last_sent_provider_error_status = text + _deduped_provider_error = False # reset; only meaningful after sanitize + + +def _should_dedup_final_response(text: str) -> bool: + """Return True if text duplicates a provider-error status already sent.""" + return bool( + _last_sent_provider_error_status + and text == _last_sent_provider_error_status + ) + + def _sanitize_gateway_final_response(platform: Any, text: str) -> str: """Sanitize final gateway replies before sending them to chat surfaces. @@ -556,7 +582,17 @@ def _sanitize_gateway_final_response(platform: Any, text: str) -> str: redacted = _redact_gateway_user_facing_secrets(str(text)) if _looks_like_gateway_provider_error(redacted): - return _gateway_provider_error_reply(redacted) + result = _gateway_provider_error_reply(redacted) + # Issue #72131: if this identical provider-error text was already + # delivered as a mid-run status (via the plain-send fallback), + # suppress the duplicate final response. + global _deduped_provider_error + if _should_dedup_final_response(result): + _deduped_provider_error = True + return "" + _deduped_provider_error = False + return result + _deduped_provider_error = False return redacted @@ -611,10 +647,18 @@ async def _send_or_update_status_coro(adapter, chat_id, status_key, content, met Issue #30045: adapters that implement send_or_update_status (currently Telegram) edit the previous bubble for the same status_key instead of appending a new one. Adapters without the method fall back to plain send. + + Issue #72131: when the fallback plain-send path is used and the content + is a provider-error reply, record it so the final-response dedup can + suppress the identical persistent message that would otherwise follow. """ sender = getattr(adapter, "send_or_update_status", None) if callable(sender): return await sender(chat_id, status_key, content, metadata=metadata) + # Fallback: plain send() → status becomes a persistent message. + # If it's a provider-error reply, record it for final-response dedup. + if _looks_like_gateway_provider_error(content): + _record_provider_error_status(content) return await adapter.send(chat_id, content, metadata=metadata) @@ -22391,7 +22435,10 @@ def _approval_notify_sync(approval_data: dict) -> None: result, final_response or "", history_len=len(agent_history), ) final_response = _sanitize_gateway_final_response(source.platform, final_response) - if not final_response: + # Issue #72131: if dedup suppressed a provider-error final + # response (already delivered as status), do NOT fall back to + # the raw error text — the user already has the sanitized reply. + if not final_response and not _deduped_provider_error: final_response = f"⚠️ {result['error']}" if result.get("error") else "" return { "final_response": final_response, diff --git a/tests/gateway/test_provider_error_dedup.py b/tests/gateway/test_provider_error_dedup.py new file mode 100644 index 0000000000000..dd65b17710f47 --- /dev/null +++ b/tests/gateway/test_provider_error_dedup.py @@ -0,0 +1,118 @@ +"""Tests for provider-error dedup between status and final response (issue #72131).""" + +import pytest + +from gateway.run import ( + _record_provider_error_status, + _sanitize_gateway_final_response, + _should_dedup_final_response, + _last_sent_provider_error_status, + _deduped_provider_error, +) + + +class TestProviderErrorDedup: + """Verify that _sanitize_gateway_final_response suppresses duplicates + when the same provider-error text was already sent as a status message + via the plain-send fallback (adapter without send_or_update_status). + """ + + def setup_method(self): + """Reset module-level dedup state before each test.""" + import gateway.run as _run + _run._last_sent_provider_error_status = None + _run._deduped_provider_error = False + + # -- recording -- + + def test_record_sets_tracking(self): + _record_provider_error_status("⏱️ rate limited") + assert _should_dedup_final_response("⏱️ rate limited") is True + assert _should_dedup_final_response("something else") is False + + def test_record_resets_dedup_flag(self): + import gateway.run as _run + _run._deduped_provider_error = True + _record_provider_error_status("⏱️ rate limited") + assert _run._deduped_provider_error is False + + # -- dedup logic -- + + def test_no_prior_status_no_dedup(self): + """Without a prior status, no dedup should fire.""" + assert _should_dedup_final_response("anything") is False + + def test_identical_text_dedups(self): + _record_provider_error_status( + "⏱️ The model provider is rate-limiting requests. " + "Please wait a moment and try again." + ) + assert _should_dedup_final_response( + "⏱️ The model provider is rate-limiting requests. " + "Please wait a moment and try again." + ) is True + + def test_different_text_no_dedup(self): + _record_provider_error_status("⏱️ rate limited") + assert _should_dedup_final_response("⚠️ auth failed") is False + + # -- sanitize dedup -- + + @pytest.mark.parametrize("platform", ["whatsapp", "discord", "slack", "signal", "matrix"]) + def test_sanitize_dedups_identical_provider_error(self, platform): + """When the same provider-error text was sent as a status, the final + response should be empty (deduped).""" + raw = "API call failed after 3 retries: HTTP 429 rate limited" + + # Simulate: status was sent via plain-send fallback + _record_provider_error_status( + "⏱️ The model provider is rate-limiting requests. " + "Please wait a moment and try again." + ) + + # Now the final response comes in — should be deduped + result = _sanitize_gateway_final_response(platform, raw) + assert result == "" + + import gateway.run as _run + assert _run._deduped_provider_error is True + + @pytest.mark.parametrize("platform", ["whatsapp", "discord", "slack", "signal", "matrix"]) + def test_sanitize_passes_when_no_prior_status(self, platform): + """Without a prior status, provider errors should be sanitized normally.""" + raw = "API call failed after 3 retries: HTTP 429 rate limited" + + result = _sanitize_gateway_final_response(platform, raw) + assert result == ( + "⏱️ The model provider is rate-limiting requests. " + "Please wait a moment and try again." + ) + + @pytest.mark.parametrize("platform", ["whatsapp", "discord", "slack", "signal", "matrix"]) + def test_sanitize_passes_when_text_differs(self, platform): + """If the final response text differs from the status, pass it through.""" + raw = "API call failed after 3 retries: HTTP 401 unauthorized" + + # A different error was sent as status + _record_provider_error_status( + "⏱️ The model provider is rate-limiting requests. " + "Please wait a moment and try again." + ) + + result = _sanitize_gateway_final_response(platform, raw) + # Auth error produces different text + assert "authentication failed" in result.lower() or "provider" in result.lower() + + def test_local_platform_not_deduped(self): + """Local/CLI platforms pass raw text unchanged — no dedup needed.""" + raw = "API call failed after 3 retries: HTTP 429 rate limited" + + _record_provider_error_status( + "⏱️ The model provider is rate-limiting requests. " + "Please wait a moment and try again." + ) + + result = _sanitize_gateway_final_response("local", raw) + # Local platforms keep raw text + assert "429" in result + assert "rate" in result.lower()