db1abce8d8
* 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>
427 lines
16 KiB
Python
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)]
|