Files
EvoScientist-Multi/tests/test_cli_channel_bridge.py
dinos db1abce8d8 refactor: extract a shared HITL/ask_user interaction engine (#342)
* chore: add pytest-asyncio in auto mode

* test: migrate channel and stream tests to native async

Convert run_async() wrapper tests to plain 'async def test_*' under
pytest-asyncio auto mode. collect_events() in stream_v3_fakes becomes a
coroutine awaited at every call site.

* test: migrate command and model/middleware tests to native async

Convert run_async() wrappers (import, alias, and fixture forms) to plain
'async def test_*'. Multi-call tests merge onto one loop as sequential
awaits; none asserted on loop identity.

* test: migrate TUI, notifier, gateway, and session tests to native async

TUI/notifier/gateway files convert run_async wrappers to plain async
tests. test_sessions.py's unittest.TestCase classes move to
unittest.IsolatedAsyncioTestCase (pytest-asyncio does not await async
methods on plain TestCase; converting blindly would have made ~70 tests
silently vacuous). Its setUpClass keeps a one-shot asyncio.run() since
IsolatedAsyncioTestCase has no async class-level hook. TestLoadingWidget
in test_tui_widgets.py drops its TestCase base for the same reason.

* test: replace direct asyncio.run() calls with native async tests

Convert tests that called asyncio.run() (directly or via a local _run
helper) to plain 'async def test_*'; delete the local helpers.

* test: drop undeclared anyio markers and delete run_async helper

The @pytest.mark.anyio tests relied on anyio being a transitive dep of
httpx; auto-mode pytest-asyncio collects them natively. run_async() and
its fixture are unreferenced after the migration, so remove them —
pytest-asyncio's per-test loop teardown covers the pending-task
cancellation the helper existed for (verified: full suite runs with no
'Event loop is closed' errors or destroyed-task warnings).

* test: add autouse fixture for watcher cleanup

* refactor: remove redundant hasattr calls

* refactor: extract shared HITL/ask_user interaction grammar

Extract prompt/question formatting, the reply grammar (approval letters,
ask_user choice letters + the 'Other' sub-flow, stop-commands), the
ApprovalPolicy (config auto-approve rule + session registry +
session-key derivation), per-flow timeout constants, and the bilingual
feedback strings into channels/interaction.py. Both drivers now point at
the shared functions: this reverses cli/channel.py's imports of consumer
privates and closes the /stop drift at the parsing layer (serve-mode
ask_user now checks stop-commands before parsing an answer, matching the
CLI path).

* refactor: add interaction engine + registry; port InboundConsumer

Introduce InteractionIO (transport adapter Protocol),
PendingReplyRegistry (one asyncio-based reply router per process), and
the engine coroutines resolve_ask_user / resolve_approval in
channels/interaction.py. Port InboundConsumer onto them: a _ConsumerIO
adapter over bus.publish_outbound + the registry, one ApprovalPolicy
replacing the config/session auto-approve checks, and a single
reply-interception point (registry.try_resolve) replacing the parallel
ask_user/HITL pending dicts. _resolve_ask_user and the approval section
of _stream_with_hitl are now thin engine calls.

Behavior unification (serve mode): an unrecognized HITL reply now
declines with the shared 'Unrecognized reply' notice instead of
rejecting-and-refeeding as a fresh turn, and /stop mid-approval cancels
cleanly — both via the shared parser.

* refactor: port CLI channel bridge onto the interaction engine

Replace the ~250-line parallel bodies of channel_ask_user_prompt /
channel_hitl_prompt with thin bridges that run resolve_ask_user /
resolve_approval on the bus loop via
run_coroutine_threadsafe(...).result() (outer = engine per-flow timeout
+ slack, so the engine's own timeout fires first). The 15s send timeout
moves into the _BridgeIO adapter.

Delete the _pending_hitl / _hitl_lock / _hitl_auto_approve module
globals and the _register_hitl_wait / _try_set_hitl_reply /
_pop_hitl_reply helpers, absorbed by one bus-loop PendingReplyRegistry +
one ApprovalPolicy. The bus consumer feeds the registry via try_resolve
ahead of normal enqueue.

* refactor: restore serve-mode refeed for unrecognized HITL replies

Gate-review fix: the engine no longer decides transport policy for
unparseable approval replies. resolve_approval now returns an
ApprovalOutcome carrying unrecognized_reply (raw text) when parsing
fails, sending no feedback itself; recognized reject keeps the sharedi
rejection message.

Consumer driver (serve mode) restores the pre-engine semantics: an
unrecognized reply rejects the pending action, confirms with the
rejection message, and the text is re-dispatched as a NEW agent turn —
_stream_with_hitl returns the captured text and _handle_message starts
the refeed turn only after the current one has released the chat lock
(old fall-through ordering). CLI bridge keeps its old no-refeed path
byte-for-byte: 'Unrecognized reply. Action rejected.' and decline.

Tests: serve refeed pinned end-to-end (prompt → unrecognized text →
rejection feedback → text reaches the stream path as a new turn), CLI
no-refeed pinned (notice sent, nothing enqueued), engine test updated to
assert the outcome struct with no engine-side feedback.

* refactor: polish the interaction engine surface

- English feedback strings (Approved / Rejected / auto-approving)
- drop the consumer's backwards-compatible re-exports and both modules'
  private timeout aliases; callers use the canonical interaction names
- replace byte-for-byte prompt goldens with structural format tests and
  assert feedback via the shared constants instead of string literals
- strip audit/design shorthand (R1/R2/G3, stage numbers) from comments

* fix: propagate pending reply task cancellation

* fix: preserve reply context when refeeding HITL replies

* fix: honor HITL session grants without bus loop

* fix: bound bridge waits by send latency

* fix: handle empty ask_user replies explicitly

* chore: remove stale interaction helpers

* fix: harden interaction engine reply edge cases

Review follow-ups on the interaction engine:
- normalize ask_user choices before .get(): the tool args come from model
  JSON and only presence is validated, so plain-string choices must render
  and parse instead of crashing the turn
- treat only None as an approval timeout, so an empty/media-only reply
  flows through the unrecognized path and serve mode refeeds it with its
  preserved context
- intercept prompt replies before _get_thread_id so a consumed reply
  cannot create an orphan graph thread or touch the sender-session LRU
- close engine coroutines the bridge failed to schedule (no bus loop /
  scheduling error) to avoid never-awaited warnings
- clear pending-response and channel-request state in the bridge test
  fixture

---------

Co-authored-by: Xi Zhang <106144707+X-iZhang@users.noreply.github.com>
2026-07-14 14:59:41 +00:00

427 lines
16 KiB
Python

"""Tests for the CLI channel bridge.
The bridge runs the shared interaction engine on the bus loop via
``run_coroutine_threadsafe`` while the calling thread blocks. Covered here:
* **Ordering** — the reply-interception point sits *ahead* of normal
enqueue, so a reply to a pending prompt is delivered into the engine's
wait and never becomes a fresh agent turn.
* **Bridge round-trip** — ``channel_hitl_prompt`` drives ``resolve_approval``
end-to-end over a real bus loop and returns today's decision payloads.
"""
import asyncio
import threading
import pytest
from EvoScientist.channels import interaction as interaction_mod
from EvoScientist.channels.bus.events import InboundMessage
from EvoScientist.channels.bus.message_bus import MessageBus
from EvoScientist.channels.channel_manager import ChannelManager
from EvoScientist.cli import channel as channel_mod
from EvoScientist.cli.channel import ChannelMessage
from tests.fakes import QueueFakeChannel
def _reset_channel_state():
channel_mod._reply_registry.clear()
channel_mod._approval_policy.clear_sessions()
while not channel_mod._message_queue.empty():
channel_mod._message_queue.get_nowait()
with channel_mod._response_lock:
channel_mod._pending_responses.clear()
with channel_mod._channel_request_lock:
channel_mod._channel_requests.clear()
channel_mod._session_requests.clear()
channel_mod._cancelled_channel_messages.clear()
@pytest.fixture(autouse=True)
def _clean_bridge_state():
_reset_channel_state()
yield
_reset_channel_state()
# ═══════════════════════════════════════════════════════════════════════
# Reply interception sits ahead of normal enqueue
# ═══════════════════════════════════════════════════════════════════════
class TestReplyInterceptionOrdering:
async def test_pending_reply_intercepted_not_enqueued(self):
"""A reply to a pending prompt resolves the wait and is NOT enqueued."""
from EvoScientist.cli.channel import _bus_inbound_consumer, _message_queue
bus = MessageBus()
manager = ChannelManager(bus)
manager.register(QueueFakeChannel())
# A prompt is waiting on fake:chat1 (as the engine's wait_reply would).
reply_fut = asyncio.ensure_future(
channel_mod._reply_registry.wait("fake:chat1", timeout=5.0)
)
await asyncio.sleep(0.01)
consumer = asyncio.create_task(_bus_inbound_consumer(bus, manager))
await bus.publish_inbound(
InboundMessage(
channel="fake",
sender_id="user1",
chat_id="chat1",
content="1",
message_id="m-reply",
)
)
# The reply is delivered into the pending wait...
got = await asyncio.wait_for(reply_fut, timeout=2.0)
assert got == "1"
# ...and did NOT become a queued agent turn.
await asyncio.sleep(0.05)
assert _message_queue.empty()
consumer.cancel()
try:
await consumer
except asyncio.CancelledError:
pass
async def test_message_without_pending_wait_is_enqueued(self):
"""With no pending prompt, a normal message flows to enqueue as before."""
from EvoScientist.cli.channel import _bus_inbound_consumer, _message_queue
bus = MessageBus()
manager = ChannelManager(bus)
manager.register(QueueFakeChannel())
consumer = asyncio.create_task(_bus_inbound_consumer(bus, manager))
await bus.publish_inbound(
InboundMessage(
channel="fake",
sender_id="user1",
chat_id="chat1",
content="hello there",
message_id="m-normal",
)
)
queued = None
for _ in range(40):
with _message_queue.mutex:
queued = _message_queue.queue[0] if _message_queue.queue else None
if queued is not None:
break
await asyncio.sleep(0.02)
assert queued is not None
assert queued.content == "hello there"
consumer.cancel()
try:
await consumer
except asyncio.CancelledError:
pass
# Drain the pending waiter created by _handle_bus_message.
channel_mod._pop_channel_response(queued.msg_id, cancel_pending=True)
# ═══════════════════════════════════════════════════════════════════════
# Bridge timeout budgeting and cancellation
# ═══════════════════════════════════════════════════════════════════════
class TestBridgeTimeouts:
def test_ask_user_outer_timeout_includes_waits_and_sends(self):
question_count = 2
per_question_worst_case = (
channel_mod.ASK_USER_TIMEOUT * channel_mod._ASK_USER_WAITS_PER_QUESTION
+ channel_mod._BRIDGE_SEND_TIMEOUT
* channel_mod._ASK_USER_SENDS_PER_QUESTION
)
assert channel_mod._ask_user_result_timeout(question_count) == (
per_question_worst_case * question_count + channel_mod._ENGINE_RESULT_SLACK
)
def test_hitl_outer_timeout_exceeds_engine_worst_case(self):
engine_worst_case = (
channel_mod.HITL_APPROVAL_TIMEOUT
+ channel_mod._BRIDGE_SEND_TIMEOUT * channel_mod._HITL_SENDS_PER_APPROVAL
)
assert channel_mod._hitl_result_timeout() == (
engine_worst_case + channel_mod._ENGINE_RESULT_SLACK
)
assert channel_mod._hitl_result_timeout() > engine_worst_case
def test_outer_timeout_cancels_engine_and_releases_reply_slot(self, monkeypatch):
session_key = "fake:timeout"
registered = threading.Event()
cancelled = threading.Event()
async def _wait_forever():
channel_mod._reply_registry.register(session_key)
registered.set()
try:
await asyncio.Future()
except asyncio.CancelledError:
cancelled.set()
raise
finally:
channel_mod._reply_registry.discard(session_key)
with _BusLoopThread() as loop:
monkeypatch.setattr(channel_mod, "_bus_loop", loop)
result = channel_mod._run_engine_on_bus(
_wait_forever(),
result_timeout=0.2,
on_error=lambda: "cancelled",
)
assert result == "cancelled"
assert registered.wait(timeout=1.0)
assert cancelled.wait(timeout=1.0)
assert channel_mod._reply_registry.try_resolve(session_key, "late") is False
# ═══════════════════════════════════════════════════════════════════════
# Bridge round-trip: channel_hitl_prompt over a real bus loop
# ═══════════════════════════════════════════════════════════════════════
class _BusLoopThread:
"""A dedicated event loop running in a background thread (like the bus)."""
def __init__(self):
self.loop = asyncio.new_event_loop()
self._thread = threading.Thread(target=self.loop.run_forever, daemon=True)
def __enter__(self):
self._thread.start()
return self.loop
def __exit__(self, *exc):
self.loop.call_soon_threadsafe(self.loop.stop)
self._thread.join(timeout=2)
self.loop.close()
def _feed_reply_when_ready(loop, session_key, reply, *, tries=200):
"""Schedule a coroutine on *loop* that resolves the pending wait."""
async def _feeder():
for _ in range(tries):
if session_key in channel_mod._reply_registry:
channel_mod._reply_registry.try_resolve(session_key, reply)
return
await asyncio.sleep(0.01)
asyncio.run_coroutine_threadsafe(_feeder(), loop)
class TestHitlPromptBridge:
def test_no_bus_loop_rejects(self, monkeypatch):
monkeypatch.setattr(interaction_mod, "config_auto_approve", lambda reqs: False)
monkeypatch.setattr(channel_mod, "_bus_loop", None)
msg = ChannelMessage(
msg_id="m1",
content="",
sender="u1",
channel_type="fake",
chat_id="chat1",
bus_ref=object(),
)
assert channel_mod.channel_hitl_prompt([{"name": "execute"}], msg) is None
def test_session_grant_approves_when_bus_loop_down(self, monkeypatch):
monkeypatch.setattr(interaction_mod, "config_auto_approve", lambda reqs: False)
monkeypatch.setattr(channel_mod, "_bus_loop", None)
msg = ChannelMessage(
msg_id="m1",
content="",
sender="u1",
channel_type="fake",
chat_id="chat1",
bus_ref=object(),
)
channel_mod._approval_policy.grant_session("fake:chat1")
assert channel_mod.channel_hitl_prompt([{"name": "execute"}], msg) == [
{"type": "approve"}
]
def test_approve_round_trip(self, monkeypatch):
# Force the manual-prompt path (no config auto-approve).
monkeypatch.setattr(interaction_mod, "config_auto_approve", lambda reqs: False)
with _BusLoopThread() as loop:
monkeypatch.setattr(channel_mod, "_bus_loop", loop)
monkeypatch.setattr(channel_mod, "_manager", None) # default caps
bus = MessageBus()
msg = ChannelMessage(
msg_id="m1",
content="",
sender="u1",
channel_type="fake",
chat_id="chat1",
bus_ref=bus,
metadata={},
)
_feed_reply_when_ready(loop, "fake:chat1", "1")
result = channel_mod.channel_hitl_prompt(
[{"name": "execute", "args": {"command": "ls"}}], msg
)
assert result == [{"type": "approve"}]
def test_reject_round_trip(self, monkeypatch):
monkeypatch.setattr(interaction_mod, "config_auto_approve", lambda reqs: False)
with _BusLoopThread() as loop:
monkeypatch.setattr(channel_mod, "_bus_loop", loop)
monkeypatch.setattr(channel_mod, "_manager", None)
bus = MessageBus()
msg = ChannelMessage(
msg_id="m1",
content="",
sender="u1",
channel_type="fake",
chat_id="chat1",
bus_ref=bus,
metadata={},
)
_feed_reply_when_ready(loop, "fake:chat1", "2")
result = channel_mod.channel_hitl_prompt(
[{"name": "execute", "args": {"command": "ls"}}], msg
)
assert result is None
def test_unrecognized_reply_declines_without_refeed(self, monkeypatch):
"""CLI-bridge policy: unparseable reply → explicit notice, NO refeed.
The reply is consumed by the interception registry, never enqueued
as a new turn, and the user gets the unrecognized-reply notice. Only
the serve-mode consumer refeeds; see
TestConsumerUnrecognizedRefeed in tests/test_interaction_engine.py.
"""
monkeypatch.setattr(interaction_mod, "config_auto_approve", lambda reqs: False)
with _BusLoopThread() as loop:
monkeypatch.setattr(channel_mod, "_bus_loop", loop)
monkeypatch.setattr(channel_mod, "_manager", None)
bus = MessageBus()
msg = ChannelMessage(
msg_id="m1",
content="",
sender="u1",
channel_type="fake",
chat_id="chat1",
bus_ref=bus,
metadata={},
)
_feed_reply_when_ready(loop, "fake:chat1", "do something else instead")
result = channel_mod.channel_hitl_prompt(
[{"name": "execute", "args": {"command": "ls"}}], msg
)
assert result is None
# Outbound: prompt, then the exact old unrecognized notice.
async def _drain():
out = []
while True:
try:
m = await asyncio.wait_for(bus.consume_outbound(), timeout=0.2)
except TimeoutError:
return out
out.append(m.content)
contents = asyncio.run_coroutine_threadsafe(_drain(), loop).result(
timeout=5
)
assert contents[-1] == interaction_mod.UNRECOGNIZED_FEEDBACK
# No refeed: nothing was enqueued for the main thread.
assert channel_mod._message_queue.empty()
def test_approve_all_grants_channel_session(self, monkeypatch):
monkeypatch.setattr(interaction_mod, "config_auto_approve", lambda reqs: False)
with _BusLoopThread() as loop:
monkeypatch.setattr(channel_mod, "_bus_loop", loop)
monkeypatch.setattr(channel_mod, "_manager", None)
bus = MessageBus()
msg = ChannelMessage(
msg_id="m1",
content="",
sender="u1",
channel_type="fake",
chat_id="chat1",
bus_ref=bus,
metadata={},
)
_feed_reply_when_ready(loop, "fake:chat1", "3")
result = channel_mod.channel_hitl_prompt(
[{"name": "execute", "args": {"command": "ls"}}], msg
)
assert result == [{"type": "approve"}]
# "Approve all" grant persists: a second prompt auto-approves with
# no reply fed at all.
result2 = channel_mod.channel_hitl_prompt(
[{"name": "execute", "args": {"command": "rm"}}], msg
)
assert result2 == [{"type": "approve"}]
class TestAskUserPromptBridge:
def test_no_msg_cancels(self):
assert channel_mod.channel_ask_user_prompt(
{"questions": [{"question": "Q?", "type": "text"}]}, None
) == {"status": "cancelled"}
def test_empty_questions_answered(self):
assert channel_mod.channel_ask_user_prompt({"questions": []}, None) == {
"answers": [],
"status": "answered",
}
def test_text_answer_round_trip(self, monkeypatch):
with _BusLoopThread() as loop:
monkeypatch.setattr(channel_mod, "_bus_loop", loop)
bus = MessageBus()
msg = ChannelMessage(
msg_id="m1",
content="",
sender="u1",
channel_type="fake",
chat_id="chat1",
bus_ref=bus,
metadata={},
)
_feed_reply_when_ready(loop, "fake:chat1", "CIFAR-10")
result = channel_mod.channel_ask_user_prompt(
{"questions": [{"question": "Which dataset?", "type": "text"}]}, msg
)
assert result == {"answers": ["CIFAR-10"], "status": "answered"}
class TestBridgeClosesUnscheduledCoroutine:
def test_no_bus_loop_closes_coro(self, monkeypatch):
"""The bridge must close an engine coroutine it never scheduled —
otherwise GC emits a "was never awaited" RuntimeWarning. (cr_frame
is not a reliable observable for close() on unstarted coroutines.)"""
import gc
import warnings
monkeypatch.setattr(channel_mod, "_bus_loop", None)
async def _engine():
return "never"
coro = _engine()
result = channel_mod._run_engine_on_bus(
coro, result_timeout=1.0, on_error=lambda: "fallback"
)
assert result == "fallback"
with warnings.catch_warnings(record=True) as caught:
warnings.simplefilter("always")
del coro
gc.collect()
assert not [w for w in caught if issubclass(w.category, RuntimeWarning)]