fix(restart): deliver completion after channel reconnects (#4931)
This commit is contained in:
@@ -2651,8 +2651,8 @@ async def test_start_all_creates_dispatch_task():
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_notify_restart_done_enqueues_outbound_message():
|
||||
"""Restart notice should schedule send_with_retry for target channel."""
|
||||
async def test_notify_restart_done_waits_until_channel_starts():
|
||||
"""Restart notice should not be sent before the target channel starts."""
|
||||
fake_config = SimpleNamespace(
|
||||
channels=ChannelsConfig(),
|
||||
providers=SimpleNamespace(groq=SimpleNamespace(api_key="")),
|
||||
@@ -2661,18 +2661,61 @@ async def test_notify_restart_done_enqueues_outbound_message():
|
||||
mgr = ChannelManager.__new__(ChannelManager)
|
||||
mgr.config = fake_config
|
||||
mgr.bus = MessageBus()
|
||||
mgr.channels = {"feishu": _StartableChannel(fake_config, mgr.bus)}
|
||||
channel = _StartableChannel(fake_config, mgr.bus)
|
||||
mgr.channels = {"feishu": channel}
|
||||
mgr._dispatch_task = None
|
||||
mgr._send_with_retry = AsyncMock()
|
||||
|
||||
notice = RestartNotice(channel="feishu", chat_id="oc_123", started_at_raw="100.0")
|
||||
with patch("nanobot.channels.manager.consume_restart_notice_from_env", return_value=notice):
|
||||
mgr._notify_restart_done_if_needed()
|
||||
task = mgr._notify_restart_done_if_needed()
|
||||
|
||||
await asyncio.sleep(0)
|
||||
mgr._send_with_retry.assert_not_awaited()
|
||||
|
||||
channel._running = True
|
||||
assert task is not None
|
||||
await asyncio.wait_for(task, timeout=1.0)
|
||||
|
||||
mgr._send_with_retry.assert_awaited_once()
|
||||
sent_channel, sent_msg = mgr._send_with_retry.await_args.args
|
||||
assert sent_channel is mgr.channels["feishu"]
|
||||
assert sent_channel is channel
|
||||
assert sent_msg.channel == "feishu"
|
||||
assert sent_msg.chat_id == "oc_123"
|
||||
assert sent_msg.content.startswith("Restart completed")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_restart_notice_retries_until_running_channel_accepts_delivery():
|
||||
"""A running flag must not make an early transport failure final."""
|
||||
|
||||
class _EventuallyDeliverableChannel(_StartableChannel):
|
||||
def __init__(self, config, bus):
|
||||
super().__init__(config, bus)
|
||||
self.attempts = 0
|
||||
self.sent: OutboundMessage | None = None
|
||||
|
||||
async def send(self, msg: OutboundMessage) -> None:
|
||||
self.attempts += 1
|
||||
if self.attempts == 1:
|
||||
raise RuntimeError("transport not ready")
|
||||
self.sent = msg
|
||||
|
||||
fake_config = SimpleNamespace(
|
||||
channels=ChannelsConfig(send_max_retries=1),
|
||||
providers=SimpleNamespace(groq=SimpleNamespace(api_key="")),
|
||||
)
|
||||
mgr = ChannelManager.__new__(ChannelManager)
|
||||
mgr.config = fake_config
|
||||
mgr.bus = MessageBus()
|
||||
channel = _EventuallyDeliverableChannel(fake_config, mgr.bus)
|
||||
channel._running = True
|
||||
mgr.channels = {"discord": channel}
|
||||
|
||||
notice = RestartNotice(channel="discord", chat_id="123", started_at_raw="")
|
||||
with patch("nanobot.channels.manager._SEND_RETRY_DELAYS", (0,)):
|
||||
await mgr._send_restart_notice_when_started(notice, timeout_s=0.1, poll_s=0.01)
|
||||
|
||||
assert channel.attempts == 2
|
||||
assert channel.sent is not None
|
||||
assert channel.sent.content == "Restart completed."
|
||||
|
||||
@@ -2,6 +2,7 @@ import asyncio
|
||||
import zipfile
|
||||
from io import BytesIO
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
@@ -17,6 +18,7 @@ if not DINGTALK_AVAILABLE:
|
||||
pytest.skip("DingTalk dependencies not installed (dingtalk-stream)", allow_module_level=True)
|
||||
|
||||
import nanobot.channels.dingtalk as dingtalk_module
|
||||
from nanobot.bus.events import OutboundMessage
|
||||
from nanobot.bus.queue import MessageBus
|
||||
from nanobot.channels.dingtalk import DingTalkChannel, DingTalkConfig, NanobotDingTalkHandler
|
||||
|
||||
@@ -864,6 +866,35 @@ async def test_send_batch_message_returns_false_on_api_error() -> None:
|
||||
assert result is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_raises_when_access_token_is_unavailable(monkeypatch) -> None:
|
||||
channel = DingTalkChannel(
|
||||
DingTalkConfig(client_id="app", client_secret="secret", allow_from=["*"]),
|
||||
MessageBus(),
|
||||
)
|
||||
monkeypatch.setattr(channel, "_get_access_token", AsyncMock(return_value=None))
|
||||
|
||||
with pytest.raises(RuntimeError, match="access token unavailable"):
|
||||
await channel.send(
|
||||
OutboundMessage(channel="dingtalk", chat_id="user123", content="hello")
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_raises_when_text_is_not_delivered(monkeypatch) -> None:
|
||||
channel = DingTalkChannel(
|
||||
DingTalkConfig(client_id="app", client_secret="secret", allow_from=["*"]),
|
||||
MessageBus(),
|
||||
)
|
||||
monkeypatch.setattr(channel, "_get_access_token", AsyncMock(return_value="token"))
|
||||
monkeypatch.setattr(channel, "_send_markdown_text", AsyncMock(return_value=False))
|
||||
|
||||
with pytest.raises(RuntimeError, match="text message was not delivered"):
|
||||
await channel.send(
|
||||
OutboundMessage(channel="dingtalk", chat_id="user123", content="hello")
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_media_ref_short_circuits_on_transport_error() -> None:
|
||||
"""When the first send fails with a transport error, _send_media_ref must
|
||||
|
||||
@@ -659,18 +659,19 @@ async def test_on_message_marks_failed_attachment_download(tmp_path, monkeypatch
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_warns_when_client_not_ready() -> None:
|
||||
# Sending without a running/ready client should be a safe no-op.
|
||||
async def test_send_raises_when_client_not_ready() -> None:
|
||||
# The manager must be able to retry while Discord is still connecting.
|
||||
channel = DiscordChannel(DiscordConfig(enabled=True, allow_from=["*"]), MessageBus())
|
||||
|
||||
await channel.send(OutboundMessage(channel="discord", chat_id="123", content="hello"))
|
||||
with pytest.raises(RuntimeError, match="client is not ready"):
|
||||
await channel.send(OutboundMessage(channel="discord", chat_id="123", content="hello"))
|
||||
|
||||
assert channel._typing_tasks == {}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_skips_when_channel_not_cached() -> None:
|
||||
# Outbound sends should be skipped when the destination channel is not resolvable.
|
||||
async def test_send_raises_when_channel_cannot_be_resolved() -> None:
|
||||
# The manager must be able to retry transient channel-resolution failures.
|
||||
owner = DiscordChannel(DiscordConfig(enabled=True, allow_from=["*"]), MessageBus())
|
||||
client = DiscordBotClient(owner, intents=discord.Intents.none())
|
||||
fetch_calls: list[int] = []
|
||||
@@ -681,7 +682,10 @@ async def test_send_skips_when_channel_not_cached() -> None:
|
||||
|
||||
client.fetch_channel = fetch_channel # type: ignore[method-assign]
|
||||
|
||||
await client.send_outbound(OutboundMessage(channel="discord", chat_id="123", content="hello"))
|
||||
with pytest.raises(RuntimeError, match="not found"):
|
||||
await client.send_outbound(
|
||||
OutboundMessage(channel="discord", chat_id="123", content="hello")
|
||||
)
|
||||
|
||||
assert client.get_channel(123) is None
|
||||
assert fetch_calls == [123]
|
||||
|
||||
@@ -266,8 +266,9 @@ async def test_send_uses_expected_feishu_msg_type_for_uploaded_files(
|
||||
|
||||
send_calls: list[tuple[str, str, str, str]] = []
|
||||
|
||||
def _record_send(receive_id_type: str, receive_id: str, msg_type: str, content: str) -> None:
|
||||
def _record_send(receive_id_type: str, receive_id: str, msg_type: str, content: str) -> str:
|
||||
send_calls.append((receive_id_type, receive_id, msg_type, content))
|
||||
return "om_test"
|
||||
|
||||
with patch.object(channel, "_upload_file_sync", return_value="file-key"), patch.object(
|
||||
channel, "_send_message_sync", side_effect=_record_send
|
||||
@@ -398,6 +399,22 @@ async def test_send_fallback_to_create_when_reply_fails() -> None:
|
||||
channel._client.im.v1.message.create.assert_called_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_raises_when_create_api_does_not_deliver() -> None:
|
||||
channel = _make_feishu_channel()
|
||||
|
||||
with patch.object(channel, "_send_message_sync", return_value=None):
|
||||
with pytest.raises(RuntimeError, match="message was not delivered"):
|
||||
await channel.send(
|
||||
OutboundMessage(
|
||||
channel="feishu",
|
||||
chat_id="oc_abc",
|
||||
content="hello",
|
||||
metadata={},
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
def test_send_message_sync_falls_back_to_text_for_interactive_error() -> None:
|
||||
channel = _make_feishu_channel()
|
||||
|
||||
|
||||
@@ -1820,6 +1820,28 @@ async def test_send_room_content_returns_room_send_response():
|
||||
assert result is client.room_send_response
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_raises_when_room_send_returns_error(monkeypatch) -> None:
|
||||
class _FakeRoomSendError:
|
||||
def __str__(self) -> str:
|
||||
return "temporary homeserver failure"
|
||||
|
||||
client = _FakeAsyncClient("", "", "", None)
|
||||
client.room_send_response = _FakeRoomSendError()
|
||||
channel = MatrixChannel(_make_config(), MessageBus())
|
||||
channel.client = client
|
||||
monkeypatch.setattr(matrix_module, "RoomSendError", _FakeRoomSendError)
|
||||
|
||||
with pytest.raises(RuntimeError, match="temporary homeserver failure"):
|
||||
await channel.send(
|
||||
OutboundMessage(
|
||||
channel="matrix",
|
||||
chat_id="!room:matrix.org",
|
||||
content="hello",
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_delta_creates_stream_buffer_and_sends_initial_message() -> None:
|
||||
channel = MatrixChannel(_make_config(), MessageBus())
|
||||
|
||||
@@ -2,6 +2,7 @@ import asyncio
|
||||
|
||||
import pytest
|
||||
|
||||
from nanobot.bus.events import OutboundMessage
|
||||
from nanobot.bus.queue import MessageBus
|
||||
from nanobot.channels.napcat import NapcatChannel, NapcatConfig
|
||||
|
||||
@@ -52,6 +53,16 @@ def _channel(config: NapcatConfig | None = None) -> NapcatChannel:
|
||||
return NapcatChannel(config or NapcatConfig(allow_from=["*"]), MessageBus())
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_raises_while_websocket_is_not_connected() -> None:
|
||||
channel = _channel()
|
||||
|
||||
with pytest.raises(RuntimeError, match="not connected"):
|
||||
await channel.send(
|
||||
OutboundMessage(channel="napcat", chat_id="private:123", content="hello")
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_group_message_requires_mention_by_default() -> None:
|
||||
channel = _channel(NapcatConfig(allow_from=["user1"], group_policy="mention"))
|
||||
|
||||
@@ -113,17 +113,18 @@ def test_guess_send_file_type_by_mime() -> None:
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_exception_caught_not_raised() -> None:
|
||||
"""Exceptions inside send() must not propagate."""
|
||||
async def test_send_exception_propagates_for_manager_retry() -> None:
|
||||
"""Delivery failures must propagate to the channel manager."""
|
||||
channel = QQChannel(QQConfig(app_id="app", secret="secret", allow_from=["*"]), MessageBus())
|
||||
channel._client = _FakeClient()
|
||||
|
||||
with patch.object(
|
||||
channel, "_send_text_only", new_callable=AsyncMock, side_effect=RuntimeError("boom")
|
||||
) as send_text:
|
||||
await channel.send(
|
||||
OutboundMessage(channel="qq", chat_id="user1", content="hello")
|
||||
)
|
||||
with pytest.raises(RuntimeError, match="boom"):
|
||||
await channel.send(
|
||||
OutboundMessage(channel="qq", chat_id="user1", content="hello")
|
||||
)
|
||||
send_text.assert_awaited_once()
|
||||
|
||||
|
||||
|
||||
@@ -412,8 +412,8 @@ async def test_send_media_file_not_found() -> None:
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_exception_caught_not_raised() -> None:
|
||||
"""Exceptions inside send() must not propagate."""
|
||||
async def test_send_exception_propagates_for_manager_retry() -> None:
|
||||
"""Delivery failures must propagate to the channel manager."""
|
||||
channel = WecomChannel(WecomConfig(bot_id="b", secret="s", allow_from=["*"]), MessageBus())
|
||||
client = _FakeWeComClient()
|
||||
channel._client = client
|
||||
@@ -423,9 +423,10 @@ async def test_send_exception_caught_not_raised() -> None:
|
||||
# Make reply_stream raise
|
||||
client.reply_stream.side_effect = RuntimeError("boom")
|
||||
|
||||
await channel.send(
|
||||
OutboundMessage(channel="wecom", chat_id="chat1", content="fail test")
|
||||
)
|
||||
with pytest.raises(RuntimeError, match="boom"):
|
||||
await channel.send(
|
||||
OutboundMessage(channel="wecom", chat_id="chat1", content="fail test")
|
||||
)
|
||||
client.reply_stream.assert_called_once()
|
||||
|
||||
|
||||
|
||||
@@ -56,6 +56,23 @@ def test_restart_notice_preserves_metadata_across_env(monkeypatch):
|
||||
assert "NANOBOT_RESTART_NOTIFY_METADATA" not in os.environ
|
||||
|
||||
|
||||
def test_restart_notice_drops_process_local_webui_turn_metadata(monkeypatch):
|
||||
monkeypatch.delenv("NANOBOT_RESTART_NOTIFY_METADATA", raising=False)
|
||||
|
||||
set_restart_notice_to_env(
|
||||
channel="websocket",
|
||||
chat_id="chat-1",
|
||||
metadata={
|
||||
"webui_turn_id": "turn-from-old-process",
|
||||
"slack": {"thread_ts": "1700.42"},
|
||||
},
|
||||
)
|
||||
|
||||
notice = consume_restart_notice_from_env()
|
||||
assert notice is not None
|
||||
assert notice.metadata == {"slack": {"thread_ts": "1700.42"}}
|
||||
|
||||
|
||||
def test_restart_notice_clears_stale_metadata(monkeypatch):
|
||||
monkeypatch.setenv("NANOBOT_RESTART_NOTIFY_METADATA", '{"stale": true}')
|
||||
set_restart_notice_to_env(channel="cli", chat_id="direct")
|
||||
|
||||
Reference in New Issue
Block a user