Skip to content

Commit dcff27a

Browse files
fix(wrapper): resolve 3 architectural gaps in praisonai wrapper layer
- Fix dual CLI entry points with semantic drift by making Typer single dispatcher - Fix thread-unsafe module-level lazy state with proper synchronization - Fix closed framework-adapter registry with proper registry pattern and entry points Addresses core architecture violations of protocol-driven core, performance-first, and multi-agent + async safe by default principles. Fixes #1533 Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com>
1 parent 914a1a2 commit dcff27a

8 files changed

Lines changed: 374 additions & 272 deletions

File tree

src/praisonai/praisonai/__init__.py

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,26 +27,37 @@
2727
'LocalManagedConfig',
2828
]
2929

30-
# Telemetry initialization state
30+
# Telemetry initialization state - thread-safe
31+
import threading
32+
33+
_telemetry_lock = threading.Lock()
3134
_telemetry_initialized = False
3235

3336
def _ensure_telemetry_defaults() -> None:
34-
"""Apply telemetry env defaults exactly once, on first observability use."""
37+
"""Apply telemetry env defaults exactly once, on first observability use.
38+
39+
Thread-safe implementation using double-checked locking pattern.
40+
"""
3541
global _telemetry_initialized
3642
if _telemetry_initialized:
3743
return
38-
import os
39-
langfuse_configured = bool(
40-
os.getenv("LANGFUSE_PUBLIC_KEY")
41-
or os.path.exists(os.path.expanduser("~/.praisonai/langfuse.env"))
42-
)
43-
if langfuse_configured:
44-
# Explicitly enable OTEL for Langfuse integration
45-
os.environ["OTEL_SDK_DISABLED"] = "false"
46-
else:
47-
os.environ.setdefault("OTEL_SDK_DISABLED", "true")
48-
os.environ.setdefault("EC_TELEMETRY", "false") # respect user overrides
49-
_telemetry_initialized = True
44+
45+
with _telemetry_lock:
46+
if _telemetry_initialized:
47+
return
48+
49+
import os
50+
langfuse_configured = bool(
51+
os.getenv("LANGFUSE_PUBLIC_KEY")
52+
or os.path.exists(os.path.expanduser("~/.praisonai/langfuse.env"))
53+
)
54+
if langfuse_configured:
55+
# Explicitly enable OTEL for Langfuse integration
56+
os.environ["OTEL_SDK_DISABLED"] = "false"
57+
else:
58+
os.environ.setdefault("OTEL_SDK_DISABLED", "true")
59+
os.environ.setdefault("EC_TELEMETRY", "false") # respect user overrides
60+
_telemetry_initialized = True
5061

5162

5263
# Lazy loading for heavy imports

src/praisonai/praisonai/__main__.py

Lines changed: 49 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -3,118 +3,43 @@
33
PraisonAI CLI — Unified Entry Point.
44
55
Single entry point for all CLI invocations.
6-
Routes to Typer-based commands for known subcommands,
7-
falls back to legacy argparse for direct prompts and YAML files.
6+
Makes Typer the single dispatcher with narrow legacy shim for bare prompts/YAML.
87
98
Design:
10-
- Typer-first: all registered commands auto-discovered via Click
11-
- Legacy fallback: prompts, .yaml paths, and deprecated --flags
12-
- No manual command lists needed — adding a Typer command Just Works
9+
- Typer owns all command resolution
10+
- Legacy shim only for bare prompt/YAML invocations via Typer callback
11+
- Fail loud on registration errors - no silent degradation
1312
"""
1413

1514
import sys
1615

1716

18-
# ---------------------------------------------------------------------------
19-
# Internal helpers
20-
# ---------------------------------------------------------------------------
21-
22-
_typer_commands_cache = None
23-
24-
25-
def _get_typer_commands():
26-
"""Auto-discover registered Typer commands via Click introspection.
27-
28-
Returns a set of command names that the Typer app knows about.
29-
This is populated from app.py's register_commands() — no manual
30-
lists to maintain.
17+
def _is_legacy_invocation(argv: list[str]) -> bool:
18+
"""Check if this is a bare prompt or bare YAML invocation.
19+
20+
Legacy invocations are:
21+
- Bare YAML file: "agents.yaml"
22+
- Free-text prompt: "Create a weather app"
23+
24+
All other invocations should be handled by Typer commands.
3125
"""
32-
global _typer_commands_cache
33-
if _typer_commands_cache is not None:
34-
return _typer_commands_cache
35-
36-
try:
37-
from praisonai.cli.app import app, register_commands
38-
register_commands()
39-
40-
import typer.main
41-
import click
42-
click_app = typer.main.get_command(app)
43-
ctx = click.Context(click_app, info_name="praisonai")
44-
_typer_commands_cache = set(click_app.list_commands(ctx))
45-
except Exception:
46-
_typer_commands_cache = set()
47-
48-
return _typer_commands_cache
49-
50-
51-
def _find_first_command(argv):
52-
"""Find the first non-flag argument in argv.
53-
54-
Skips global flags (--json, --verbose, etc.) and their values.
55-
Returns the first positional arg, or None if only flags are present.
56-
"""
57-
# Flags that consume a following value
58-
VALUE_FLAGS = {"--output-format", "-o"}
59-
60-
skip_next = False
6126
for arg in argv:
62-
if skip_next:
63-
skip_next = False
64-
continue
6527
if arg.startswith("-"):
66-
if arg in VALUE_FLAGS:
67-
skip_next = True
6828
continue
69-
return arg # First non-flag arg
70-
return None
71-
72-
73-
def _run_typer(argv):
74-
"""Dispatch to the Typer CLI app."""
75-
from praisonai.cli.app import app, register_commands
76-
register_commands() # idempotent
77-
78-
original = sys.argv
79-
sys.argv = ["praisonai"] + list(argv)
80-
try:
81-
app()
82-
except SystemExit as e:
83-
sys.exit(e.code if isinstance(e.code, int) else 0)
84-
finally:
85-
sys.argv = original
29+
# Check if it's a YAML file or contains spaces (free-text prompt)
30+
return (arg.endswith((".yaml", ".yml")) or
31+
" " in arg or
32+
not arg.isidentifier())
33+
return False
8634

8735

88-
def _run_legacy(argv):
89-
"""Dispatch to the legacy argparse CLI (prompts, YAML, deprecated flags)."""
90-
from praisonai.cli.main import PraisonAI
91-
92-
original = sys.argv
93-
sys.argv = ["praisonai"] + list(argv)
94-
try:
95-
praison = PraisonAI()
96-
result = praison.main()
97-
code = 0 if result is None else (1 if result is False else 0)
98-
sys.exit(code)
99-
except SystemExit as e:
100-
sys.exit(e.code if isinstance(e.code, int) else 0)
101-
finally:
102-
sys.argv = original
103-
104-
105-
# ---------------------------------------------------------------------------
106-
# Main entry point
107-
# ---------------------------------------------------------------------------
108-
10936
def main():
110-
"""Unified CLI entry point Typer-first, legacy fallback.
37+
"""Unified CLI entry point - Typer is the single dispatcher.
11138
11239
Routing rules (in order):
113-
1. --version / -V → print version and exit
114-
2. --help / -h → Typer help (global or command-level)
115-
3. No arguments → Typer interactive TUI
116-
4. First arg is a Typer cmd→ Typer (auto-discovered from app.py)
117-
5. Everything else → Legacy (prompt, .yaml, deprecated flags)
40+
1. --version / -V → print version and exit
41+
2. Legacy invocation → legacy shim (bare prompts/YAML only)
42+
3. Everything else → Typer (owns all subcommands)
11843
"""
11944
argv = sys.argv[1:]
12045

@@ -124,30 +49,36 @@ def main():
12449
print(f"PraisonAI version {__version__}")
12550
return
12651

127-
# 2. Help flags → always Typer (global help or command help)
128-
if "--help" in argv or "-h" in argv:
129-
_run_typer(argv)
130-
return
131-
132-
# 3. No arguments → Typer (interactive TUI)
133-
if not argv:
134-
_run_typer(argv)
52+
# 2. Check for legacy invocation (bare prompt/YAML)
53+
if _is_legacy_invocation(argv):
54+
from praisonai.cli.main import PraisonAI
55+
original = sys.argv
56+
sys.argv = ["praisonai"] + list(argv)
57+
try:
58+
praison = PraisonAI()
59+
result = praison.main()
60+
code = 0 if result is None else (1 if result is False else 0)
61+
sys.exit(code)
62+
except SystemExit as e:
63+
sys.exit(e.code if isinstance(e.code, int) else 0)
64+
finally:
65+
sys.argv = original
13566
return
13667

137-
# 4. Find first non-flag argument and check if it's a Typer command
138-
first_cmd = _find_first_command(argv)
139-
140-
if first_cmd is None:
141-
# Only flags, no command → Typer handles global flags
142-
_run_typer(argv)
143-
return
144-
145-
if first_cmd in _get_typer_commands():
146-
# Known Typer command → Typer
147-
_run_typer(argv)
148-
else:
149-
# Prompt, YAML file, or legacy invocation → legacy
150-
_run_legacy(argv)
68+
# 3. All other invocations → Typer (fail loud on registration errors)
69+
from praisonai.cli.app import app, register_commands
70+
71+
# CRITICAL: Fail loud - do not swallow registration exceptions
72+
register_commands() # Let any ImportError/other exceptions propagate
73+
74+
original = sys.argv
75+
sys.argv = ["praisonai"] + list(argv)
76+
try:
77+
app()
78+
except SystemExit as e:
79+
sys.exit(e.code if isinstance(e.code, int) else 0)
80+
finally:
81+
sys.argv = original
15182

15283

15384
if __name__ == "__main__":

src/praisonai/praisonai/_async_bridge.py

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,18 @@ def get(self) -> asyncio.AbstractEventLoop:
3434
)
3535
self._thread.start()
3636
return self._loop
37+
38+
def get_unlocked(self) -> asyncio.AbstractEventLoop:
39+
"""Get loop assuming caller holds _lock. For run_sync() use only."""
40+
if self._loop is None or self._loop.is_closed():
41+
self._loop = asyncio.new_event_loop()
42+
self._thread = threading.Thread(
43+
target=self._loop.run_forever,
44+
name="praisonai-async",
45+
daemon=False,
46+
)
47+
self._thread.start()
48+
return self._loop
3749

3850
def shutdown(self, timeout: float = 5.0) -> None:
3951
with self._lock:
@@ -87,12 +99,11 @@ def run_sync(coro: Awaitable[T], *, timeout: float | None = _DEFAULT_TIMEOUT) ->
8799
except RuntimeError:
88100
running = False
89101

90-
if not running:
91-
# Reuse the background loop instead of creating a new one per call.
92-
fut: Future = asyncio.run_coroutine_threadsafe(coro, _BG.get())
93-
return fut.result(timeout=timeout)
94-
95-
fut = asyncio.run_coroutine_threadsafe(coro, _BG.get())
102+
# Submit the coroutine inside the lock to prevent shutdown races
103+
with _BG._lock:
104+
loop = _BG.get_unlocked() # get loop while holding lock
105+
fut: Future = asyncio.run_coroutine_threadsafe(coro, loop)
106+
96107
return fut.result(timeout=timeout)
97108

98109

src/praisonai/praisonai/agents_generator.py

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,8 @@
1717
import keyword
1818

1919
# Import new architecture components
20-
from .framework_adapters import (
21-
FrameworkAdapter, CrewAIAdapter, AutoGenAdapter,
22-
AutoGenV4Adapter, AG2Adapter, PraisonAIAdapter
23-
)
20+
from .framework_adapters.base import FrameworkAdapter
21+
from .framework_adapters.registry import FrameworkAdapterRegistry
2422
from .tool_registry import ToolRegistry
2523

2624
# Import availability flags
@@ -51,14 +49,8 @@
5149
except ImportError:
5250
pass
5351

54-
# Registry of available adapters (lazy-loaded)
55-
FRAMEWORK_ADAPTERS = {
56-
"crewai": CrewAIAdapter,
57-
"autogen": AutoGenAdapter,
58-
"autogen_v4": AutoGenV4Adapter,
59-
"ag2": AG2Adapter,
60-
"praisonai": PraisonAIAdapter
61-
}
52+
# Framework adapter registry - now uses proper registry pattern
53+
# This replaces the hardcoded FRAMEWORK_ADAPTERS dict
6254

6355
# Note: OTEL_SDK_DISABLED moved to CLI entry point per issue requirements
6456

@@ -258,12 +250,8 @@ def _get_framework_adapter(self, framework: str) -> FrameworkAdapter:
258250
Raises:
259251
ValueError: If framework is not supported
260252
"""
261-
if framework not in FRAMEWORK_ADAPTERS:
262-
raise ValueError(f"Unsupported framework: {framework}. "
263-
f"Supported frameworks: {list(FRAMEWORK_ADAPTERS.keys())}")
264-
265-
adapter_class = FRAMEWORK_ADAPTERS[framework]
266-
return adapter_class()
253+
adapter_registry = FrameworkAdapterRegistry.get_instance()
254+
return adapter_registry.create(framework)
267255

268256
def _merge_cli_config(self, config, cli_config):
269257
"""

src/praisonai/praisonai/async_agent_scheduler.py

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import asyncio
99
import logging
10+
import threading
1011
from datetime import datetime
1112
from typing import Optional, Dict, Any, Callable, Union
1213
from abc import ABC, abstractmethod
@@ -106,16 +107,25 @@ def __init__(
106107
self._success_count = 0
107108
self._failure_count = 0
108109

109-
# Created lazily on first async entry — binds to the caller's loop
110+
# Sync lock for async primitives creation and bound loop tracking
111+
self._primitives_lock = threading.Lock()
110112
self._cancel_event: Optional[asyncio.Event] = None
111113
self._stats_lock: Optional[asyncio.Lock] = None
114+
self._bound_loop: Optional[asyncio.AbstractEventLoop] = None
112115

113116
def _ensure_async_primitives(self) -> None:
114-
"""Create async primitives if they don't exist yet."""
115-
if self._cancel_event is None:
116-
self._cancel_event = asyncio.Event()
117-
if self._stats_lock is None:
118-
self._stats_lock = asyncio.Lock()
117+
"""Create async primitives if they don't exist yet.
118+
119+
Thread-safe and loop-aware: primitives are bound to the current running loop.
120+
If called from a different loop, new primitives are created.
121+
"""
122+
loop = asyncio.get_running_loop() # must be called from a coroutine
123+
124+
with self._primitives_lock:
125+
if self._bound_loop is not loop:
126+
self._cancel_event = asyncio.Event()
127+
self._stats_lock = asyncio.Lock()
128+
self._bound_loop = loop
119129

120130
async def start(
121131
self,

0 commit comments

Comments
 (0)