Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions src/backend/adapters/message_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,12 @@ def _check_rate_limit(key: str, max_msgs: Optional[int] = None, window: Optional
return True


# #3603: what a channel member is told when a turn does not answer. A failed
# turn's error is for the operator and never reaches the chat.
CHANNEL_FAILURE_MESSAGE = "Sorry, I encountered an error processing your message. Please try again."
CHANNEL_CANCELLED_MESSAGE = "That request was cancelled before it was answered." # always, never the row's error


class ChannelMessageRouter:
"""Channel-agnostic message dispatcher."""

Expand Down Expand Up @@ -877,11 +883,19 @@ async def _run_agent_task(
logger.info(f"[ROUTER:{channel}] Step 9 - task {result.status}: {error_msg}")
await self._resolve_indicator(adapter, message, success=False)

# Reply with the actual error if available, otherwise generic message
if error_msg and error_msg != "Unknown error":
response_text = error_msg
# #3603: the error is the operator's diagnosis (a missing model
# key, an exit code, where to fix a credential) and a channel is
# read by its members — every member, in a group. They get a
# plain line; the detail stays on the execution row and in the
# log line above (the public-chat twin is #3478). A cancelled
# turn still says so (#679), in the fixed line: on a pull-pilot
# agent a terminated turn keeps the failure body's content and
# the pull sink stores it as the cancelled row's error, so that
# error can be the operator's too.
if result.status == "cancelled":
response_text = CHANNEL_CANCELLED_MESSAGE
else:
response_text = "Sorry, I encountered an error processing your message."
response_text = CHANNEL_FAILURE_MESSAGE

await adapter.send_response(
message.channel_id,
Expand Down Expand Up @@ -917,7 +931,7 @@ async def _run_agent_task(
await adapter.send_response(
message.channel_id,
ChannelResponse(
text="Sorry, I encountered an error processing your message. Please try again.",
text=CHANNEL_FAILURE_MESSAGE,
metadata={"bot_token": bot_token, "agent_name": agent_name}
),
thread_id=message.thread_id,
Expand Down
11 changes: 11 additions & 0 deletions tests/registry.json
Original file line number Diff line number Diff line change
Expand Up @@ -6237,6 +6237,17 @@
],
"description": "recreate_container_with_updated_config re-bakes AGENT_NAME and the trinity.agent-name label from the live name beside TRINITY_AGENT_AUTH_TOKEN (driven through the real recreate, red before the fix on the replayed old name); check_agent_name_env_matches flags a present-and-different env or label, matches absent values and unreadable attrs; start_agent_internal recreates a stuck renamed agent on COLD start only and leaves never-renamed and running agents alone."
},
{
"file": "unit/test_3603_channel_failure_reply.py",
"feature": "#3603",
"added": "2026-10-20",
"categories": [
"backend",
"channels",
"unit"
],
"description": "A failed channel turn sends the fixed member-facing line, never the execution's error (auth failure, exit code, timeout, capacity, rate limit), in a group and one-to-one, through the real ChannelMessageRouter._run_agent_task; the error stays in the router's log line; a cancelled turn always sends the fixed cancelled line (#679), never the row's error, including a pull-pilot terminated turn whose cancelled row carries the operator's error; a raised dispatch sends the same line; a successful turn is untouched. Mutation: restoring the error forwarding turns 14 red; restoring the cancel-reason forwarding turns 8 red."
},
{
"file": "unit/test_2891_randomly_guard.py",
"feature": "#2891",
Expand Down
151 changes: 151 additions & 0 deletions tests/unit/test_3603_channel_failure_reply.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
"""#3603 — a failed channel turn tells the chat so in plain words, never the
operator's error.

A channel (Telegram, Slack, WhatsApp) is read by its members — every member in
a group. The router used to post the execution's own `error` into the chat:
"Authentication failure: Not logged in · Please run /login. Check subscription
token or API key configuration." Now a failed turn sends a fixed member-facing
line, and the error stays on the execution row and in the router's log line for
the operator. A cancelled turn still says it was cancelled (#679), in a fixed
line: on pull-pilot agents a cancelled row's error can be the operator's error. The
public-chat twin is #3478.

Driven through `ChannelMessageRouter._run_agent_task` with the dispatch stubbed
where its lazy import finds it (the `test_3274_channel_quote` harness).
"""
import logging
import sys
from pathlib import Path
from types import SimpleNamespace
from unittest.mock import AsyncMock, MagicMock

import pytest

_BACKEND = Path(__file__).resolve().parents[2] / "src" / "backend"
if str(_BACKEND) not in sys.path:
sys.path.insert(0, str(_BACKEND))

pytestmark = pytest.mark.unit

OPERATOR_ERRORS = [
"Authentication failure: Not logged in · Please run /login. Check subscription "
"token or API key configuration.",
"Task execution failed (exit code 1): No authentication configured. Set "
"ANTHROPIC_API_KEY or assign a subscription token.",
"Task execution timed out after 900 seconds",
"Agent at capacity (3/3 parallel tasks running)",
"HTTP error: rate_limit_exceeded",
]


def _harness(monkeypatch, result):
import adapters.message_router as mr
import services.task_execution_service as tes

monkeypatch.setattr(tes, "dispatch_and_await_terminal", AsyncMock(return_value=result))
monkeypatch.setattr(mr, "get_task_execution_service", lambda: MagicMock())
monkeypatch.setattr(mr, "build_public_channel_caller_prompt", lambda *a, **kw: None)
monkeypatch.setattr(mr, "build_voice_capability_prompt", lambda *a, **kw: None)
monkeypatch.setattr(mr, "_get_channel_allowed_tools", lambda: ["WebSearch"])
monkeypatch.setattr(mr.db, "get_public_channel_model", lambda agent: None)
channel = MagicMock()
channel.get_source_identifier.return_value = "telegram:9001:424242"
channel.send_response = AsyncMock()
channel.indicate_done = AsyncMock()
router = mr.ChannelMessageRouter.__new__(mr.ChannelMessageRouter)
return mr, channel, router


def _message():
return SimpleNamespace(text="@bot what did we just discuss?", channel_id="-100", thread_id=None,
files=[], sender_id="5",
metadata={"raw_message": {"from": {"id": 5, "first_name": "Ada"}},
"bot_id": "77", "chat_title": "Ops", "username": "ada"})


async def _run(router, channel, *, is_group=True):
return await router._run_agent_task(
channel, _message(), "finance", "bot-token", "telegram", is_group,
container=None, upload_dir=None, context_prompt="[prompt]",
verified_email=None, image_data=[])


def _sent(channel):
channel.send_response.assert_awaited_once()
return channel.send_response.await_args.args[1].text


@pytest.mark.asyncio
@pytest.mark.parametrize("is_group", [True, False], ids=["group", "one-to-one"])
@pytest.mark.parametrize("error", OPERATOR_ERRORS)
async def test_a_failed_turn_sends_the_plain_line_not_the_error(monkeypatch, error, is_group):
mr, channel, router = _harness(monkeypatch, SimpleNamespace(status="failed", error=error, response=""))
assert await _run(router, channel, is_group=is_group) is None
text = _sent(channel)
assert text == mr.CHANNEL_FAILURE_MESSAGE
assert error not in text


@pytest.mark.asyncio
@pytest.mark.parametrize("error", [None, "", "Unknown error"])
async def test_a_failed_turn_with_no_error_sends_the_same_line(monkeypatch, error):
mr, channel, router = _harness(monkeypatch, SimpleNamespace(status="failed", error=error, response=""))
await _run(router, channel)
assert _sent(channel) == mr.CHANNEL_FAILURE_MESSAGE


@pytest.mark.asyncio
async def test_the_error_stays_with_the_operator(monkeypatch, caplog):
error = OPERATOR_ERRORS[0]
_, channel, router = _harness(monkeypatch, SimpleNamespace(status="failed", error=error, response=""))
with caplog.at_level(logging.INFO, logger="adapters.message_router"):
await _run(router, channel)
assert any(error in r.getMessage() for r in caplog.records)


@pytest.mark.asyncio
@pytest.mark.parametrize("error", OPERATOR_ERRORS)
async def test_a_cancelled_pull_pilot_turn_never_sends_the_operator_error(monkeypatch, error):
"""A pull-pilot turn terminated mid-flight keeps the failure body's content
(`pull_worker.py` relabels it `cancelled` but leaves `content`), and the
pull sink stores that content as the cancelled row's `error`. So a
cancelled row's error can be the operator's error; the chat must get the
fixed cancelled line instead."""
mr, channel, router = _harness(monkeypatch, SimpleNamespace(status="cancelled", error=error, response=""))
assert await _run(router, channel) is None
text = _sent(channel)
assert text == mr.CHANNEL_CANCELLED_MESSAGE
assert error not in text


@pytest.mark.asyncio
@pytest.mark.parametrize("error", [None, "", "Execution cancelled by user", "Cancelled by user while queued",
"[agent_error] caller went away"])
async def test_a_cancelled_turn_always_says_cancelled_in_the_fixed_line(monkeypatch, error):
"""#679: a cancelled turn still says it was cancelled, in one fixed line,
whatever the row's error holds."""
mr, channel, router = _harness(monkeypatch, SimpleNamespace(status="cancelled", error=error, response=""))
await _run(router, channel)
assert _sent(channel) == mr.CHANNEL_CANCELLED_MESSAGE
assert "cancel" in mr.CHANNEL_CANCELLED_MESSAGE.lower()


@pytest.mark.asyncio
async def test_a_dispatch_that_raises_sends_the_same_line(monkeypatch):
"""The exception branch and the failed-row branch say the same thing."""
import adapters.message_router as mr
import services.task_execution_service as tes

_, channel, router = _harness(monkeypatch, None)
monkeypatch.setattr(tes, "dispatch_and_await_terminal",
AsyncMock(side_effect=RuntimeError("upstream exploded: token=abc")))
await _run(router, channel)
assert _sent(channel) == mr.CHANNEL_FAILURE_MESSAGE


@pytest.mark.asyncio
async def test_a_successful_turn_is_untouched(monkeypatch):
result = SimpleNamespace(status="success", error=None, response="Here is the summary.", cost=0.0)
_, channel, router = _harness(monkeypatch, result)
assert await _run(router, channel) is result
channel.send_response.assert_not_awaited()
62 changes: 6 additions & 56 deletions tests/unit/test_slack_timeout_fix.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@

Verifies:
- timeout_seconds=None is passed to TaskExecutionService (uses agent timeout)
- Error messages are forwarded to Slack users as-is
- Generic fallback only for empty/unknown errors
- (error replies: see test_3603_channel_failure_reply.py)

Module: src/backend/adapters/message_router.py
Issue: https://github.com/abilityai/trinity/issues/221
Expand All @@ -13,60 +12,11 @@
import pytest


class TestErrorMessageRouting:
"""Test the error message logic extracted from message_router.py.

The router does:
if error_msg and error_msg != "Unknown error":
response_text = error_msg
else:
response_text = "Sorry, I encountered an error processing your message."
"""

@staticmethod
def get_response_text(error_msg):
"""Reproduce the router's error message logic."""
if error_msg and error_msg != "Unknown error":
return error_msg
else:
return "Sorry, I encountered an error processing your message."

def test_timeout_error_forwarded(self):
"""Timeout error from TaskExecutionService is sent to Slack user."""
msg = "Task execution timed out after 60 seconds"
assert self.get_response_text(msg) == msg

def test_timeout_900s_forwarded(self):
"""Agent-configured timeout (900s) error is forwarded."""
msg = "Task execution timed out after 900 seconds"
assert self.get_response_text(msg) == msg

def test_capacity_error_forwarded(self):
"""Capacity error is sent to Slack user."""
msg = "Agent at capacity (3/3 parallel tasks running)"
assert self.get_response_text(msg) == msg

def test_auth_error_forwarded(self):
"""Auth error from agent is sent to Slack user."""
msg = "Task execution failed (exit code 1): No authentication configured. Set ANTHROPIC_API_KEY or assign a subscription token."
assert self.get_response_text(msg) == msg

def test_billing_error_forwarded(self):
"""Billing/rate-limit error is sent to Slack user."""
msg = "HTTP error: rate_limit_exceeded"
assert self.get_response_text(msg) == msg

def test_unknown_error_gets_generic(self):
"""'Unknown error' gets the generic fallback message."""
assert self.get_response_text("Unknown error") == "Sorry, I encountered an error processing your message."

def test_none_error_gets_generic(self):
"""None error gets the generic fallback message."""
assert self.get_response_text(None) == "Sorry, I encountered an error processing your message."

def test_empty_error_gets_generic(self):
"""Empty string error gets the generic fallback message."""
assert self.get_response_text("") == "Sorry, I encountered an error processing your message."
# #3603 retired #221's error forwarding: a channel member is no longer sent the
# execution's own error (it is the operator's diagnosis, and in a group every
# member reads it). The behaviour is executed against the real router in
# test_3603_channel_failure_reply.py; the copy of the old logic that stood here
# could not fail.


class TestTimeoutParameter:
Expand Down
Loading