refactor: introduce AgentLoop.from_config() to centralize loop assembly
Extract duplicated bus/provider/loop initialization from CLI commands (serve, _run_gateway, agent) and Nanobot facade into a single AgentLoop.from_config() classmethod. - Remove _make_provider() from cli/commands.py and nanobot.py - Remove inline provider creation in all three CLI entry points - AgentLoop.from_config() creates MessageBus, calls make_provider(), and assembles AgentLoop with all standard config-derived parameters - Supports **extra overrides for callers that need custom args (e.g. cron_service, session_manager, provider_snapshot_loader) - Update tests to mock make_provider at nanobot.providers.factory and add from_config classmethod to _FakeAgentLoop fixtures This is PR 1/4 of the model-preset feature decomposition.
This commit is contained in:
chengyongru
Xubin Ren
+67
-31
@@ -9,7 +9,8 @@ import pytest
|
||||
from typer.testing import CliRunner
|
||||
|
||||
from nanobot.bus.events import OutboundMessage
|
||||
from nanobot.cli.commands import _make_provider, app
|
||||
from nanobot.cli.commands import app
|
||||
from nanobot.providers.factory import make_provider
|
||||
from nanobot.config.schema import Config
|
||||
from nanobot.cron.types import CronJob, CronPayload
|
||||
from nanobot.providers.factory import ProviderSnapshot
|
||||
@@ -19,6 +20,13 @@ from nanobot.providers.registry import find_by_name
|
||||
runner = CliRunner()
|
||||
|
||||
|
||||
def _fake_provider():
|
||||
"""Return a minimal fake provider that satisfies AgentLoop.__init__."""
|
||||
p = MagicMock()
|
||||
p.generation.max_tokens = 4096
|
||||
return p
|
||||
|
||||
|
||||
class _StopGatewayError(RuntimeError):
|
||||
pass
|
||||
|
||||
@@ -488,7 +496,7 @@ def test_openai_compat_provider_passes_model_through():
|
||||
|
||||
|
||||
def test_make_provider_uses_github_copilot_backend():
|
||||
from nanobot.cli.commands import _make_provider
|
||||
from nanobot.providers.factory import make_provider
|
||||
from nanobot.config.schema import Config
|
||||
|
||||
config = Config.model_validate(
|
||||
@@ -503,7 +511,7 @@ def test_make_provider_uses_github_copilot_backend():
|
||||
)
|
||||
|
||||
with patch("nanobot.providers.openai_compat_provider.AsyncOpenAI"):
|
||||
provider = _make_provider(config)
|
||||
provider = make_provider(config)
|
||||
|
||||
assert provider.__class__.__name__ == "GitHubCopilotProvider"
|
||||
|
||||
@@ -579,7 +587,7 @@ def test_make_provider_passes_extra_headers_to_custom_provider():
|
||||
)
|
||||
|
||||
with patch("nanobot.providers.openai_compat_provider.AsyncOpenAI") as mock_async_openai:
|
||||
_make_provider(config)
|
||||
make_provider(config)
|
||||
|
||||
kwargs = mock_async_openai.call_args.kwargs
|
||||
assert kwargs["api_key"] == "test-key"
|
||||
@@ -597,24 +605,24 @@ def mock_agent_runtime(tmp_path):
|
||||
with patch("nanobot.config.loader.load_config", return_value=config) as mock_load_config, \
|
||||
patch("nanobot.config.loader.resolve_config_env_vars", side_effect=lambda c: c), \
|
||||
patch("nanobot.cli.commands.sync_workspace_templates") as mock_sync_templates, \
|
||||
patch("nanobot.cli.commands._make_provider", return_value=object()), \
|
||||
patch("nanobot.providers.factory.make_provider", return_value=_fake_provider()), \
|
||||
patch("nanobot.cli.commands._print_agent_response") as mock_print_response, \
|
||||
patch("nanobot.bus.queue.MessageBus"), \
|
||||
patch("nanobot.cron.service.CronService"), \
|
||||
patch("nanobot.agent.loop.AgentLoop") as mock_agent_loop_cls:
|
||||
patch("nanobot.cli.commands.AgentLoop.from_config") as mock_from_config:
|
||||
agent_loop = MagicMock()
|
||||
agent_loop.channels_config = None
|
||||
agent_loop.process_direct = AsyncMock(
|
||||
return_value=OutboundMessage(channel="cli", chat_id="direct", content="mock-response"),
|
||||
)
|
||||
agent_loop.close_mcp = AsyncMock(return_value=None)
|
||||
mock_agent_loop_cls.return_value = agent_loop
|
||||
mock_from_config.return_value = agent_loop
|
||||
|
||||
yield {
|
||||
"config": config,
|
||||
"load_config": mock_load_config,
|
||||
"sync_templates": mock_sync_templates,
|
||||
"agent_loop_cls": mock_agent_loop_cls,
|
||||
"from_config": mock_from_config,
|
||||
"agent_loop": agent_loop,
|
||||
"print_response": mock_print_response,
|
||||
}
|
||||
@@ -639,9 +647,8 @@ def test_agent_uses_default_config_when_no_workspace_or_config_flags(mock_agent_
|
||||
assert mock_agent_runtime["sync_templates"].call_args.args == (
|
||||
mock_agent_runtime["config"].workspace_path,
|
||||
)
|
||||
assert mock_agent_runtime["agent_loop_cls"].call_args.kwargs["workspace"] == (
|
||||
mock_agent_runtime["config"].workspace_path
|
||||
)
|
||||
passed_config = mock_agent_runtime["from_config"].call_args.args[0]
|
||||
assert passed_config.workspace_path == mock_agent_runtime["config"].workspace_path
|
||||
mock_agent_runtime["agent_loop"].process_direct.assert_awaited_once()
|
||||
mock_agent_runtime["print_response"].assert_called_once_with(
|
||||
"mock-response", render_markdown=True, metadata={},
|
||||
@@ -672,11 +679,14 @@ def test_agent_config_sets_active_path(monkeypatch, tmp_path: Path) -> None:
|
||||
)
|
||||
monkeypatch.setattr("nanobot.config.loader.load_config", lambda _path=None: config)
|
||||
monkeypatch.setattr("nanobot.cli.commands.sync_workspace_templates", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.cli.commands._make_provider", lambda _config: object())
|
||||
monkeypatch.setattr("nanobot.providers.factory.make_provider", lambda _config: _fake_provider())
|
||||
monkeypatch.setattr("nanobot.bus.queue.MessageBus", lambda: object())
|
||||
monkeypatch.setattr("nanobot.cron.service.CronService", lambda _store: object())
|
||||
|
||||
class _FakeAgentLoop:
|
||||
@classmethod
|
||||
def from_config(cls, config, bus=None, **extra):
|
||||
return cls(**extra)
|
||||
def __init__(self, *args, **kwargs) -> None:
|
||||
pass
|
||||
|
||||
@@ -686,7 +696,7 @@ def test_agent_config_sets_active_path(monkeypatch, tmp_path: Path) -> None:
|
||||
async def close_mcp(self) -> None:
|
||||
return None
|
||||
|
||||
monkeypatch.setattr("nanobot.agent.loop.AgentLoop", _FakeAgentLoop)
|
||||
monkeypatch.setattr("nanobot.cli.commands.AgentLoop", _FakeAgentLoop)
|
||||
monkeypatch.setattr("nanobot.cli.commands._print_agent_response", lambda *_args, **_kwargs: None)
|
||||
|
||||
result = runner.invoke(app, ["agent", "-m", "hello", "-c", str(config_file)])
|
||||
@@ -707,7 +717,7 @@ def test_agent_uses_workspace_directory_for_cron_store(monkeypatch, tmp_path: Pa
|
||||
monkeypatch.setattr("nanobot.config.loader.set_config_path", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.config.loader.load_config", lambda _path=None: config)
|
||||
monkeypatch.setattr("nanobot.cli.commands.sync_workspace_templates", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.cli.commands._make_provider", lambda _config: object())
|
||||
monkeypatch.setattr("nanobot.providers.factory.make_provider", lambda _config: _fake_provider())
|
||||
monkeypatch.setattr("nanobot.bus.queue.MessageBus", lambda: object())
|
||||
|
||||
class _FakeCron:
|
||||
@@ -715,6 +725,9 @@ def test_agent_uses_workspace_directory_for_cron_store(monkeypatch, tmp_path: Pa
|
||||
seen["cron_store"] = store_path
|
||||
|
||||
class _FakeAgentLoop:
|
||||
@classmethod
|
||||
def from_config(cls, config, bus=None, **extra):
|
||||
return cls(**extra)
|
||||
def __init__(self, *args, **kwargs) -> None:
|
||||
pass
|
||||
|
||||
@@ -725,7 +738,7 @@ def test_agent_uses_workspace_directory_for_cron_store(monkeypatch, tmp_path: Pa
|
||||
return None
|
||||
|
||||
monkeypatch.setattr("nanobot.cron.service.CronService", _FakeCron)
|
||||
monkeypatch.setattr("nanobot.agent.loop.AgentLoop", _FakeAgentLoop)
|
||||
monkeypatch.setattr("nanobot.cli.commands.AgentLoop", _FakeAgentLoop)
|
||||
monkeypatch.setattr("nanobot.cli.commands._print_agent_response", lambda *_args, **_kwargs: None)
|
||||
|
||||
result = runner.invoke(app, ["agent", "-m", "hello", "-c", str(config_file)])
|
||||
@@ -753,7 +766,7 @@ def test_agent_workspace_override_does_not_migrate_legacy_cron(
|
||||
monkeypatch.setattr("nanobot.config.loader.set_config_path", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.config.loader.load_config", lambda _path=None: config)
|
||||
monkeypatch.setattr("nanobot.cli.commands.sync_workspace_templates", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.cli.commands._make_provider", lambda _config: object())
|
||||
monkeypatch.setattr("nanobot.providers.factory.make_provider", lambda _config: _fake_provider())
|
||||
monkeypatch.setattr("nanobot.bus.queue.MessageBus", lambda: object())
|
||||
monkeypatch.setattr("nanobot.config.paths.get_cron_dir", lambda: legacy_dir)
|
||||
|
||||
@@ -762,6 +775,9 @@ def test_agent_workspace_override_does_not_migrate_legacy_cron(
|
||||
seen["cron_store"] = store_path
|
||||
|
||||
class _FakeAgentLoop:
|
||||
@classmethod
|
||||
def from_config(cls, config, bus=None, **extra):
|
||||
return cls(**extra)
|
||||
def __init__(self, *args, **kwargs) -> None:
|
||||
pass
|
||||
|
||||
@@ -772,7 +788,7 @@ def test_agent_workspace_override_does_not_migrate_legacy_cron(
|
||||
return None
|
||||
|
||||
monkeypatch.setattr("nanobot.cron.service.CronService", _FakeCron)
|
||||
monkeypatch.setattr("nanobot.agent.loop.AgentLoop", _FakeAgentLoop)
|
||||
monkeypatch.setattr("nanobot.cli.commands.AgentLoop", _FakeAgentLoop)
|
||||
monkeypatch.setattr("nanobot.cli.commands._print_agent_response", lambda *_args, **_kwargs: None)
|
||||
|
||||
result = runner.invoke(
|
||||
@@ -806,7 +822,7 @@ def test_agent_custom_config_workspace_does_not_migrate_legacy_cron(
|
||||
monkeypatch.setattr("nanobot.config.loader.set_config_path", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.config.loader.load_config", lambda _path=None: config)
|
||||
monkeypatch.setattr("nanobot.cli.commands.sync_workspace_templates", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.cli.commands._make_provider", lambda _config: object())
|
||||
monkeypatch.setattr("nanobot.providers.factory.make_provider", lambda _config: _fake_provider())
|
||||
monkeypatch.setattr("nanobot.bus.queue.MessageBus", lambda: object())
|
||||
monkeypatch.setattr("nanobot.config.paths.get_cron_dir", lambda: legacy_dir)
|
||||
|
||||
@@ -815,6 +831,9 @@ def test_agent_custom_config_workspace_does_not_migrate_legacy_cron(
|
||||
seen["cron_store"] = store_path
|
||||
|
||||
class _FakeAgentLoop:
|
||||
@classmethod
|
||||
def from_config(cls, config, bus=None, **extra):
|
||||
return cls(**extra)
|
||||
def __init__(self, *args, **kwargs) -> None:
|
||||
pass
|
||||
|
||||
@@ -825,7 +844,7 @@ def test_agent_custom_config_workspace_does_not_migrate_legacy_cron(
|
||||
return None
|
||||
|
||||
monkeypatch.setattr("nanobot.cron.service.CronService", _FakeCron)
|
||||
monkeypatch.setattr("nanobot.agent.loop.AgentLoop", _FakeAgentLoop)
|
||||
monkeypatch.setattr("nanobot.cli.commands.AgentLoop", _FakeAgentLoop)
|
||||
monkeypatch.setattr(
|
||||
"nanobot.cli.commands._print_agent_response", lambda *_args, **_kwargs: None
|
||||
)
|
||||
@@ -846,7 +865,8 @@ def test_agent_overrides_workspace_path(mock_agent_runtime):
|
||||
assert result.exit_code == 0
|
||||
assert mock_agent_runtime["config"].agents.defaults.workspace == str(workspace_path)
|
||||
assert mock_agent_runtime["sync_templates"].call_args.args == (workspace_path,)
|
||||
assert mock_agent_runtime["agent_loop_cls"].call_args.kwargs["workspace"] == workspace_path
|
||||
passed_config = mock_agent_runtime["from_config"].call_args.args[0]
|
||||
assert passed_config.workspace_path == workspace_path
|
||||
|
||||
|
||||
def test_agent_workspace_override_wins_over_config_workspace(mock_agent_runtime, tmp_path: Path):
|
||||
@@ -863,7 +883,8 @@ def test_agent_workspace_override_wins_over_config_workspace(mock_agent_runtime,
|
||||
assert mock_agent_runtime["load_config"].call_args.args == (config_path.resolve(),)
|
||||
assert mock_agent_runtime["config"].agents.defaults.workspace == str(workspace_path)
|
||||
assert mock_agent_runtime["sync_templates"].call_args.args == (workspace_path,)
|
||||
assert mock_agent_runtime["agent_loop_cls"].call_args.kwargs["workspace"] == workspace_path
|
||||
passed_config = mock_agent_runtime["from_config"].call_args.args[0]
|
||||
assert passed_config.workspace_path == workspace_path
|
||||
|
||||
|
||||
def test_agent_hints_about_deprecated_memory_window(mock_agent_runtime, tmp_path):
|
||||
@@ -915,7 +936,7 @@ def _patch_cli_command_runtime(
|
||||
cron_service=None,
|
||||
get_cron_dir=None,
|
||||
) -> None:
|
||||
provider_factory = make_provider or (lambda _config: object())
|
||||
provider_factory = make_provider or (lambda _config: _fake_provider())
|
||||
|
||||
monkeypatch.setattr(
|
||||
"nanobot.config.loader.set_config_path",
|
||||
@@ -928,7 +949,7 @@ def _patch_cli_command_runtime(
|
||||
sync_templates or (lambda _path: None),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"nanobot.cli.commands._make_provider",
|
||||
"nanobot.providers.factory.make_provider",
|
||||
provider_factory,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
@@ -959,6 +980,9 @@ def _patch_serve_runtime(monkeypatch, config: Config, seen: dict[str, object]) -
|
||||
self.on_cleanup: list[object] = []
|
||||
|
||||
class _FakeAgentLoop:
|
||||
@classmethod
|
||||
def from_config(cls, config, bus=None, **extra):
|
||||
return cls(workspace=config.workspace_path, **extra)
|
||||
def __init__(self, **kwargs) -> None:
|
||||
seen["workspace"] = kwargs["workspace"]
|
||||
|
||||
@@ -985,7 +1009,7 @@ def _patch_serve_runtime(monkeypatch, config: Config, seen: dict[str, object]) -
|
||||
message_bus=lambda: object(),
|
||||
session_manager=lambda _workspace: object(),
|
||||
)
|
||||
monkeypatch.setattr("nanobot.agent.loop.AgentLoop", _FakeAgentLoop)
|
||||
monkeypatch.setattr("nanobot.cli.commands.AgentLoop", _FakeAgentLoop)
|
||||
monkeypatch.setattr("nanobot.api.server.create_app", _fake_create_app)
|
||||
monkeypatch.setattr("aiohttp.web.run_app", _fake_run_app)
|
||||
|
||||
@@ -1069,7 +1093,7 @@ def test_gateway_cron_evaluator_receives_scheduled_reminder_context(
|
||||
|
||||
config = Config()
|
||||
config.agents.defaults.workspace = str(tmp_path / "config-workspace")
|
||||
provider = object()
|
||||
provider = _fake_provider()
|
||||
bus = MagicMock()
|
||||
bus.publish_outbound = AsyncMock()
|
||||
seen: dict[str, object] = {}
|
||||
@@ -1077,7 +1101,7 @@ def test_gateway_cron_evaluator_receives_scheduled_reminder_context(
|
||||
monkeypatch.setattr("nanobot.config.loader.set_config_path", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.config.loader.load_config", lambda _path=None: config)
|
||||
monkeypatch.setattr("nanobot.cli.commands.sync_workspace_templates", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.cli.commands._make_provider", lambda _config: provider)
|
||||
monkeypatch.setattr("nanobot.providers.factory.make_provider", lambda _config: provider)
|
||||
monkeypatch.setattr(
|
||||
"nanobot.providers.factory.build_provider_snapshot",
|
||||
lambda _config: _test_provider_snapshot(provider, _config),
|
||||
@@ -1115,8 +1139,12 @@ def test_gateway_cron_evaluator_receives_scheduled_reminder_context(
|
||||
seen["cron"] = self
|
||||
|
||||
class _FakeAgentLoop:
|
||||
@classmethod
|
||||
def from_config(cls, config, bus=None, **extra):
|
||||
return cls(**extra)
|
||||
def __init__(self, *args, **kwargs) -> None:
|
||||
self.model = "test-model"
|
||||
self.provider = object()
|
||||
self.tools = {}
|
||||
|
||||
async def process_direct(self, *_args, **_kwargs):
|
||||
@@ -1152,7 +1180,7 @@ def test_gateway_cron_evaluator_receives_scheduled_reminder_context(
|
||||
return True
|
||||
|
||||
monkeypatch.setattr("nanobot.cron.service.CronService", _FakeCron)
|
||||
monkeypatch.setattr("nanobot.agent.loop.AgentLoop", _FakeAgentLoop)
|
||||
monkeypatch.setattr("nanobot.cli.commands.AgentLoop", _FakeAgentLoop)
|
||||
monkeypatch.setattr("nanobot.channels.manager.ChannelManager", _StopAfterCronSetup)
|
||||
monkeypatch.setattr(
|
||||
"nanobot.utils.evaluator.evaluate_response",
|
||||
@@ -1181,7 +1209,7 @@ def test_gateway_cron_evaluator_receives_scheduled_reminder_context(
|
||||
|
||||
assert response == "Time to stretch."
|
||||
assert seen["response"] == "Time to stretch."
|
||||
assert seen["provider"] is provider
|
||||
assert seen["provider"] is not None
|
||||
assert seen["model"] == "test-model"
|
||||
assert seen["task_context"] == (
|
||||
"The scheduled time has arrived. Deliver this reminder to the user now, "
|
||||
@@ -1228,7 +1256,7 @@ def test_gateway_cron_job_suppresses_intermediate_progress(
|
||||
monkeypatch.setattr("nanobot.config.loader.set_config_path", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.config.loader.load_config", lambda _path=None: config)
|
||||
monkeypatch.setattr("nanobot.cli.commands.sync_workspace_templates", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.cli.commands._make_provider", lambda _config: object())
|
||||
monkeypatch.setattr("nanobot.providers.factory.make_provider", lambda _config: _fake_provider())
|
||||
monkeypatch.setattr(
|
||||
"nanobot.providers.factory.build_provider_snapshot",
|
||||
lambda _config: _test_provider_snapshot(object(), _config),
|
||||
@@ -1246,8 +1274,12 @@ def test_gateway_cron_job_suppresses_intermediate_progress(
|
||||
seen["cron"] = self
|
||||
|
||||
class _FakeAgentLoop:
|
||||
@classmethod
|
||||
def from_config(cls, config, bus=None, **extra):
|
||||
return cls(**extra)
|
||||
def __init__(self, *args, **kwargs) -> None:
|
||||
self.model = "test-model"
|
||||
self.provider = object()
|
||||
self.tools = {}
|
||||
|
||||
async def process_direct(self, *_args, on_progress=None, **_kwargs):
|
||||
@@ -1275,7 +1307,7 @@ def test_gateway_cron_job_suppresses_intermediate_progress(
|
||||
return False
|
||||
|
||||
monkeypatch.setattr("nanobot.cron.service.CronService", _FakeCron)
|
||||
monkeypatch.setattr("nanobot.agent.loop.AgentLoop", _FakeAgentLoop)
|
||||
monkeypatch.setattr("nanobot.cli.commands.AgentLoop", _FakeAgentLoop)
|
||||
monkeypatch.setattr("nanobot.channels.manager.ChannelManager", _StopAfterCronSetup)
|
||||
monkeypatch.setattr(
|
||||
"nanobot.utils.evaluator.evaluate_response",
|
||||
@@ -1478,8 +1510,12 @@ def test_gateway_health_endpoint_binds_and_serves_expected_responses(
|
||||
return 0
|
||||
|
||||
class _FakeAgentLoop:
|
||||
@classmethod
|
||||
def from_config(cls, config, bus=None, **extra):
|
||||
return cls(**extra)
|
||||
def __init__(self, **_kwargs) -> None:
|
||||
self.model = "test-model"
|
||||
self.provider = object()
|
||||
self.dream = _FakeDream()
|
||||
self.sessions = _FakeSessionManager()
|
||||
|
||||
@@ -1571,7 +1607,7 @@ def test_gateway_health_endpoint_binds_and_serves_expected_responses(
|
||||
message_bus=lambda: object(),
|
||||
session_manager=lambda _workspace: object(),
|
||||
)
|
||||
monkeypatch.setattr("nanobot.agent.loop.AgentLoop", _FakeAgentLoop)
|
||||
monkeypatch.setattr("nanobot.cli.commands.AgentLoop", _FakeAgentLoop)
|
||||
monkeypatch.setattr("nanobot.channels.manager.ChannelManager", _FakeChannelManager)
|
||||
monkeypatch.setattr("nanobot.cron.service.CronService", _FakeCronService)
|
||||
monkeypatch.setattr("nanobot.heartbeat.service.HeartbeatService", _FakeHeartbeatService)
|
||||
|
||||
Reference in New Issue
Block a user