Files
EvoScientist/tests/test_channel_notification_origin.py
T
Ziheng Zhang 2364e6b130 fix(cli): forward async-notifier replies back to originating channel (#244)
* fix(cli): forward async-notifier replies back to originating channel

When PR #214's auto-notifier fires a synthetic agent turn after a
channel-originated conversation, the synthesized response only rendered
to the local CLI/TUI — the channel user (iMessage etc.) saw nothing
and had to manually re-prompt to find out what happened.

Adds a per-thread channel-origin registry in cli/channel.py and wires
the three notifier paths (Rich CLI / TUI / serve) to publish the final
response back via bus.publish_outbound when the originating thread was
started by a channel turn. Publish is fire-and-forget (scheduled on the
bus loop + done-callback for failure logging) so the notifier turn
doesn't block on the asyncio / textual event loop.

The registry is cleared on /new and /resume rotation so stale entries
don't accumulate.

* fix(cli): address review feedback on channel-origin forwarding

Follow-up to the review on #244 (din0s, X-iZhang):

- Guard the /resume origin cleanup on a real thread change in Rich CLI
  and TUI (serve mode already did via thread_changed). Resuming the
  already-active thread no longer wipes its still-live origin, which
  would otherwise silently drop a later async-notifier forward — the
  exact gap this PR closes.
- Re-bind the now-current thread to its channel after a channel-issued
  /new or /resume slash command (which rotates the thread inside the
  dispatch), so notifier turns on the rotated thread still forward.
- Guard the publish done-callback against a cancelled future, whose
  .exception() raises CancelledError (rather than returning it) on
  bus-loop teardown, so the intended warning still logs.
- Mirror the normal reply path's manager.record_message(channel, "sent")
  for forwarded notifications so per-channel stats stay accurate.
- Print the closing "[channel: Replied to ...]" line in all three
  notifier paths (Rich CLI / TUI / serve) when a forward actually
  happened, so the forwarded block reads as terminated on screen.

Adds test_publish_records_sent_metric. ruff clean; notification-origin
suite (10) + related channel/CLI/serve suites (728) pass.

* fix(cli): store sender information separately from chat_id in channel origin

---------

Co-authored-by: X-iZhang <zacharyzhang2022@gmail.com>
2026-05-31 14:43:42 +01:00

273 lines
9.3 KiB
Python

"""Tests for the per-thread channel-origin registry used by the async-notifier
push-back path."""
from __future__ import annotations
import asyncio
import threading
from unittest.mock import MagicMock
import pytest
from EvoScientist.cli import channel as channel_cli
from EvoScientist.cli.channel import ChannelMessage
@pytest.fixture(autouse=True)
def _restore_channel_globals():
"""Restore bus globals + the origin registry between tests."""
original = {
"_manager": channel_cli._manager,
"_bus_loop": channel_cli._bus_loop,
"_bus_thread": channel_cli._bus_thread,
}
with channel_cli._thread_channel_origins_lock:
original_origins = dict(channel_cli._thread_channel_origins)
channel_cli._thread_channel_origins.clear()
yield
channel_cli._manager = original["_manager"]
channel_cli._bus_loop = original["_bus_loop"]
channel_cli._bus_thread = original["_bus_thread"]
with channel_cli._thread_channel_origins_lock:
channel_cli._thread_channel_origins.clear()
channel_cli._thread_channel_origins.update(original_origins)
def _make_msg(
*,
channel_type: str = "imessage",
chat_id: str = "+15551234567",
metadata: dict | None = None,
) -> ChannelMessage:
return ChannelMessage(
msg_id="msg-1",
content="hi",
sender="alice",
channel_type=channel_type,
metadata=metadata if metadata is not None else {"foo": "bar"},
chat_id=chat_id,
)
def _install_fake_bus(
monkeypatch,
) -> tuple[asyncio.AbstractEventLoop, MagicMock, threading.Thread]:
"""Spin up a real asyncio loop on a background thread + a stub bus.
Returns (loop, publish_outbound_mock, thread). The caller is responsible
for stopping the loop at the end of the test.
"""
loop = asyncio.new_event_loop()
ready = threading.Event()
def _runner():
asyncio.set_event_loop(loop)
ready.set()
loop.run_forever()
thread = threading.Thread(target=_runner, daemon=True)
thread.start()
ready.wait(timeout=2)
publish_outbound = MagicMock()
async def _publish_outbound(msg):
publish_outbound(msg)
bus = MagicMock()
bus.publish_outbound = _publish_outbound
manager = MagicMock()
manager.bus = bus
monkeypatch.setattr(channel_cli, "_bus_loop", loop)
monkeypatch.setattr(channel_cli, "_manager", manager)
return loop, publish_outbound, thread
def _stop_loop(loop: asyncio.AbstractEventLoop, thread: threading.Thread) -> None:
loop.call_soon_threadsafe(loop.stop)
thread.join(timeout=2)
loop.close()
def _wait_for_publish(
mock: MagicMock, *, expected: int = 1, timeout: float = 2.0
) -> None:
"""Block until ``mock`` has been called ``expected`` times (or timeout)."""
import time
deadline = time.time() + timeout
while mock.call_count < expected and time.time() < deadline:
time.sleep(0.01)
def test_remember_and_publish_roundtrip(monkeypatch):
loop, publish_outbound, thread = _install_fake_bus(monkeypatch)
try:
msg = _make_msg(metadata={"thread_root": "root-1"})
channel_cli.remember_channel_origin("tid-1", msg)
assert channel_cli.publish_to_channel_origin("tid-1", "hello") is True
_wait_for_publish(publish_outbound)
publish_outbound.assert_called_once()
sent = publish_outbound.call_args.args[0]
assert sent.channel == "imessage"
assert sent.chat_id == "+15551234567"
assert sent.content == "hello"
assert sent.metadata == {"thread_root": "root-1"}
assert sent.reply_to is None
finally:
_stop_loop(loop, thread)
def test_origin_remembers_sender_distinct_from_chat_id():
"""The origin stores the human-readable sender separately from chat_id, so
the notifier closer can show the same handle a normal turn shows.
Regression: iMessage exposes an internal chat id (e.g. "1") as chat_id
while the handle (phone number) lives on sender. The closer must display
sender, not chat_id, or the user sees a confusing "Replied to 1".
"""
channel_cli.remember_channel_origin(
"tid-sender",
_make_msg(chat_id="1"), # iMessage-style internal chat id
)
origin = channel_cli.get_channel_origin("tid-sender")
assert origin is not None
assert origin.chat_id == "1" # routing id preserved
assert origin.sender == "alice" # human handle preserved for display
def test_publish_records_sent_metric(monkeypatch):
"""A forwarded notification records a "sent" metric, mirroring the normal
channel-reply path (``manager.record_message``)."""
loop, _publish_outbound, thread = _install_fake_bus(monkeypatch)
try:
channel_cli.remember_channel_origin(
"tid-metric", _make_msg(channel_type="telegram")
)
assert channel_cli.publish_to_channel_origin("tid-metric", "done") is True
# record_message runs right after publish_outbound inside the same
# coroutine; sync on it before asserting.
_wait_for_publish(channel_cli._manager.record_message)
channel_cli._manager.record_message.assert_called_once_with("telegram", "sent")
finally:
_stop_loop(loop, thread)
def test_publish_returns_false_without_origin(monkeypatch):
loop, publish_outbound, thread = _install_fake_bus(monkeypatch)
try:
assert channel_cli.publish_to_channel_origin("unknown-tid", "hi") is False
publish_outbound.assert_not_called()
finally:
_stop_loop(loop, thread)
def test_publish_returns_false_when_bus_down(monkeypatch):
monkeypatch.setattr(channel_cli, "_bus_loop", None)
monkeypatch.setattr(channel_cli, "_manager", None)
channel_cli.remember_channel_origin("tid-2", _make_msg())
assert channel_cli.publish_to_channel_origin("tid-2", "hi") is False
def test_publish_returns_false_for_empty_content(monkeypatch):
loop, publish_outbound, thread = _install_fake_bus(monkeypatch)
try:
channel_cli.remember_channel_origin("tid-3", _make_msg())
assert channel_cli.publish_to_channel_origin("tid-3", "") is False
assert channel_cli.publish_to_channel_origin("tid-3", " \n ") is False
publish_outbound.assert_not_called()
finally:
_stop_loop(loop, thread)
def test_forget_origin(monkeypatch):
loop, publish_outbound, thread = _install_fake_bus(monkeypatch)
try:
channel_cli.remember_channel_origin("tid-4", _make_msg())
channel_cli.forget_channel_origin("tid-4")
assert channel_cli.publish_to_channel_origin("tid-4", "hi") is False
publish_outbound.assert_not_called()
finally:
_stop_loop(loop, thread)
def test_remember_overwrites_same_thread(monkeypatch):
loop, publish_outbound, thread = _install_fake_bus(monkeypatch)
try:
channel_cli.remember_channel_origin(
"tid-5", _make_msg(channel_type="telegram", chat_id="111")
)
channel_cli.remember_channel_origin(
"tid-5", _make_msg(channel_type="imessage", chat_id="222")
)
assert channel_cli.publish_to_channel_origin("tid-5", "hi") is True
_wait_for_publish(publish_outbound)
sent = publish_outbound.call_args.args[0]
assert sent.channel == "imessage"
assert sent.chat_id == "222"
finally:
_stop_loop(loop, thread)
def test_publish_swallows_bus_error(monkeypatch, caplog):
loop = asyncio.new_event_loop()
ready = threading.Event()
def _runner():
asyncio.set_event_loop(loop)
ready.set()
loop.run_forever()
thread = threading.Thread(target=_runner, daemon=True)
thread.start()
ready.wait(timeout=2)
async def _boom(_msg):
raise RuntimeError("bus dead")
bus = MagicMock()
bus.publish_outbound = _boom
manager = MagicMock()
manager.bus = bus
monkeypatch.setattr(channel_cli, "_bus_loop", loop)
monkeypatch.setattr(channel_cli, "_manager", manager)
channel_cli.remember_channel_origin("tid-6", _make_msg())
try:
with caplog.at_level("WARNING", logger="EvoScientist.cli.channel"):
# The publish is scheduled (returns True); the coroutine raises
# asynchronously and the done-callback logs the failure.
assert channel_cli.publish_to_channel_origin("tid-6", "hi") is True
# Give the bus thread a moment to run the coroutine + callback.
import time
deadline = time.time() + 2.0
while (
not any(
"Async notification publish" in r.message for r in caplog.records
)
and time.time() < deadline
):
time.sleep(0.01)
assert any("Async notification publish" in r.message for r in caplog.records)
finally:
_stop_loop(loop, thread)
def test_remember_with_falsy_thread_id_noop(monkeypatch):
"""Defensive: a falsy thread_id must not pollute the registry."""
msg = _make_msg()
channel_cli.remember_channel_origin(None, msg)
channel_cli.remember_channel_origin("", msg)
with channel_cli._thread_channel_origins_lock:
assert channel_cli._thread_channel_origins == {}
def test_get_channel_origin_returns_none_for_unknown():
assert channel_cli.get_channel_origin("never-registered") is None
assert channel_cli.get_channel_origin(None) is None