Skip to content

Wrapper gaps: nested asyncio.run, hot-path logging.basicConfig, and fragmented approval/trust schema #1456

@MervinPraison

Description

@MervinPraison

Summary

In-depth analysis of src/praisonai/praisonai/ against the project philosophy (agent-centric, protocol-driven, performance-first, multi-agent + async safe, 3-way feature surface) surfaces three high-impact gaps that violate the stated principles. Each finding below was validated against the current code on main with file:line citations.


1. asyncio.run() invoked inside hot paths breaks multi-agent / nested-loop safety

Where (validated):

  • src/praisonai/praisonai/agents_generator.py:766_run_autogen_v4 defines an inner async def then calls asyncio.run(...) from a sync method.
  • src/praisonai/praisonai/bots/_approval_base.py:126-139sync_wrapper falls back to asyncio.run and even spawns a ThreadPoolExecutor per call when a loop is already running.
  • src/praisonai/praisonai/integrations/base.py:298-312as_tool() returns a sync wrapper that spins up a fresh event loop per tool call, with a ThreadPoolExecutor sidecar when nested.
  • src/praisonai/praisonai/endpoints/a2u_server.py:182, src/praisonai/praisonai/integrations/managed_local.py:358,373,442,632,647, src/praisonai/praisonai/integrations/registry.py:253,256 — same pattern repeated.

Current code (agents_generator.py:760-769):

            finally:
                # Close the model client
                await model_client.close()

        # Run the async function
        try:
            return asyncio.run(run_autogen_v4_async())
        except Exception as e:
            self.logger.error(f"Error running AutoGen v0.4: {str(e)}")
            return f"### AutoGen v0.4 Error ###\n{str(e)}"

Current code (bots/_approval_base.py:126-139):

def sync_wrapper(async_fn, timeout: float):
    """Run *async_fn* (a coroutine) synchronously, handling nested loops."""
    try:
        loop = asyncio.get_running_loop()
    except RuntimeError:
        loop = None

    if loop and loop.is_running():
        import concurrent.futures
        with concurrent.futures.ThreadPoolExecutor(max_workers=1) as pool:
            future = pool.submit(asyncio.run, async_fn)
            return future.result(timeout=timeout + 10)
    else:
        return asyncio.run(async_fn)

Why this is a problem:

  • asyncio.run() must own the event loop — calling it from inside an existing loop raises RuntimeError: asyncio.run() cannot be called from a running event loop. The "fix" used in several places (ThreadPoolExecutor + asyncio.run) avoids the error but spawns a thread + new loop per call, defeating the whole point of async and adding GIL/loop teardown overhead on every tool/approval invocation.
  • This directly violates the philosophy line "Multi-agent + async safe by default" and "Performance-first: no hot-path regressions."
  • It breaks the most common deployment surfaces: Jupyter, Chainlit, Gradio, FastAPI, MCP stdio — all of which have an outer running loop.
  • Each call does loop creation + teardown, which is expensive enough to dominate latency on small tool calls.

Suggested fix (pattern, applies to all sites):

Make the hot path async-native and provide a single, central sync bridge that reuses a loop instead of constructing one per call.

# praisonai/_async_bridge.py  (new, single source of truth)
import asyncio
import threading
from concurrent.futures import Future
from typing import Awaitable, TypeVar

T = TypeVar("T")

_loop: asyncio.AbstractEventLoop | None = None
_loop_lock = threading.Lock()


def _ensure_background_loop() -> asyncio.AbstractEventLoop:
    global _loop
    with _loop_lock:
        if _loop is None or _loop.is_closed():
            _loop = asyncio.new_event_loop()
            t = threading.Thread(target=_loop.run_forever, name="praisonai-async", daemon=True)
            t.start()
        return _loop


def run_sync(coro: Awaitable[T], *, timeout: float | None = None) -> T:
    """Run *coro* synchronously, safe inside a running loop."""
    try:
        running = asyncio.get_running_loop()
    except RuntimeError:
        running = None

    if running is None:
        # Cheap path: no outer loop, just run.
        return asyncio.run(coro)

    # Outer loop exists -> schedule on background loop, do NOT nest asyncio.run.
    fut: Future = asyncio.run_coroutine_threadsafe(coro, _ensure_background_loop())
    return fut.result(timeout=timeout)

Refactor each site to either expose an async def public method (preferred) or call run_sync(...) once at the boundary:

# agents_generator.py — preferred: expose async + sync wrapper
async def _run_autogen_v4_async(self, config, topic, tools_dict):
    # ... move the entire body of run_autogen_v4_async here, no asyncio.run ...
    return result

def _run_autogen_v4(self, config, topic, tools_dict):
    from ._async_bridge import run_sync
    try:
        return run_sync(self._run_autogen_v4_async(config, topic, tools_dict))
    except Exception as e:
        self.logger.error(f"Error running AutoGen v0.4: {e}")
        return f"### AutoGen v0.4 Error ###\n{e}"
# integrations/base.py — replace per-call ThreadPoolExecutor + asyncio.run
def as_tool(self):
    integration = self
    def tool_func(query: str) -> str:
        from .._async_bridge import run_sync
        return run_sync(integration.execute(query))
    tool_func.__name__ = f"{self.cli_command}_tool"
    tool_func.__doc__  = f"Execute {self.cli_command} for coding tasks."
    return tool_func

This removes per-call loop creation, fixes nested-loop crashes, and keeps a single canonical bridge so the pattern is not re-invented in every module.


2. logging.basicConfig() runs in hot paths and on every AgentsGenerator instance

Where (validated):

  • src/praisonai/praisonai/agents_generator.py:181 — called inside AgentsGenerator.__init__, so once per agent-generator instance.
  • src/praisonai/praisonai/cli/main.py:224 — module-level call executed on every import praisonai.cli.main (which the CLI entrypoint touches early).

Current code (agents_generator.py:177-183):

        self.log_level = log_level or logging.getLogger().getEffectiveLevel()
        if self.log_level == logging.NOTSET:
            self.log_level = os.environ.get('LOGLEVEL', 'INFO').upper()

        logging.basicConfig(level=self.log_level, format='%(asctime)s - %(levelname)s - %(message)s')
        self.logger = logging.getLogger(__name__)
        self.logger.setLevel(self.log_level)

Current code (cli/main.py:224):

logging.basicConfig(level=os.environ.get('LOGLEVEL', 'WARNING') or 'WARNING', format='%(asctime)s - %(levelname)s - %(message)s')

Why this is a problem:

  • logging.basicConfig is a no-op only after the first call on the root logger, but it still walks handlers, acquires logging._lock, etc. on every call. In multi-agent runs we instantiate many AgentsGenerator objects (orchestration, sub-agents, retries) — that's lock contention and wasted work on the hot path.
  • It is a library-side mutation of the root logger. Embedders (Chainlit, Jupyter, FastAPI, downstream apps) lose their own handler/format the first time PraisonAI is imported. That violates "Production-ready / safe by default" and the wrapper layer's promise of "no heavy module-level work."
  • Because two different modules pick different defaults (INFO vs WARNING), whichever loads first wins — the behavior depends on import order, which is the canonical multi-agent footgun.

Suggested fix:

Configure logging exactly once, only at the CLI entrypoint, and only configure a namespaced logger from library code:

# praisonai/_logging.py  (new)
import logging
import os

_PKG_LOGGER = "praisonai"
_configured = False


def configure_cli_logging(level: str | int | None = None) -> None:
    """Configure root logging. Must only be called from the CLI entrypoint."""
    global _configured
    if _configured:
        return
    lvl = level or os.environ.get("LOGLEVEL", "WARNING")
    logging.basicConfig(level=lvl, format="%(asctime)s - %(levelname)s - %(message)s")
    _configured = True


def get_logger(name: str | None = None) -> logging.Logger:
    """Return a namespaced logger; never touches root."""
    return logging.getLogger(f"{_PKG_LOGGER}.{name}" if name else _PKG_LOGGER)
# cli/main.py — call exactly once, early
from .._logging import configure_cli_logging
configure_cli_logging()
# agents_generator.py — DO NOT call basicConfig from a constructor
from ._logging import get_logger

class AgentsGenerator:
    def __init__(self, ..., log_level=None, ...):
        self.logger = get_logger("agents_generator")
        if log_level:
            self.logger.setLevel(log_level)
        # NOTE: no logging.basicConfig() here

Net effect: zero root-logger mutation from library code, no per-instance work, and embedders keep their own logging configuration intact.


3. --trust / approval / guardrails configuration is fragmented across CLI, YAML, and Python — the 3-way feature surface is broken

Where (validated):

  • CLI flags: src/praisonai/praisonai/cli/main.py:1061-1069 (--trust, --approval, --approve-all-tools, --approval-timeout, --approve-level).
  • CLI→YAML mapping: src/praisonai/praisonai/agents_generator.py:241-260 (field_mappings = {'guardrail': 'guardrails', 'trust': 'approval'}, with --trust collapsed to approval=True).
  • YAML→Python reconstruction: src/praisonai/praisonai/agents_generator.py:1195-1232 (scatter/gather across approval, backend, backend_name, approve_all_tools, all_tools, approval_timeout, timeout).
  • Python API call site: src/praisonai/praisonai/agents_generator.py:1242-1258 (PraisonAgent(... approval=approval_config, guardrails=guardrails_config ...) — note: no trust parameter exists).

Current code (agents_generator.py:241-260):

        agent_level_fields = ['trust', 'tool_timeout', 'planning_tools', 'autonomy', 'guardrail', 'approval', 'approve_all_tools', 'approval_timeout']
        agent_overrides = {k: v for k, v in cli_config.items() if k in agent_level_fields}

        # Map CLI field names to YAML field names
        field_mappings = {
            'guardrail': 'guardrails',  # CLI uses --guardrail, YAML uses guardrails
            'trust': 'approval'         # --trust maps to approval=True
        }

        for cli_field in field_mappings:
            if cli_field in agent_overrides:
                value = agent_overrides.pop(cli_field)
                if cli_field == 'trust' and value:
                    agent_overrides['approval'] = True
                elif cli_field == 'guardrail':
                    agent_overrides['guardrails'] = value

Current code (agents_generator.py:1195-1219):

            # Reconstruct approval config from potentially scattered settings
            approval_val = details.get('approval')
            approve_all = details.get('approve_all_tools')
            approval_timeout = details.get('approval_timeout')

            approval_config = None
            if approval_val is not None or approve_all is not None or approval_timeout is not None:
                if isinstance(approval_val, dict):
                    approval_dict = approval_val
                else:
                    approval_dict = {'backend': approval_val}

                if approve_all is not None:
                    approval_dict['approve_all_tools'] = approve_all
                if approval_timeout is not None:
                    approval_dict['approval_timeout'] = approval_timeout

                try:
                    from .cli.features.approval import resolve_approval_config
                    approval_config = resolve_approval_config(
                        backend_name=approval_dict.get('backend') or approval_dict.get('backend_name'),
                        all_tools=approval_dict.get('approve_all_tools') or approval_dict.get('all_tools', False),
                        timeout=approval_dict.get('approval_timeout') or approval_dict.get('timeout')
                    )

Why this is a problem:

  • The philosophy mandates "Each feature runs 3 ways: CLI, YAML, Python" with a consistent, minimal API. Today:
    • CLI: --trust, --approval, --approve-all-tools, --approval-timeout, --approve-level, --guardrail.
    • YAML: approval (which can be bool or string or dict), approve_all_tools, approval_timeout, guardrails. Dict form silently accepts both backend/backend_name, both approve_all_tools/all_tools, both approval_timeout/timeout.
    • Python: there is no trust parameter at all; users have to construct ApprovalConfig themselves and pass it as approval=.
  • 'trust': 'approval' collapses a boolean into a polymorphic field. approval=True and approval="console" both end up working, but mean different things — and --approve-level is silently dropped because it isn't in agent_level_fields.
  • The fallback chains (backend or backend_name, approve_all_tools or all_tools) are a clear "no single source of truth" smell. A user typo (backed: instead of backend:) is silently accepted as "use defaults" — there is no schema validation. This violates "Simple to adopt, hard to misuse, safe by default."

Suggested fix:

Introduce a single canonical ApprovalSpec and have all three surfaces normalise into it; reject unknown keys.

# praisonai/approval/spec.py  (new)
from dataclasses import dataclass, field
from typing import Optional, Literal

Backend = Literal["console", "slack", "telegram", "discord", "webhook", "http", "agent", "auto", "none"]


@dataclass(frozen=True)
class ApprovalSpec:
    enabled: bool = False
    backend: Backend = "console"
    approve_all_tools: bool = False
    timeout: Optional[float] = None
    approve_level: Optional[Literal["low", "medium", "high", "critical"]] = None
    guardrails: Optional[str] = None

    @classmethod
    def from_cli(cls, args) -> "ApprovalSpec":
        enabled = bool(args.trust or args.approval or args.approve_all_tools or args.approve_level)
        return cls(
            enabled=enabled,
            backend=(args.approval or ("console" if args.trust else "none")),
            approve_all_tools=bool(args.approve_all_tools),
            timeout=_parse_timeout(args.approval_timeout),
            approve_level=args.approve_level,
            guardrails=args.guardrail,
        )

    @classmethod
    def from_yaml(cls, node) -> "ApprovalSpec":
        # node is the value of the `approval:` key in YAML. Accept bool/str/dict, reject unknown keys.
        if node is None or node is False:
            return cls(enabled=False, backend="none")
        if node is True:
            return cls(enabled=True, backend="console")
        if isinstance(node, str):
            return cls(enabled=True, backend=node)  # type: ignore[arg-type]
        if isinstance(node, dict):
            allowed = {"backend", "approve_all_tools", "timeout", "approve_level", "guardrails", "enabled"}
            unknown = set(node) - allowed
            if unknown:
                raise ValueError(f"Unknown approval keys: {sorted(unknown)}. Allowed: {sorted(allowed)}")
            return cls(
                enabled=node.get("enabled", True),
                backend=node.get("backend", "console"),
                approve_all_tools=bool(node.get("approve_all_tools", False)),
                timeout=node.get("timeout"),
                approve_level=node.get("approve_level"),
                guardrails=node.get("guardrails"),
            )
        raise TypeError(f"Unsupported approval node type: {type(node).__name__}")

Then collapse the merge / reconstruction logic to a single line per surface:

# agents_generator.py
from .approval.spec import ApprovalSpec

# CLI merge
if any(k in cli_config for k in ("trust", "approval", "approve_all_tools",
                                 "approval_timeout", "approve_level", "guardrail")):
    spec = ApprovalSpec.from_cli(_namespace_from(cli_config))
    for role in config.get("roles", {}).values():
        role["approval"] = spec  # single canonical key

# YAML -> agent
spec: ApprovalSpec = (
    details["approval"] if isinstance(details.get("approval"), ApprovalSpec)
    else ApprovalSpec.from_yaml(details.get("approval"))
)

agent = PraisonAgent(
    name=role_filled,
    ...
    approval=spec,                # single param, single type
    guardrails=spec.guardrails,
)

Result:

  • One key (approval:) per surface; --trust, --approval, --approve-all-tools etc. all funnel into the same ApprovalSpec.
  • Typos become hard errors instead of silent defaults.
  • Python users get a single, documented dataclass instead of having to know which of 8 alias keys is "the right one."
  • The 3-way feature surface promise is actually upheld.

Validation

All file:line references above were re-read against the current main branch (snippets quoted verbatim from the source). The asyncio.run pattern was additionally counted via rg "asyncio\.run\(" and confirmed in 11+ wrapper files (agents_generator.py, bots/_approval_base.py, endpoints/a2u_server.py, integrations/base.py, integrations/managed_local.py, integrations/registry.py, acp/server.py, mcp_server/transports/stdio.py, browser/cli.py, cli/commands/gateway.py, cli/main.py).

Each fix is self-contained and can land independently (bridge module, logging module, approval spec module).

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingclaudeAuto-trigger Claude analysisperformance

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions