From 5c829942d755980c447139627ffda5fb12806d58 Mon Sep 17 00:00:00 2001 From: dinos Date: Thu, 30 Apr 2026 19:06:22 +0200 Subject: [PATCH] refactor(cli): replace channel module globals with ChannelRuntime (#197) * refactor(cli): replace channel module globals with ChannelRuntime Removes _cli_agent / _cli_thread_id from EvoScientist/cli/channel.py and threads a ChannelRuntime via CommandContext.channel_runtime so /model and /channel rebind without poking module-level state. * fix(cli): address coderabbit review - _auto_start_channel: bind ChannelRuntime only after _start_channels_bus_mode succeeds, so a startup failure no longer leaves a stale binding pointing at channels that never started. - _sync_tui_command_completion (TUI) and the Rich CLI command-completion paths: rebind the runtime on thread rotation, not just agent swap, so /new and /resume keep ChannelRuntime in sync with the running thread (matches the serve-mode hook contract). - test_hook_syncs_channel_runtime: pin ctx.thread_id explicitly so a bare MagicMock attribute can't silently mutate runtime.thread_id. - New regression test covering the rebind-on-thread-rotation contract. --- EvoScientist/cli/channel.py | 44 +++++++++----- EvoScientist/cli/commands.py | 60 ++++++++++--------- EvoScientist/cli/interactive.py | 50 +++++++++++++--- EvoScientist/cli/tui_interactive.py | 23 +++++-- EvoScientist/commands/base.py | 17 ++++++ .../commands/implementation/channel.py | 23 +++---- EvoScientist/commands/implementation/model.py | 12 ++-- tests/test_channel_command.py | 48 +++++---------- tests/test_cli_channel_bus_mode.py | 12 ++-- tests/test_cli_serve.py | 2 +- tests/test_serve_agent_holder.py | 56 +++++++---------- tests/test_tui_command_sync.py | 37 ++++++++++-- 12 files changed, 228 insertions(+), 156 deletions(-) diff --git a/EvoScientist/cli/channel.py b/EvoScientist/cli/channel.py index 01e15f9..fec8e2b 100644 --- a/EvoScientist/cli/channel.py +++ b/EvoScientist/cli/channel.py @@ -22,6 +22,7 @@ from typing import Any from rich.panel import Panel from rich.text import Text +from ..commands.base import ChannelRuntime from ..stream.console import console _channel_logger = logging.getLogger(__name__) @@ -256,6 +257,7 @@ async def dispatch_channel_slash_command( handle_session_resume_cb: Callable[..., Awaitable[None]] | None = None, await_agent_ready: Callable[[], Awaitable[Any]] | None = None, on_cmd_completed: Callable[..., Awaitable[None]] | None = None, + channel_runtime: ChannelRuntime | None = None, ) -> bool: """Dispatch a slash command from a channel message. @@ -317,6 +319,7 @@ async def dispatch_channel_slash_command( handle_session_resume_cb=handle_session_resume_cb, await_agent_ready=await_agent_ready, on_cmd_completed=on_cmd_completed, + channel_runtime=channel_runtime, ) except Exception as exc: # Last-ditch safety: any uncaught exception from inside the @@ -351,6 +354,7 @@ async def _dispatch_channel_slash_impl( handle_session_resume_cb: Callable[..., Awaitable[None]] | None, await_agent_ready: Callable[[], Awaitable[Any]] | None, on_cmd_completed: Callable[..., Awaitable[None]] | None, + channel_runtime: ChannelRuntime | None, ) -> bool: """Inner body of ``dispatch_channel_slash_command``. @@ -389,6 +393,7 @@ async def _dispatch_channel_slash_impl( ui=ui, workspace_dir=workspace_dir, checkpointer=checkpointer, + channel_runtime=channel_runtime, ) try: @@ -695,8 +700,6 @@ def channel_hitl_prompt( _manager: Any | None = None # ChannelManager _bus_loop: asyncio.AbstractEventLoop | None = None _bus_thread: threading.Thread | None = None -_cli_agent: Any = None # shared agent reference (same as CLI) -_cli_thread_id: str | None = None # shared thread_id (same conversation) def _channels_is_running(channel_type: str | None = None) -> bool: @@ -714,9 +717,18 @@ def _channels_running_list() -> list[str]: return _manager.running_channels() if _manager else [] -def _channels_stop(channel_type: str | None = None) -> None: - """Stop channel(s) and clean up module-level state.""" - global _manager, _bus_loop, _bus_thread, _cli_agent, _cli_thread_id +def _channels_stop( + channel_type: str | None = None, + *, + runtime: ChannelRuntime | None = None, +) -> None: + """Stop channel(s) and clean up module-level state. + + ``runtime`` is the ``ChannelRuntime`` whose binding should be + cleared once the channels are gone — the caller owns it (commands + keep a reference via ``ctx.channel_runtime``). + """ + global _manager, _bus_loop, _bus_thread if channel_type is None: # Stop everything @@ -736,8 +748,8 @@ def _channels_stop(channel_type: str | None = None) -> None: _manager = None _bus_loop = None _bus_thread = None - _cli_agent = None - _cli_thread_id = None + if runtime is not None: + runtime.clear() return # Stop a specific channel @@ -751,9 +763,8 @@ def _channels_stop(channel_type: str | None = None) -> None: except Exception as e: _channel_logger.debug(f"Error removing channel {channel_type}: {e}") - if _manager and not _manager.running_channels(): - _cli_agent = None - _cli_thread_id = None + if _manager and not _manager.running_channels() and runtime is not None: + runtime.clear() def _start_channels_bus_mode( @@ -1071,6 +1082,7 @@ def _auto_start_channel( config, *, send_thinking: bool | None = None, + runtime: ChannelRuntime | None = None, ) -> None: """Start channels automatically from config (bus mode). @@ -1078,21 +1090,23 @@ def _auto_start_channel( agent: Compiled agent graph. thread_id: Current thread ID. config: EvoScientistConfig with channel settings. + runtime: Caller-owned ``ChannelRuntime`` to bind so commands + running over the channels can swap the agent later. ``None`` + is accepted for callers that don't yet pass one. """ - global _cli_agent, _cli_thread_id - if not config.channel_enabled: return - _cli_agent = agent - _cli_thread_id = thread_id - _start_channels_bus_mode( config, agent, thread_id, send_thinking=send_thinking, ) + # Bind only after startup succeeds; a failure above must not leave + # a stale runtime binding pointing at channels that never started. + if runtime is not None: + runtime.bind(agent, thread_id) types = [t.strip() for t in config.channel_enabled.split(",") if t.strip()] results = [(ct, True, "connected (bus)") for ct in types] _print_channel_panel(results) diff --git a/EvoScientist/cli/commands.py b/EvoScientist/cli/commands.py index f46c1e4..bf883ce 100644 --- a/EvoScientist/cli/commands.py +++ b/EvoScientist/cli/commands.py @@ -501,7 +501,10 @@ async def compact_conversation( _serve_logger = logging.getLogger(__name__) -def _make_serve_start_new_session_cb(agent_holder: dict[str, Any]): +def _make_serve_start_new_session_cb( + agent_holder: dict[str, Any], + channel_runtime: Any | None = None, +): """Build the ``start_new_session_cb`` used by serve mode. ``/new`` delegates session rotation entirely to this callback: it @@ -510,8 +513,8 @@ def _make_serve_start_new_session_cb(agent_holder: dict[str, Any]): fresh thread id. Without a wired callback the channel user gets ``ChannelCommandUI``'s fallback "restart the channel link" message and nothing actually rotates. This helper generates a new thread - id, updates the shared holder, and syncs the channel-module global - so subsequent messages land on the new thread. + id, updates the shared holder, and syncs the channel runtime so + subsequent messages land on the new thread. """ def _cb() -> None: @@ -519,25 +522,23 @@ def _make_serve_start_new_session_cb(agent_holder: dict[str, Any]): new_tid = generate_thread_id() agent_holder["thread_id"] = new_tid - try: - import EvoScientist.cli.channel as _ch_mod - - _ch_mod._cli_thread_id = new_tid - except Exception: # pragma: no cover — defensive - pass + if channel_runtime is not None: + channel_runtime.thread_id = new_tid console.print(f"[dim][serve] New thread: {new_tid}[/dim]") return _cb -def _make_serve_cmd_completed_hook(agent_holder: dict[str, Any]): +def _make_serve_cmd_completed_hook( + agent_holder: dict[str, Any], + channel_runtime: Any | None = None, +): """Build the ``on_cmd_completed`` hook used by serve mode. Adopts ``/model`` agent swaps and ``/resume`` thread/workspace swaps back into ``agent_holder`` so the outer poll loop picks up the new handles on subsequent messages. Also keeps - ``EvoScientist.cli.channel`` globals in sync so other readers - (e.g. the bus) see the new values. + ``channel_runtime`` in sync so the bus sees the new values. For ``/resume`` specifically, surface a user-visible warning via ``ctx.ui``: serve uses ``InMemorySaver`` (not the SQLite @@ -551,14 +552,10 @@ def _make_serve_cmd_completed_hook(agent_holder: dict[str, Any]): """ async def _hook(ctx: Any, original_agent: Any, cmd: Any) -> None: - import EvoScientist.cli.channel as _ch_mod - if ctx.agent is not None and ctx.agent is not original_agent: agent_holder["agent"] = ctx.agent - try: - _ch_mod._cli_agent = ctx.agent - except Exception: # pragma: no cover — defensive - pass + if channel_runtime is not None: + channel_runtime.agent = ctx.agent # ``/resume`` mutates ``ctx.thread_id`` directly (its UI callback # is a no-op in serve mode since there's no REPL to reset). Pick @@ -572,10 +569,8 @@ def _make_serve_cmd_completed_hook(agent_holder: dict[str, Any]): thread_changed = bool(new_tid) and new_tid != agent_holder.get("thread_id") if thread_changed: agent_holder["thread_id"] = new_tid - try: - _ch_mod._cli_thread_id = new_tid - except Exception: # pragma: no cover — defensive - pass + if channel_runtime is not None: + channel_runtime.thread_id = new_tid new_workspace = getattr(ctx, "workspace_dir", None) if new_workspace and new_workspace != agent_holder.get("workspace_dir"): @@ -608,6 +603,7 @@ def _serve_process_message( show_thinking: bool, on_cmd_completed: Callable[..., Awaitable[None]] | None = None, start_new_session_cb: Callable[[], None] | None = None, + channel_runtime: Any | None = None, ) -> None: """Process a single channel message in headless serve mode. @@ -728,9 +724,10 @@ def _serve_process_message( checkpointer=None, append_system=lambda t, s="dim": console.print(t, style=s), start_new_session_cb=start_new_session_cb - or _make_serve_start_new_session_cb(agent_holder), + or _make_serve_start_new_session_cb(agent_holder, channel_runtime), on_cmd_completed=on_cmd_completed - or _make_serve_cmd_completed_hook(agent_holder), + or _make_serve_cmd_completed_hook(agent_holder, channel_runtime), + channel_runtime=channel_runtime, ) ) except Exception as exc: @@ -887,11 +884,19 @@ def serve( "workspace_dir": ws, } + from ..commands.base import ChannelRuntime + + channel_runtime = ChannelRuntime(agent=agent, thread_id=tid) + # Build the slash-dispatch callbacks once; the poll loop reuses # them for every inbound message. Without this hoist each message # would allocate a fresh closure pair. - _serve_on_cmd_completed = _make_serve_cmd_completed_hook(agent_holder) - _serve_start_new_session_cb = _make_serve_start_new_session_cb(agent_holder) + _serve_on_cmd_completed = _make_serve_cmd_completed_hook( + agent_holder, channel_runtime + ) + _serve_start_new_session_cb = _make_serve_start_new_session_cb( + agent_holder, channel_runtime + ) _start_channels_bus_mode( config, @@ -946,6 +951,7 @@ def serve( show_thinking=effective_channel_thinking, on_cmd_completed=_serve_on_cmd_completed, start_new_session_cb=_serve_start_new_session_cb, + channel_runtime=channel_runtime, ) except KeyboardInterrupt: shutdown_event.set() @@ -956,7 +962,7 @@ def serve( signal.signal(signal.SIGINT, _orig_sigint) signal.signal(signal.SIGTERM, _orig_sigterm) console.print("\n[dim]Shutting down...[/dim]") - _channels_stop() + _channels_stop(runtime=channel_runtime) console.print("[dim]Stopped.[/dim]") diff --git a/EvoScientist/cli/interactive.py b/EvoScientist/cli/interactive.py index c0943dd..ec2af7d 100644 --- a/EvoScientist/cli/interactive.py +++ b/EvoScientist/cli/interactive.py @@ -321,6 +321,10 @@ def cmd_interactive( "status_last_input_tokens": None, } + from ..commands.base import ChannelRuntime + + channel_runtime = ChannelRuntime() + progress_tracker = MCPProgressTracker() def _on_mcp_progress(event: str, server: str, detail: str) -> None: @@ -386,8 +390,7 @@ def cmd_interactive( raise await _refresh_status_snapshot(reset_streaming_text=True) if _channels_is_running(): - _ch_mod._cli_agent = agent - _ch_mod._cli_thread_id = state["thread_id"] + channel_runtime.bind(agent, state["thread_id"]) return agent def _rebuild_status_snapshot() -> None: @@ -833,9 +836,22 @@ def cmd_interactive( state["status_base_snapshot"] = make_empty_status_snapshot( model ) - if _channels_is_running(): - _ch_mod._cli_agent = ctx.agent - _ch_mod._cli_thread_id = state["thread_id"] + + # Rebind the runtime whenever the agent OR + # thread_id may have moved — ``/new`` and + # ``/resume`` rotate ``state["thread_id"]`` + # without swapping the agent, and the bus + # expects both to stay in sync (matches the + # serve-mode hook contract). + if _channels_is_running(): + runtime_agent = ( + ctx.agent + if ctx.agent is not None + else agent_loader.agent + ) + if runtime_agent is not None: + channel_runtime.bind(runtime_agent, state["thread_id"]) + # ``/new`` rotates ``state["thread_id"]`` / workspace, # ``/compact`` reduces token usage — both need the # status snapshot re-rendered even when the agent @@ -858,6 +874,7 @@ def cmd_interactive( handle_session_resume_cb=_on_handle_session_resume, await_agent_ready=_await_agent_ready, on_cmd_completed=_on_channel_cmd_completed, + channel_runtime=channel_runtime, ) if _slash_handled: _print_separator() @@ -956,6 +973,7 @@ def cmd_interactive( state["thread_id"], cfg, send_thinking=channel_send_thinking, + runtime=channel_runtime, ) _auto_start_task = asyncio.create_task( @@ -1036,6 +1054,7 @@ def cmd_interactive( checkpointer=checkpointer, config=config, input_tokens_hint=state.get("status_last_input_tokens"), + channel_runtime=channel_runtime, ) await cmd_manager.execute(user_input, ctx) @@ -1046,7 +1065,7 @@ def cmd_interactive( # Agent swap (e.g. /model successfully built a # new agent): adopt into loader + reset status - # snapshot + sync channel globals. + # snapshot + sync channel runtime. agent_swapped = ( ctx.agent is not None and ctx.agent is not _agent_for_ctx @@ -1060,9 +1079,22 @@ def cmd_interactive( state["status_base_snapshot"] = ( make_empty_status_snapshot(model) ) - if _channels_is_running(): - _ch_mod._cli_agent = ctx.agent - _ch_mod._cli_thread_id = state["thread_id"] + + # Rebind the runtime whenever the agent OR + # thread_id may have moved — ``/new`` / + # ``/resume`` rotate ``state["thread_id"]`` + # without swapping the agent, and the bus + # expects both to stay in sync. + if _channels_is_running(): + runtime_agent = ( + ctx.agent + if ctx.agent is not None + else agent_loader.agent + ) + if runtime_agent is not None: + channel_runtime.bind( + runtime_agent, state["thread_id"] + ) # Commands that mutate status fields need an # async refresh here (/compact + /new use sync diff --git a/EvoScientist/cli/tui_interactive.py b/EvoScientist/cli/tui_interactive.py index f4dc469..07b11cb 100644 --- a/EvoScientist/cli/tui_interactive.py +++ b/EvoScientist/cli/tui_interactive.py @@ -207,9 +207,15 @@ async def _sync_tui_command_completion( update_model = getattr(app, "update_status_after_model_change", None) if callable(update_model): update_model(cfg.model, cfg.provider) - if _channels_is_running(): - _ch_mod._cli_agent = ctx.agent - _ch_mod._cli_thread_id = app._conversation_tid + + # Rebind the runtime whenever the agent OR thread_id may have moved + # — ``/new`` and ``/resume`` rotate ``app._conversation_tid`` + # without swapping the agent, and the bus expects both to stay in + # sync (matches the serve-mode hook contract). + if _channels_is_running(): + runtime_agent = ctx.agent if ctx.agent is not None else app._agent_loader.agent + if runtime_agent is not None: + app._channel_runtime.bind(runtime_agent, app._conversation_tid) await app._refresh_status_snapshot(reset_streaming_text=True) @@ -391,6 +397,9 @@ def run_textual_interactive( self._history_index: int = -1 # -1 = not browsing history self._history_saved_input: str = "" # saved current input before browsing self._background_tasks: set[asyncio.Task] = set() + from ..commands.base import ChannelRuntime + + self._channel_runtime = ChannelRuntime() self._quit_pending: bool = False self._current_model: str | None = model self._current_provider: str | None = provider @@ -425,8 +434,7 @@ def run_textual_interactive( def _on_agent_load_success(self, agent: Any) -> None: if _channels_is_running(): - _ch_mod._cli_agent = agent - _ch_mod._cli_thread_id = self._conversation_tid + self._channel_runtime.bind(agent, self._conversation_tid) self._finish_loader_widget() self._render_status() @@ -730,6 +738,7 @@ def run_textual_interactive( self._conversation_tid, cfg, send_thinking=self._channel_send_thinking, + runtime=self._channel_runtime, ) types = [ t.strip() for t in cfg.channel_enabled.split(",") if t.strip() @@ -1915,6 +1924,7 @@ def run_textual_interactive( handle_session_resume_cb=self.handle_session_resume, await_agent_ready=self._await_agent_ready, on_cmd_completed=self._on_channel_cmd_completed, + channel_runtime=self._channel_runtime, ) if _slash_handled: return # outer finally handles _busy / widget cleanup @@ -2372,6 +2382,7 @@ def run_textual_interactive( workspace_dir=self._workspace_dir, checkpointer=self._checkpointer, input_tokens_hint=self._status_last_input_tokens, + channel_runtime=self._channel_runtime, ) if await cmd_manager.execute(command, ctx): @@ -2500,7 +2511,7 @@ def run_textual_interactive( self._started_channel_types.clear() if _channels_is_running(): try: - _channels_stop() + _channels_stop(runtime=self._channel_runtime) except Exception: pass self.exit() diff --git a/EvoScientist/commands/base.py b/EvoScientist/commands/base.py index 351acb4..fdee96c 100644 --- a/EvoScientist/commands/base.py +++ b/EvoScientist/commands/base.py @@ -51,6 +51,22 @@ class CommandUI(Protocol): async def flush(self) -> None: ... +@dataclass +class ChannelRuntime: + """Mutable handle to the agent + thread bound to running channels.""" + + agent: Any = None + thread_id: str | None = None + + def bind(self, agent: Any, thread_id: str | None) -> None: + self.agent = agent + self.thread_id = thread_id + + def clear(self) -> None: + self.agent = None + self.thread_id = None + + @dataclass class CommandContext: """Context passed to commands during execution.""" @@ -61,6 +77,7 @@ class CommandContext: workspace_dir: str | None = None checkpointer: Any = None config: Any = None + channel_runtime: ChannelRuntime | None = None # Real LLM input token count from last usage_metadata (includes system # prompt + tool schemas). Used by /compact for accurate display. input_tokens_hint: int | None = None diff --git a/EvoScientist/commands/implementation/channel.py b/EvoScientist/commands/implementation/channel.py index 4e83624..5ee40fd 100644 --- a/EvoScientist/commands/implementation/channel.py +++ b/EvoScientist/commands/implementation/channel.py @@ -71,10 +71,10 @@ class ChannelCommand(Command): ctx.ui.append_system("No channels are running.", style="dim") else: if target: - _channels_stop(target) + _channels_stop(target, runtime=ctx.channel_runtime) ctx.ui.append_system(f"Channel '{target}' stopped.", style="green") else: - _channels_stop() + _channels_stop(runtime=ctx.channel_runtime) ctx.ui.append_system("All channels stopped.", style="green") return @@ -98,12 +98,11 @@ class ChannelCommand(Command): ctx.ui.append_system(f"Adding channel(s): {', '.join(requested)}...") from ...cli.channel import _add_channel_to_running_bus - # Sync CLI-side globals up-front so partial-success states (one - # channel attached, next one raises) still leave - # _auto_start_channel observing the latest agent/thread refs. - # See cli/channel.py _cli_agent / _cli_thread_id. - _ch_mod._cli_agent = ctx.agent - _ch_mod._cli_thread_id = ctx.thread_id + # Bind the runtime up-front so partial-success states (one + # channel attached, next one raises) still leave the bus + # observing the latest agent/thread refs. + if ctx.channel_runtime is not None: + ctx.channel_runtime.bind(ctx.agent, ctx.thread_id) try: for ct in requested: _add_channel_to_running_bus(ct, config, send_thinking=send_thinking) @@ -137,12 +136,8 @@ class ChannelCommand(Command): ctx.thread_id, send_thinking=send_thinking, ) - # Sync CLI-side globals so _auto_start_channel observes the - # latest agent/thread refs (see cli/channel.py _cli_agent / - # _cli_thread_id). The pre-migration inline helper set these - # in the same branch. - _ch_mod._cli_agent = ctx.agent - _ch_mod._cli_thread_id = ctx.thread_id + if ctx.channel_runtime is not None: + ctx.channel_runtime.bind(ctx.agent, ctx.thread_id) # Show status panel if _ch_mod._manager: diff --git a/EvoScientist/commands/implementation/model.py b/EvoScientist/commands/implementation/model.py index f8d65b7..4be8f15 100644 --- a/EvoScientist/commands/implementation/model.py +++ b/EvoScientist/commands/implementation/model.py @@ -217,14 +217,10 @@ class ModelCommand(Command): set_config_value("model", model_name) set_config_value("provider", provider) - # Propagate to channel module if channels are running - try: - import EvoScientist.cli.channel as _ch_mod - - if getattr(_ch_mod, "_cli_agent", None) is not None: - _ch_mod._cli_agent = new_agent - except Exception: - pass + # Propagate to the channel runtime if channels are running so the + # bus picks up the new agent on the next inbound message. + if ctx.channel_runtime is not None and ctx.channel_runtime.agent is not None: + ctx.channel_runtime.agent = new_agent # Update status bar if available update_model_fn = getattr(ctx.ui, "update_status_after_model_change", None) diff --git a/tests/test_channel_command.py b/tests/test_channel_command.py index ef90bff..a466785 100644 --- a/tests/test_channel_command.py +++ b/tests/test_channel_command.py @@ -3,37 +3,23 @@ from types import SimpleNamespace from unittest.mock import MagicMock, patch -import pytest - from tests.conftest import run_async as _run -@pytest.fixture(autouse=True) -def _reset_channel_globals(): - """Reset module-level ``_cli_agent``/``_cli_thread_id`` around every test. - - These globals are written by the start / add-to-running paths and would - otherwise leak between tests (and into unrelated suites). - """ - import EvoScientist.cli.channel as _ch - - _ch._cli_agent = None - _ch._cli_thread_id = None - try: - yield - finally: - _ch._cli_agent = None - _ch._cli_thread_id = None - - def _ctx(): - from EvoScientist.commands.base import CommandContext + from EvoScientist.commands.base import ChannelRuntime, CommandContext ui = MagicMock() ui.supports_interactive = True - return CommandContext( - agent=object(), thread_id="tid-42", ui=ui, workspace_dir="/ws" - ), ui + runtime = ChannelRuntime() + ctx = CommandContext( + agent=object(), + thread_id="tid-42", + ui=ui, + workspace_dir="/ws", + channel_runtime=runtime, + ) + return ctx, ui class TestNeedsAgent: @@ -69,8 +55,7 @@ class TestNeedsAgent: class TestStartPath: """Start flow must propagate agent/thread_id globals.""" - def test_start_sets_cli_agent_globals(self): - import EvoScientist.cli.channel as _ch_mod + def test_start_binds_channel_runtime(self): from EvoScientist.commands.implementation.channel import ChannelCommand ctx, _ui = _ctx() @@ -93,8 +78,8 @@ class TestStartPath: ), ): _run(ChannelCommand().execute(ctx, ["telegram"])) - assert _ch_mod._cli_agent is ctx.agent - assert _ch_mod._cli_thread_id == "tid-42" + assert ctx.channel_runtime.agent is ctx.agent + assert ctx.channel_runtime.thread_id == "tid-42" def test_start_propagates_send_thinking(self): """send_thinking flag must reach _start_channels_bus_mode.""" @@ -133,8 +118,7 @@ class TestStartPath: class TestAddToRunningPath: - def test_add_to_running_sets_cli_agent_globals(self): - import EvoScientist.cli.channel as _ch_mod + def test_add_to_running_binds_channel_runtime(self): from EvoScientist.commands.implementation.channel import ChannelCommand ctx, _ui = _ctx() @@ -157,8 +141,8 @@ class TestAddToRunningPath: ), ): _run(ChannelCommand().execute(ctx, ["discord"])) - assert _ch_mod._cli_agent is ctx.agent - assert _ch_mod._cli_thread_id == "tid-42" + assert ctx.channel_runtime.agent is ctx.agent + assert ctx.channel_runtime.thread_id == "tid-42" def test_add_to_running_propagates_send_thinking(self): """Adding to a running bus must honor config.channel_send_thinking.""" diff --git a/tests/test_cli_channel_bus_mode.py b/tests/test_cli_channel_bus_mode.py index fe77b4a..a8a35a3 100644 --- a/tests/test_cli_channel_bus_mode.py +++ b/tests/test_cli_channel_bus_mode.py @@ -11,23 +11,21 @@ from EvoScientist.cli import channel as channel_cli @pytest.fixture(autouse=True) def _restore_channel_globals(): - """Restore mutable module globals after each test.""" + """Restore the bus-mode globals after each test.""" original = { "_manager": channel_cli._manager, "_bus_loop": channel_cli._bus_loop, "_bus_thread": channel_cli._bus_thread, - "_cli_agent": channel_cli._cli_agent, - "_cli_thread_id": channel_cli._cli_thread_id, } yield channel_cli._manager = original["_manager"] channel_cli._bus_loop = original["_bus_loop"] channel_cli._bus_thread = original["_bus_thread"] - channel_cli._cli_agent = original["_cli_agent"] - channel_cli._cli_thread_id = original["_cli_thread_id"] def test_auto_start_channel_passes_send_thinking(monkeypatch): + from EvoScientist.commands.base import ChannelRuntime + captured = {} def _fake_start(config, agent, thread_id, *, send_thinking=None): @@ -40,13 +38,17 @@ def test_auto_start_channel_passes_send_thinking(monkeypatch): config = SimpleNamespace(channel_enabled="telegram") agent = object() + runtime = ChannelRuntime() channel_cli._auto_start_channel( agent, "thread-1", config, send_thinking=False, + runtime=runtime, ) assert captured["send_thinking"] is False assert captured["thread_id"] == "thread-1" assert captured["agent"] is agent + assert runtime.agent is agent + assert runtime.thread_id == "thread-1" diff --git a/tests/test_cli_serve.py b/tests/test_cli_serve.py index d40cd59..d70bb73 100644 --- a/tests/test_cli_serve.py +++ b/tests/test_cli_serve.py @@ -65,7 +65,7 @@ def _run_serve_once( captured["send_thinking"] = send_thinking captured["thread_id"] = thread_id - def _fake_channels_stop(): + def _fake_channels_stop(channel_type=None, *, runtime=None): captured["stopped"] = True class _InterruptQueue: diff --git a/tests/test_serve_agent_holder.py b/tests/test_serve_agent_holder.py index 022c633..b1aa210 100644 --- a/tests/test_serve_agent_holder.py +++ b/tests/test_serve_agent_holder.py @@ -8,8 +8,6 @@ captured at startup. from unittest.mock import AsyncMock, MagicMock, patch -import pytest - from EvoScientist.cli.channel import ( ChannelMessage, _register_channel_request, @@ -19,22 +17,10 @@ from EvoScientist.cli.commands import ( _make_serve_start_new_session_cb, _serve_process_message, ) +from EvoScientist.commands.base import ChannelRuntime from tests.conftest import run_async as _run -@pytest.fixture(autouse=True) -def _restore_channel_globals(): - import EvoScientist.cli.channel as _ch_mod - - prev_agent = _ch_mod._cli_agent - prev_tid = _ch_mod._cli_thread_id - try: - yield - finally: - _ch_mod._cli_agent = prev_agent - _ch_mod._cli_thread_id = prev_tid - - def test_hook_updates_holder_on_agent_swap(): """``/model`` mutates ``ctx.agent`` to a new handle — the hook must push that handle into the shared holder so the outer poll loop sees @@ -52,22 +38,26 @@ def test_hook_updates_holder_on_agent_swap(): assert holder["agent"] == "new-agent" -def test_hook_syncs_channel_module_global(): - """Other readers (the bus) look at ``cli.channel._cli_agent``; the - hook keeps that global in sync with the holder update.""" - import EvoScientist.cli.channel as _ch_mod - - holder = {"agent": "original-agent"} - hook = _make_serve_cmd_completed_hook(holder) +def test_hook_syncs_channel_runtime(): + """Other readers (the bus) look at ``ChannelRuntime.agent``; the + hook keeps the runtime in sync with the holder update.""" + holder = {"agent": "original-agent", "thread_id": "t"} + runtime = ChannelRuntime(agent="original-agent", thread_id="t") + hook = _make_serve_cmd_completed_hook(holder, runtime) ctx = MagicMock() ctx.agent = "new-agent" + # Pin ctx.thread_id explicitly — a bare MagicMock would let the + # hook's getattr fall through to a fresh MagicMock attribute and + # silently mutate runtime.thread_id, hiding regressions. + ctx.thread_id = "t" cmd = MagicMock() cmd.name = "/model" _run(hook(ctx, "original-agent", cmd)) - assert _ch_mod._cli_agent == "new-agent" + assert runtime.agent == "new-agent" + assert runtime.thread_id == "t" def test_hook_noop_when_agent_unchanged(): @@ -137,13 +127,12 @@ def test_hook_updates_workspace_dir_on_resume(): assert holder["workspace_dir"] == "/restored-ws" -def test_hook_syncs_channel_module_thread_id(): - """The bus reads ``cli.channel._cli_thread_id``; hook must sync it +def test_hook_syncs_channel_runtime_thread_id(): + """The bus reads ``ChannelRuntime.thread_id``; hook must sync it alongside the holder update.""" - import EvoScientist.cli.channel as _ch_mod - holder = {"agent": "a", "thread_id": "original-tid"} - hook = _make_serve_cmd_completed_hook(holder) + runtime = ChannelRuntime(agent="a", thread_id="original-tid") + hook = _make_serve_cmd_completed_hook(holder, runtime) ctx = MagicMock() ctx.agent = "a" @@ -153,7 +142,7 @@ def test_hook_syncs_channel_module_thread_id(): _run(hook(ctx, "a", cmd)) - assert _ch_mod._cli_thread_id == "new-tid" + assert runtime.thread_id == "new-tid" def test_hook_noop_when_thread_id_unchanged(): @@ -220,20 +209,19 @@ def test_hook_emits_resume_warning_when_thread_changed(): def test_start_new_session_cb_rotates_thread_id(): """``/new`` via channel calls this callback — must generate a new - thread id, push into holder, and sync the channel-module global.""" - import EvoScientist.cli.channel as _ch_mod - + thread id, push into holder, and sync the channel runtime.""" holder = {"agent": "a", "thread_id": "old-tid"} + runtime = ChannelRuntime(agent="a", thread_id="old-tid") with patch( "EvoScientist.sessions.generate_thread_id", return_value="freshly-generated-tid", ): - cb = _make_serve_start_new_session_cb(holder) + cb = _make_serve_start_new_session_cb(holder, runtime) cb() assert holder["thread_id"] == "freshly-generated-tid" - assert _ch_mod._cli_thread_id == "freshly-generated-tid" + assert runtime.thread_id == "freshly-generated-tid" def test_start_new_session_cb_leaves_agent_alone(): diff --git a/tests/test_tui_command_sync.py b/tests/test_tui_command_sync.py index 4b0d642..747ec94 100644 --- a/tests/test_tui_command_sync.py +++ b/tests/test_tui_command_sync.py @@ -4,7 +4,7 @@ from types import SimpleNamespace import pytest -from EvoScientist.commands.base import CommandContext +from EvoScientist.commands.base import ChannelRuntime, CommandContext from tests.conftest import run_async as _run pytest.importorskip("textual") @@ -13,15 +13,18 @@ pytest.importorskip("textual") class _Loader: def __init__(self) -> None: self.adopt_calls: list[object] = [] + self.agent: object | None = None def adopt(self, agent: object) -> None: self.adopt_calls.append(agent) + self.agent = agent class _StubApp: def __init__(self) -> None: self._agent_loader = _Loader() self._conversation_tid = "thread-1" + self._channel_runtime = ChannelRuntime() self.model_updates: list[tuple[str, str | None]] = [] self.refresh_calls: list[bool] = [] @@ -58,16 +61,15 @@ def test_sync_tui_command_completion_adopts_agent_swap(monkeypatch): lambda: SimpleNamespace(model="gpt-5.5", provider="openai"), ) monkeypatch.setattr(tui_mod, "_channels_is_running", lambda: True) - monkeypatch.setattr(tui_mod._ch_mod, "_cli_agent", "old-agent", raising=False) - monkeypatch.setattr(tui_mod._ch_mod, "_cli_thread_id", "old-thread", raising=False) + app._channel_runtime.bind("old-agent", "old-thread") _run(tui_mod._sync_tui_command_completion(app, ctx, "old-agent", cmd)) assert app._agent_loader.adopt_calls == ["new-agent"] assert app.model_updates == [("gpt-5.5", "openai")] assert app.refresh_calls == [True] - assert tui_mod._ch_mod._cli_agent == "new-agent" - assert tui_mod._ch_mod._cli_thread_id == "thread-1" + assert app._channel_runtime.agent == "new-agent" + assert app._channel_runtime.thread_id == "thread-1" def test_sync_tui_command_completion_refreshes_without_agent_swap(monkeypatch): @@ -88,3 +90,28 @@ def test_sync_tui_command_completion_refreshes_without_agent_swap(monkeypatch): assert app._agent_loader.adopt_calls == [] assert app.model_updates == [] assert app.refresh_calls == [True] + + +def test_sync_tui_rebinds_runtime_on_thread_rotation_without_agent_swap(monkeypatch): + """Regression: ``/new`` and ``/resume`` rotate ``app._conversation_tid`` + without swapping the agent. The runtime must still pick up the new + thread id so the bus contract stays consistent with serve mode.""" + import EvoScientist.cli.tui_interactive as tui_mod + + app = _StubApp() + app._conversation_tid = "rotated-thread" + app._agent_loader.agent = "same-agent" + app._channel_runtime.bind("same-agent", "old-thread") + ctx = CommandContext( + agent="same-agent", + thread_id="rotated-thread", + ui=SimpleNamespace(), + ) + cmd = SimpleNamespace(name="/new") + + monkeypatch.setattr(tui_mod, "_channels_is_running", lambda: True) + + _run(tui_mod._sync_tui_command_completion(app, ctx, "same-agent", cmd)) + + assert app._channel_runtime.agent == "same-agent" + assert app._channel_runtime.thread_id == "rotated-thread"