From 49567927af562d737c4daab320cc911654f7aff0 Mon Sep 17 00:00:00 2001 From: x7peeps Date: Mon, 27 Jul 2026 02:48:54 +0800 Subject: [PATCH] fix(gateway): dedup provider-error status and final response for adapters without send_or_update_status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #72131 根因分析: 在平台适配器未实现可选方法 send_or_update_status 时，_send_or_update_status_coro 会降级为 调用 adapter.send() 发送状态消息。当 provider 发生错误时，两条通道都会发送相同的改写后文本： 1. 运行中状态回调 → _prepare_gateway_status_message 改写后通过 _send_or_update_status_coro 发送 2. 失败的 turn 最终响应 → _sanitize_gateway_final_response 改写后再次发送 对于 Telegram 等实现了 send_or_update_status 的适配器，状态气泡会被原地编辑并在运行结束后 清理，所以只有最终消息保留。但对于没有该方法的适配器，状态消息变成持久消息，与最终响应产生 重复。 修复方式: - 新增模块级变量 _last_sent_provider_error_status 跟踪通过 plain-send 降级路径发送的 provider 错误状态文本 - 在 _send_or_update_status_coro 降级路径中，如果内容是 provider 错误回复，记录该文本 - 在 _sanitize_gateway_final_response 中，如果改写结果与已发送的状态文本相同，返回空字符串 跳过重复发送，并设置 _deduped_provider_error 标志 - 在调用方（final_response 为空时的回退逻辑）中，检查 _deduped_provider_error 标志，如果是 因为 dedup 导致的空结果，不再回退到原始错误文本 新增测试: tests/gateway/test_provider_error_dedup.py (21 tests) --- gateway/run.py | 51 ++++++++- tests/gateway/test_provider_error_dedup.py | 118 +++++++++++++++++++++ 2 files changed, 167 insertions(+), 2 deletions(-) create mode 100644 tests/gateway/test_provider_error_dedup.py 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()