fix: resolve architectural gaps in src/praisonai/praisonai (#1658)

* fix: resolve architectural gaps in src/praisonai/praisonai (fixes #1657)

- Fix framework adapter dispatch kwargs mismatch by adding required keyword-only arguments to AutoGenAdapter, AutoGenV4Adapter, and AG2Adapter run() methods
- Fix InteractiveRuntime lifecycle management by keeping runtime alive during agent execution instead of stopping it prematurely
- Consolidate tools.py loading paths by making ToolResolver the single source of truth and replacing duplicate loading logic in agents_generator.py
- Remove unused load_tools_from_tools_py() method
- Update ToolResolver to use _safe_loader for consistent security model

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

* fix: define tools_path in ToolResolver local loader logging

Agent-Logs-Url: https://github.com/MervinPraison/PraisonAI/sessions/bee41da9-2153-41cd-9834-439aa753a848

Co-authored-by: MervinPraison <454862+MervinPraison@users.noreply.github.com>

* fix: resolve critical NameError and event loop leak

- Fix NameError in ToolResolver._load_local_tools() for undefined tools_path variable
- Fix event loop leak in AgentsGenerator when InteractiveRuntime startup fails
- Change autogen_v4/ag2 adapters to raise NotImplementedError instead of returning strings

🤖 Generated with Claude Code

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

---------

Co-authored-by: praisonai-triage-agent[bot] <272766704+praisonai-triage-agent[bot]@users.noreply.github.com>
Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: MervinPraison <454862+MervinPraison@users.noreply.github.com>

Author: 3 people

src/praisonai/praisonai/agents_generator.py

@@ -448,36 +448,6 @@ def load_tools_from_package(self, package_path):
                     tools_dict[name] = obj
         return tools_dict
 
-    def load_tools_from_tools_py(self):
-        """
-        Imports and returns all contents from tools.py file.
-        Uses the tool registry instead of global namespace pollution.
-
-        Returns:
-            list: A list of callable functions with proper formatting
-        """
-        tools_list = []
-        try:
-            # Try to import tools.py from current directory using safe loading
-            from ._safe_loader import load_user_module
-            module = load_user_module("tools.py", name="tools")
-            if module is None:
-                self.logger.debug("tools.py not found or local tools loading disabled")
-                return tools_list
-
-            # Register functions in the tool registry instead of globals()
-            registered_tools = self.tool_registry.register_from_module(module)
-            tools_list = [self.tool_registry.get_function(name) for name in registered_tools]
-
-            self.logger.debug(f"Loaded {len(tools_list)} tool functions from tools.py")
-            self.logger.debug(f"Registered tools: {registered_tools}")
-            
-        except FileNotFoundError:
-            self.logger.debug("tools.py not found in current directory")
-        except Exception as e:
-            self.logger.warning(f"Error loading tools from tools.py: {e}")
-            
-        return tools_list
 
     def generate_crew_and_kickoff(self):
         """
@@ -587,12 +557,10 @@ def generate_crew_and_kickoff(self):
         tools_py_path = os.path.join(root_directory, 'tools.py')
         tools_dir_path = Path(root_directory) / 'tools'
 
+        # Use consolidated ToolResolver for tools.py loading
+        tools_dict.update(self.tool_resolver.get_local_tool_classes())
         if os.path.isfile(tools_py_path):
-            from ._safe_loader import load_user_module
-            module = load_user_module(tools_py_path, name="tools_module")
-            if module is not None:
-                tools_dict.update(self._extract_tool_classes(module))
-                self.logger.debug("tools.py exists in the root directory. Loading tools.py and skipping tools folder.")
+            self.logger.debug("tools.py exists in the root directory. Loading tools.py and skipping tools folder.")
         elif tools_dir_path.is_dir():
             from ._safe_loader import load_user_module
             for py_file in tools_dir_path.glob("*.py"):
@@ -1195,8 +1163,8 @@ def _run_praisonai(self, config, topic, tools_dict):
 
         # Use existing tool resolver instance
 
-        # Load tools from local tools.py (backward compat)
-        tools_list = self.load_tools_from_tools_py()
+        # Load tools from local tools.py (backward compat) - use consolidated ToolResolver
+        tools_list = self.tool_resolver.get_local_callables()
         self.logger.debug(f"Loaded tools from tools.py: {tools_list}")
 
         # Initialize InteractiveRuntime for ACP/LSP if enabled globally
@@ -1224,25 +1192,24 @@ def _run_praisonai(self, config, topic, tools_dict):
 
                 # Create a scoped event loop instead of modifying process globals
                 interactive_loop = asyncio.new_event_loop()
-                try:
-                    interactive_loop.run_until_complete(interactive_runtime.start())
-                    
-                    centric_tools = create_agent_centric_tools(interactive_runtime)
-                    self.logger.info(f"Loaded {len(centric_tools)} InteractiveRuntime tools")
-                    tools_list.extend(centric_tools)
-                    
-                finally:
-                    try:
-                        interactive_loop.run_until_complete(interactive_runtime.stop())
-                    except Exception as stop_error:
-                        self.logger.warning(f"Error stopping InteractiveRuntime: {stop_error}")
-                    finally:
-                        interactive_loop.close()
+                
+                # Start the runtime but keep it alive for agent execution
+                interactive_loop.run_until_complete(interactive_runtime.start())
+                
+                centric_tools = create_agent_centric_tools(interactive_runtime)
+                self.logger.info(f"Loaded {len(centric_tools)} InteractiveRuntime tools")
+                tools_list.extend(centric_tools)
 
             except ImportError as e:
                 self.logger.warning(f"Failed to load InteractiveRuntime components: {e}")
+                interactive_runtime = None
+                interactive_loop = None
             except Exception as e:
                 self.logger.error(f"Error starting InteractiveRuntime: {e}")
+                if 'interactive_loop' in locals() and interactive_loop is not None:
+                    interactive_loop.close()
+                interactive_runtime = None
+                interactive_loop = None
 
         # Create agents from config
         for role, details in config['roles'].items():
@@ -1482,6 +1449,8 @@ def _run_praisonai(self, config, topic, tools_dict):
                     interactive_loop.run_until_complete(interactive_runtime.stop())
                 except Exception as e:
                     self.logger.error(f"Error stopping InteractiveRuntime: {e}")
+                finally:
+                    interactive_loop.close()
 
         if AGENTOPS_AVAILABLE:
             import agentops

src/praisonai/praisonai/framework_adapters/autogen_adapter.py

@@ -5,7 +5,7 @@
 """
 
 import logging
-from typing import Dict, List, Any
+from typing import Dict, List, Any, Optional, Callable
 from .base import BaseFrameworkAdapter
 
 logger = logging.getLogger(__name__)
@@ -26,14 +26,28 @@ def is_available(self) -> bool:
         except ImportError:
             return False
 
-    def run(self, config: Dict[str, Any], llm_config: List[Dict], topic: str) -> str:
+    def run(
+        self,
+        config: Dict[str, Any],
+        llm_config: List[Dict],
+        topic: str,
+        *,
+        tools_dict: Optional[Dict[str, Any]] = None,
+        agent_callback: Optional[Callable] = None,
+        task_callback: Optional[Callable] = None,
+        cli_config: Optional[Dict[str, Any]] = None,
+    ) -> str:
         """
         Run AutoGen v0.2 with given configuration.
         
         Args:
             config: AutoGen configuration with agents
             llm_config: LLM configuration list
             topic: Topic for the tasks
+            tools_dict: Available tools dictionary
+            agent_callback: Callback for agent events
+            task_callback: Callback for task events
+            cli_config: CLI configuration
             
         Returns:
             Execution result as string
@@ -110,14 +124,28 @@ def is_available(self) -> bool:
         except ImportError:
             return False
 
-    def run(self, config: Dict[str, Any], llm_config: List[Dict], topic: str) -> str:
+    def run(
+        self,
+        config: Dict[str, Any],
+        llm_config: List[Dict],
+        topic: str,
+        *,
+        tools_dict: Optional[Dict[str, Any]] = None,
+        agent_callback: Optional[Callable] = None,
+        task_callback: Optional[Callable] = None,
+        cli_config: Optional[Dict[str, Any]] = None,
+    ) -> str:
         """
         Run AutoGen v0.4 with given configuration.
         
         Args:
             config: AutoGen v0.4 configuration with agents
             llm_config: LLM configuration list
             topic: Topic for the tasks
+            tools_dict: Available tools dictionary
+            agent_callback: Callback for agent events
+            task_callback: Callback for task events
+            cli_config: CLI configuration
             
         Returns:
             Execution result as string
@@ -128,7 +156,7 @@ def run(self, config: Dict[str, Any], llm_config: List[Dict], topic: str) -> str
         # For now, return a proper error message instead of delegating
         # TODO: Implement full AutoGen v0.4 adapter logic
         logger.warning("AutoGen v0.4 adapter is not yet fully implemented")
-        return "### AutoGen v0.4 Output ###\nAutoGen v0.4 adapter is not yet fully implemented. Please use 'autogen' framework for AutoGen v0.2 support."
+        raise NotImplementedError("AutoGen v0.4 adapter is not yet fully implemented. Please use 'autogen' framework for AutoGen v0.2 support.")
 
 
 class AG2Adapter(BaseFrameworkAdapter):
@@ -148,14 +176,28 @@ def is_available(self) -> bool:
         except Exception:
             return False
 
-    def run(self, config: Dict[str, Any], llm_config: List[Dict], topic: str) -> str:
+    def run(
+        self,
+        config: Dict[str, Any],
+        llm_config: List[Dict],
+        topic: str,
+        *,
+        tools_dict: Optional[Dict[str, Any]] = None,
+        agent_callback: Optional[Callable] = None,
+        task_callback: Optional[Callable] = None,
+        cli_config: Optional[Dict[str, Any]] = None,
+    ) -> str:
         """
         Run AG2 with given configuration.
         
         Args:
             config: AG2 configuration with agents
             llm_config: LLM configuration list  
             topic: Topic for the tasks
+            tools_dict: Available tools dictionary
+            agent_callback: Callback for agent events
+            task_callback: Callback for task events
+            cli_config: CLI configuration
             
         Returns:
             Execution result as string
@@ -166,4 +208,4 @@ def run(self, config: Dict[str, Any], llm_config: List[Dict], topic: str) -> str
         # For now, return a proper error message instead of delegating
         # TODO: Implement full AG2 adapter logic
         logger.warning("AG2 adapter is not yet fully implemented")
-        return "### AG2 Output ###\nAG2 adapter is not yet fully implemented. Please use 'autogen' framework for AutoGen/AG2 support."
+        raise NotImplementedError("AG2 adapter is not yet fully implemented. Please use 'autogen' framework for AutoGen/AG2 support.")

src/praisonai/praisonai/tool_resolver.py

@@ -30,6 +30,7 @@
 from pathlib import Path
 from typing import Any, Callable, Dict, List, Mapping, Optional
 from types import MappingProxyType
+from ._safe_loader import load_user_module
 
 logger = logging.getLogger(__name__)
 
@@ -74,31 +75,16 @@ def _load_local_tools(self) -> Mapping[str, Callable]:
             if self._local_tools_loaded:  # Double-check inside lock
                 return self._local_tools_cache
 
-            # Security: Require explicit opt-in for local tools loading
-            if os.environ.get("PRAISONAI_ALLOW_LOCAL_TOOLS", "").lower() != "true":
-                logger.debug("Local tools loading disabled. Set PRAISONAI_ALLOW_LOCAL_TOOLS=true to enable.")
-                self._local_tools_cache = MappingProxyType({})
-                self._local_tools_loaded = True
-                return self._local_tools_cache
-            
             tools_path = Path(self._tools_py_path)
-            if not tools_path.exists():
-                logger.debug(f"No local tools.py found at {tools_path}")
-                self._local_tools_cache = MappingProxyType({})
-                self._local_tools_loaded = True
-                return self._local_tools_cache
-            
             try:
-                spec = importlib.util.spec_from_file_location("tools", str(tools_path))
-                if spec is None or spec.loader is None:
-                    logger.warning(f"Could not load spec for {tools_path}")
+                # Use the same safe loader as other tools.py loading paths
+                module = load_user_module(self._tools_py_path, name="tools")
+                if module is None:
+                    logger.debug(f"Local tools loading disabled or tools.py not found at {self._tools_py_path}")
                     self._local_tools_cache = MappingProxyType({})
                     self._local_tools_loaded = True
                     return self._local_tools_cache
 
-                module = importlib.util.module_from_spec(spec)
-                spec.loader.exec_module(module)
-                
                 # Build cache locally, then freeze
                 cache: Dict[str, Callable] = {}
                 for name, obj in inspect.getmembers(module):
@@ -108,13 +94,13 @@ def _load_local_tools(self) -> Mapping[str, Callable]:
                         cache[name] = obj
                         logger.debug(f"Loaded local tool: {name}")
 
-                logger.info(f"Loaded {len(cache)} tools from {tools_path}")
+                logger.info(f"Loaded {len(cache)} tools from {self._tools_py_path}")
 
                 # Create immutable view to prevent concurrent modification
                 self._local_tools_cache = MappingProxyType(cache)
 
             except Exception as e:
-                logger.warning(f"Error loading tools from {tools_path}: {e}")
+                logger.warning(f"Error loading tools from {self._tools_py_path}: {e}")
                 self._local_tools_cache = MappingProxyType({})
 
             self._local_tools_loaded = True
@@ -381,6 +367,58 @@ def clear_cache(self) -> None:
         with self._local_tools_lock:
             self._local_tools_cache = MappingProxyType({})
             self._local_tools_loaded = False
+    
+    def get_local_callables(self) -> List[Callable]:
+        """Get functions exposed by tools.py (path A semantics).
+        
+        Returns:
+            List of callable functions from tools.py
+        """
+        local_tools = self._load_local_tools()
+        return list(local_tools.values())
+    
+    def get_local_tool_classes(self) -> Dict[str, Any]:
+        """Get BaseTool/langchain class instances from tools.py (path B semantics).
+        
+        Returns:
+            Dictionary mapping class names to instantiated tool objects
+        """
+        try:
+            # Use the same safe loader to get the module
+            module = load_user_module(self._tools_py_path, name="tools_module")
+            if module is None:
+                return {}
+            
+            # Import the necessary classes (matching agents_generator.py logic)
+            BaseTool = None
+            PRAISONAI_TOOLS_AVAILABLE = False
+            try:
+                from praisonai_tools import BaseTool
+                PRAISONAI_TOOLS_AVAILABLE = True
+            except ImportError:
+                try:
+                    from praisonai.tools import BaseTool
+                    PRAISONAI_TOOLS_AVAILABLE = True
+                except ImportError:
+                    pass
+            
+            result = {}
+            for name, obj in inspect.getmembers(module, 
+                lambda x: inspect.isclass(x) and (
+                    x.__module__.startswith('langchain_community.tools') or 
+                    (PRAISONAI_TOOLS_AVAILABLE and BaseTool and issubclass(x, BaseTool))
+                ) and x is not BaseTool):
+                try:
+                    result[name] = obj()
+                    logger.debug(f"Loaded local tool class: {name}")
+                except Exception as e:
+                    logger.warning(f"Error instantiating tool class {name}: {e}")
+                    continue
+            
+            return result
+        except Exception as e:
+            logger.warning(f"Error loading tool classes from {self._tools_py_path}: {e}")
+            return {}
 
 
 # Convenience functions that construct resolver explicitly (no global singleton)