From 733b34d6854fe8551374aed7f2d48a239131b201 Mon Sep 17 00:00:00 2001
From: chengyongru <chengyongru.ai@gmail.com>
Date: Sat, 9 May 2026 14:34:11 +0800
Subject: [PATCH] refactor: address code review feedback on
 AgentLoop.from_config()

- Accept optional `provider` kwarg in from_config() to avoid double
  instantiation in _run_gateway (which already builds provider_snapshot)
- Restore try/except ValueError wrappers in serve() and agent() for
  clean error messages on provider creation failure
- Update test: _FakeAgentLoop captures provider from kwargs, restore
  strong assertion (seen["provider"] is provider)
---
 nanobot/agent/loop.py      |  9 +++++++--
 nanobot/cli/commands.py    | 33 +++++++++++++++++++++------------
 tests/cli/test_commands.py |  4 ++--
 3 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py
index a26cb3272..b22332a30 100644
--- a/nanobot/agent/loop.py
+++ b/nanobot/agent/loop.py
@@ -348,13 +348,18 @@ class AgentLoop:
         bus: MessageBus | None = None,
         **extra: Any,
     ) -> AgentLoop:
-        """Create an AgentLoop from config with the common parameter set."""
+        """Create an AgentLoop from config with the common parameter set.
+
+        Extra keyword arguments are forwarded to ``AgentLoop.__init__``,
+        allowing callers to override or extend the standard config-derived
+        parameters (e.g. ``cron_service``, ``session_manager``).
+        """
         from nanobot.providers.factory import make_provider
 
         if bus is None:
             bus = MessageBus()
         defaults = config.agents.defaults
-        provider = make_provider(config)
+        provider = extra.pop("provider", None) or make_provider(config)
         return cls(
             bus=bus,
             provider=provider,
diff --git a/nanobot/cli/commands.py b/nanobot/cli/commands.py
index 1a7e05c88..23ed6ab1d 100644
--- a/nanobot/cli/commands.py
+++ b/nanobot/cli/commands.py
@@ -543,14 +543,18 @@ def serve(
     sync_workspace_templates(runtime_config.workspace_path)
     bus = MessageBus()
     session_manager = SessionManager(runtime_config.workspace_path)
-    agent_loop = AgentLoop.from_config(
-        runtime_config, bus,
-        session_manager=session_manager,
-        image_generation_provider_configs={
-            "openrouter": runtime_config.providers.openrouter,
-            "aihubmix": runtime_config.providers.aihubmix,
-        },
-    )
+    try:
+        agent_loop = AgentLoop.from_config(
+            runtime_config, bus,
+            session_manager=session_manager,
+            image_generation_provider_configs={
+                "openrouter": runtime_config.providers.openrouter,
+                "aihubmix": runtime_config.providers.aihubmix,
+            },
+        )
+    except ValueError as exc:
+        console.print(f"[red]Error: {exc}[/red]")
+        raise typer.Exit(1) from exc
 
     model_name = runtime_config.agents.defaults.model
     console.print(f"{__logo__} Starting OpenAI-compatible API server")
@@ -650,6 +654,7 @@ def _run_gateway(
     # Create agent with cron service
     agent = AgentLoop.from_config(
         config, bus,
+        provider=provider_snapshot.provider,
         cron_service=cron,
         session_manager=session_manager,
         image_generation_provider_configs={
@@ -1025,10 +1030,14 @@ def agent(
     else:
         logger.disable("nanobot")
 
-    agent_loop = AgentLoop.from_config(
-        config, bus,
-        cron_service=cron,
-    )
+    try:
+        agent_loop = AgentLoop.from_config(
+            config, bus,
+            cron_service=cron,
+        )
+    except ValueError as exc:
+        console.print(f"[red]Error: {exc}[/red]")
+        raise typer.Exit(1) from exc
     restart_notice = consume_restart_notice_from_env()
     if restart_notice and should_show_cli_restart_notice(restart_notice, session_id):
         _print_agent_response(
diff --git a/tests/cli/test_commands.py b/tests/cli/test_commands.py
index 28813fb8f..b0c3c43ee 100644
--- a/tests/cli/test_commands.py
+++ b/tests/cli/test_commands.py
@@ -1144,7 +1144,7 @@ def test_gateway_cron_evaluator_receives_scheduled_reminder_context(
             return cls(**extra)
         def __init__(self, *args, **kwargs) -> None:
             self.model = "test-model"
-            self.provider = object()
+            self.provider = kwargs.get("provider", object())
             self.tools = {}
 
         async def process_direct(self, *_args, **_kwargs):
@@ -1209,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 not None
+    assert seen["provider"] is provider
     assert seen["model"] == "test-model"
     assert seen["task_context"] == (
         "The scheduled time has arrived. Deliver this reminder to the user now, "