fix(review): address critical architectural issues from reviewer feedback

- Fix CLI routing regression in __main__.py - replace isidentifier() with proper YAML/prompt detection
- Fix thread safety races in integrations/registry.py - add proper instance-level locking
- Fix critical NameError in auto.py - migrate remaining lazy-loading functions to _load_optional
- Fix dead code in _async_bridge.py - remove unused variable, restore running-loop guard
- Fix loop safety in async_agent_scheduler.py - add loop consistency check in stop()
- Fix exception handling in framework_adapters/registry.py - improve specificity and logging
- Remove unused lru_cache import in auto.py

Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>

Author: praisonai-triage-agent[bot]

src/praisonai/praisonai/__main__.py

@@ -18,18 +18,27 @@ def _is_legacy_invocation(argv: list[str]) -> bool:
     """Check if this is a bare prompt or bare YAML invocation.
     
     Legacy invocations are:
-    - Bare YAML file: "agents.yaml"
+    - Bare YAML file: "agents.yaml" 
     - Free-text prompt: "Create a weather app"
     
     All other invocations should be handled by Typer commands.
     """
-    for arg in argv:
-        if arg.startswith("-"):
-            continue
-        # Check if it's a YAML file or contains spaces (free-text prompt)
-        return (arg.endswith((".yaml", ".yml")) or 
-                " " in arg or 
-                not arg.isidentifier())
+    import os
+    
+    # Only the very first positional token is considered; option values never are.
+    if not argv or argv[0].startswith("-"):
+        return False
+    
+    first = argv[0]
+    
+    # Check for free-text prompt (contains spaces)
+    if " " in first:
+        return True
+        
+    # Check for YAML file that actually exists on disk
+    if first.endswith((".yaml", ".yml")) and os.path.isfile(first):
+        return True
+        
     return False
 
 

src/praisonai/praisonai/_async_bridge.py

@@ -75,12 +75,10 @@ async def _cancel_all() -> None:
 
 def run_sync(coro: Awaitable[T], *, timeout: float | None = _DEFAULT_TIMEOUT) -> T:
     """
-    Run a coroutine synchronously, safe inside a running loop.
+    Run a coroutine synchronously using the background loop.
     
-    This function automatically detects if there's already a running event loop
-    and handles the execution appropriately:
-    - If no loop is running: uses background loop (consistent behavior)
-    - If a loop is running: schedules on background loop (safe path)
+    IMPORTANT: This function cannot be called from within a running event loop
+    as it would cause deadlock. Use 'await coro' directly from async contexts.
     
     Args:
         coro: The coroutine to run
@@ -90,20 +88,24 @@ def run_sync(coro: Awaitable[T], *, timeout: float | None = _DEFAULT_TIMEOUT) ->
         The result of the coroutine
         
     Raises:
+        RuntimeError: If called from within a running event loop
         TimeoutError: If timeout is exceeded
         Any exception raised by the coroutine
     """
     try:
         asyncio.get_running_loop()
-        running = True
     except RuntimeError:
-        running = False
+        pass
+    else:
+        raise RuntimeError(
+            "run_sync() cannot be called from a running event loop; "
+            "await the coroutine directly instead."
+        )
 
     # Submit the coroutine inside the lock to prevent shutdown races
     with _BG._lock:
-        loop = _BG.get_unlocked()  # get loop while holding lock
+        loop = _BG.get_unlocked()
         fut: Future = asyncio.run_coroutine_threadsafe(coro, loop)
-    
     return fut.result(timeout=timeout)
 
 

src/praisonai/praisonai/async_agent_scheduler.py

@@ -183,12 +183,30 @@ async def stop(self) -> bool:
         """
         Stop the scheduler gracefully with proper cancellation.
         
+        IMPORTANT: This method must be called from the same event loop
+        that was used to start the scheduler.
+        
         Returns:
             True if stopped successfully
+            
+        Raises:
+            RuntimeError: If called from a different event loop than start()
         """
         if not self._is_running:
             logger.info("Scheduler is not running")
             return True
+        
+        # Ensure we're on the same loop that was bound during start()
+        try:
+            current_loop = asyncio.get_running_loop()
+            if self._bound_loop is not None and current_loop is not self._bound_loop:
+                raise RuntimeError(
+                    "stop() must be called from the same event loop as start(). "
+                    f"Expected: {self._bound_loop}, got: {current_loop}"
+                )
+        except RuntimeError:
+            # No running loop - this is fine if scheduler was never started
+            pass
 
         logger.info("Stopping async agent scheduler...")
         self._cancel_event.set()

src/praisonai/praisonai/auto.py

@@ -26,7 +26,6 @@
 # =============================================================================
 
 import threading
-from functools import lru_cache
 
 # Thread-safe lazy cache for optional dependencies
 _optional_lock = threading.Lock()
@@ -173,48 +172,38 @@ def tools_loader():
 # --- LiteLLM lazy loading ---
 def _check_litellm_available() -> bool:
     """Check if litellm is available (cached)."""
-    global _litellm_available
-    if _litellm_available is None:
-        try:
-            import litellm  # noqa: F401
-            _litellm_available = True
-        except ImportError:
-            _litellm_available = False
-    return _litellm_available
+    result = _load_optional("litellm")
+    return result is not None
 
 
 def _get_litellm():
     """Lazy load litellm module."""
-    global _litellm
-    if _litellm is None:
-        import litellm as _litellm_module
-        _litellm = _litellm_module
-    return _litellm
+    result = _load_optional("litellm")
+    if result is None:
+        raise ImportError("Install with: pip install litellm")
+    return result
 
 
 # --- OpenAI lazy loading ---
 def _check_openai_available() -> bool:
     """Check if openai is available (cached)."""
-    global _openai_available
-    if _openai_available is None:
-        try:
-            import openai  # noqa: F401
-            _openai_available = True
-        except ImportError:
-            _openai_available = False
-    return _openai_available
+    result = _load_optional("openai")
+    return result is not None
 
 
 def _get_openai_client(api_key: str = None, base_url: str = None):
     """Lazy load OpenAI client."""
-    global _openai_client
-    if _openai_client is None:
+    def create_openai_client():
         from openai import OpenAI
-        _openai_client = OpenAI(
+        return OpenAI(
             api_key=api_key or os.environ.get("OPENAI_API_KEY"),
             base_url=base_url
         )
-    return _openai_client
+    
+    result = _load_optional("openai_client", create_openai_client)
+    if result is None:
+        raise ImportError("Install with: pip install openai")
+    return result
 
 
 _loglevel = os.environ.get('LOGLEVEL', 'INFO').strip().upper() or 'INFO'

src/praisonai/praisonai/framework_adapters/registry.py

@@ -167,6 +167,11 @@ def is_available(self, name: str) -> bool:
         """
         try:
             adapter = self.create(name)
+        except ValueError:
+            return False
+        
+        try:
             return adapter.is_available()
-        except (ValueError, Exception):
+        except Exception:
+            logger.warning("is_available() raised for adapter %r", name, exc_info=True)
             return False

src/praisonai/praisonai/integrations/registry.py

@@ -48,6 +48,7 @@ class ExternalAgentRegistry:
     def __init__(self):
         """Initialize the registry with built-in integrations."""
         self._integrations: Dict[str, Type[BaseCLIIntegration]] = {}
+        self._lock = threading.Lock()
         self._register_builtin_integrations()
 
     @classmethod
@@ -108,7 +109,7 @@ def register(self, name: str, integration_class: Type[BaseCLIIntegration]) -> No
             )
 
         # Thread-safe registration
-        with self._instance_lock:
+        with self._lock:
             self._integrations[name] = integration_class
 
     def unregister(self, name: str) -> bool:
@@ -122,11 +123,8 @@ def unregister(self, name: str) -> bool:
             bool: True if the integration was found and removed, False otherwise
         """
         # Thread-safe unregistration with atomic check-then-delete
-        with self._instance_lock:
-            if name in self._integrations:
-                del self._integrations[name]
-                return True
-            return False
+        with self._lock:
+            return self._integrations.pop(name, None) is not None
 
     def create(self, name: str, **kwargs: Any) -> Optional[BaseCLIIntegration]:
         """
@@ -139,7 +137,9 @@ def create(self, name: str, **kwargs: Any) -> Optional[BaseCLIIntegration]:
         Returns:
             BaseCLIIntegration: Instance of the integration, or None if not found
         """
-        integration_class = self._integrations.get(name)
+        with self._lock:
+            integration_class = self._integrations.get(name)
+        
         if integration_class is None:
             return None
 
@@ -152,7 +152,8 @@ def list_registered(self) -> List[str]:
         Returns:
             List[str]: List of registered integration names
         """
-        return list(self._integrations.keys())
+        with self._lock:
+            return list(self._integrations.keys())
 
     async def get_available(self) -> Dict[str, bool]:
         """
@@ -164,7 +165,10 @@ async def get_available(self) -> Dict[str, bool]:
         import inspect
         availability = {}
 
-        for name, integration_class in self._integrations.items():
+        with self._lock:
+            snapshot = list(self._integrations.items())
+        
+        for name, integration_class in snapshot:
             try:
                 # Check if constructor requires parameters beyond self
                 sig = inspect.signature(integration_class.__init__)