src/praisonai wrapper: multi-tenant chat_history bleed, per-loop httpx pool leak, and a framework-availability probe that pins on the first transient miss
In-depth review of the wrapper layer (src/praisonai/praisonai/) against the guiding philosophy — protocol-driven, DRY, multi-agent & async safe by default, 3-way surface (CLI + YAML + Python) must not fork, no global singletons — turned up three defects that ship in the current main and violate that contract in production. Each is anchored to the exact file/line, has a concrete failure mode I can describe end-to-end, and has a fix small enough to land as one PR each.
I looked far more widely (silent-swallows in endpoints/providers/mcp.py, recipe/core.py; duplicated default-registry singleton in six subsystems; ProfilerCompat.__new__ singleton without a lock, etc.) but they are either lower blast radius, cosmetic under normal use, or already partially mitigated. What follows is the shortlist that actually breaks a real deployment.
Scope: everything below is inside src/praisonai/praisonai/. I did not propose changes in the SDK (praisonaiagents) or in praisonai-tools.
1. Multi-tenant chat_history bleed: the two session-isolation paths already disagree, and one silently swallows the reset that guarantees isolation
Where
src/praisonai/praisonai/api/agent_invoke.py:307-336src/praisonai/praisonai/app/agentos.py:340-358
The two paths are supposed to do the same thing. They both take a template agent, clone it, wipe chat_history, then bind _session_id / _history_session_id so per-session isolation holds under concurrent multi-tenant requests. The comment on agent_invoke.py:310 explicitly warns:
A clone failure must never silently fall back to the shared template: doing so would leak one session's chat_history into another.
But two lines after that guard, the same file silently swallows the failure of the reset the isolation depends on:
# src/praisonai/praisonai/api/agent_invoke.py:307-336
try:
agent = _clone_agent(template)
except Exception as e:
# A clone failure must never silently fall back to the shared template:
# doing so would leak one session's chat_history into another. Fail the
# request instead so isolation is guaranteed.
logger.error(
f"Failed to clone agent '{agent_id}' for session isolation: {e}"
)
raise RuntimeError(
f"Failed to isolate agent '{agent_id}' for session: {e}"
) from e
# Reset per-request conversation state so the clone starts clean and
# (re)loads the requested session's history lazily on first chat.
try:
agent.chat_history = []
except Exception:
pass # ← swallows the very thing that enforces isolation
if hasattr(agent, "_session_store_initialized"):
agent._session_store_initialized = False
if session_id:
agent._session_id = session_id
...
return agentclone_for_channel() in the SDK is a shallow-ish copy: chat_history starts life as a shared reference to the template's list (or, in LocalManagedAgent / persistence-backed subclasses, a @property whose setter can raise on lock contention, disk-write failure, an unwritable session store, or a subclass that installs __slots__ without a slot for chat_history). Any of those routes turns the swallowed except: pass into: the clone keeps the template's history, then we stamp _session_id = "tenant-B-req-42" on top of it — and the very next agent.chat(...) call replies with tenant A's remembered turns baked into the prompt.
The other path doesn't even use a try/except, so the same operation crashes on one surface and silently corrupts on the other:
# src/praisonai/praisonai/app/agentos.py:340-358
if _supports_session_isolation(template) and not has_handoffs:
try:
agent = _clone_agent(template)
except Exception as e:
raise HTTPException(
status_code=500,
detail=f"Failed to isolate agent for session: {e}",
)
agent.chat_history = [] # ← no try/except; crashes as 500
if hasattr(agent, "_session_store_initialized"):
agent._session_store_initialized = False
if request.session_id:
agent._session_id = request.session_id
...
else:
agent = templateSo we have two problems in one:
- Silent cross-tenant chat_history leak on the
/invokerouter path. - DRY violation across the 3-way feature surface — the same isolation contract is duplicated in two files and has already drifted (one wrapped, one unwrapped). Anyone fixing (1) on
agent_invoke.pywill missagentos.py, and vice versa. This is exactly the "no forked behavior across CLI/YAML/Python" bullet in the philosophy.
Reproduction sketch
# demonstrates the failure mode
class BrokenSubclass(SomeSessionAwareAgent):
@property
def chat_history(self):
return self._chat_history
@chat_history.setter
def chat_history(self, value):
# e.g. persistence backend momentarily locked, or a version
# that persists on write and can raise PermissionError
raise RuntimeError("history store busy")
# Tenant A hits /invoke first, populates chat_history with "my SSN is 123…"
# Tenant B hits /invoke with session_id="tenant-B"
# _clone_agent returns a clone whose chat_history is still a reference
# to the template's list (or the setter blew up under lock contention).
# The `except Exception: pass` hides it.
# agent._session_id is now "tenant-B", but the prompt sent to the LLM
# includes A's turns.Turns silent data leaks into loud errors + eliminates the drift.
Fix — one helper, two callers
Introduce a single wrapper helper and route both call sites through it:
# src/praisonai/praisonai/api/agent_invoke.py
def bind_session(agent: Any, session_id: Optional[str]) -> Any:
"""Wipe conversation state and bind ``session_id``. Fails loudly."""
try:
agent.chat_history = []
except Exception as e:
# Never silently: a swallowed reset means the clone still holds
# the template's history and the next chat() leaks it to whoever
# session_id belongs to.
raise RuntimeError(
f"Cannot reset chat_history for session isolation: {e}"
) from e
if hasattr(agent, "_session_store_initialized"):
agent._session_store_initialized = False
agent._session_id = session_id
if hasattr(agent, "_history_session_id"):
agent._history_session_id = session_id
return agentThen both call sites collapse to a single line:
# src/praisonai/praisonai/api/agent_invoke.py:307
agent = _clone_agent(template) # already raises loudly on failure
return bind_session(agent, session_id)
# src/praisonai/praisonai/app/agentos.py:340
if _supports_session_isolation(template) and not has_handoffs:
try:
agent = _clone_agent(template)
except Exception as e:
raise HTTPException(status_code=500,
detail=f"Failed to isolate agent for session: {e}")
agent = bind_session(agent, request.session_id)
else:
agent = templateValidation — grep for agent.chat_history = [] in src/praisonai/praisonai/** and confirm only bind_session retains that line; add a regression test where the chat_history setter raises and assert the request 500s rather than returning template history.
2. Shared httpx.AsyncClient cache leaks its connection pool across event loops and crashes long-lived callers on the loop it just replaced
Where src/praisonai/praisonai/capabilities/passthrough.py:20-58
# src/praisonai/praisonai/capabilities/passthrough.py
_sync_client: Any = None
_async_client: Any = None
_async_client_loop: Any = None
_client_lock = threading.Lock()
def _get_async_client() -> Any:
"""Return the shared async httpx client for the running event loop.
``httpx.AsyncClient`` binds its connection pool to the event loop that first
used it, so a client cached across separate ``asyncio.run()`` lifecycles (or
reused from a different loop) fails with a loop-closed error. We therefore
key the cached client to its owning loop and rebuild it whenever the current
loop differs from the one that created it.
"""
global _async_client, _async_client_loop
import httpx
try:
current_loop = asyncio.get_running_loop()
except RuntimeError:
current_loop = None
with _client_lock:
if _async_client is None or _async_client_loop is not current_loop:
_async_client = httpx.AsyncClient() # ← previous client is dropped, never aclose()d
_async_client_loop = current_loop
return _async_clientWhy it's a real bug
The cache is a single module-level slot. Two independent failure modes:
(a) Connection-pool leak on every loop change. When a coroutine on loop L1 first calls _get_async_client(), _async_client is bound to L1's connection pool (keep-alive sockets, TLS contexts, DNS entries). A later call from loop L2 (a fresh asyncio.run(...) in a CLI subcommand, a scoped async bridge, a background worker thread with its own loop, a Jupyter cell re-run, a test that spins up its own loop) sees _async_client_loop is not current_loop and replaces the slot. The L1 client is never awaited-closed — its pool + open TCP sockets + TLS state leak permanently until the process exits.
(b) Wrong-loop crash on interleaved use. L1 hasn't finished. It stashed the L1 client and yielded. L2 kicks in, replaces _async_client. L1 resumes, calls _get_async_client() again believing it's calling into its own loop's client — but the current slot is L2's. Or, L1 already has a local reference to the L1 client, then a downstream coroutine calls _get_async_client() again on L2 and now the shared slot is a client bound to L2's loop. Any subsequent await client.get(...) from L1 hits RuntimeError: <event loop> is already running / <Future ...> attached to a different loop.
close_clients() (line 61-77) explicitly documents this: "The async client cannot be awaited from this sync helper, so it is dropped (and rebuilt per-loop on next use); the sync client is closed here" — which acknowledges the leak but declines to fix it.
Reproduction sketch
import asyncio, gc
from praisonai.capabilities.passthrough import _get_async_client
# Loop 1: get a client, use it, exit
async def in_loop_1():
c1 = _get_async_client()
print("l1 id:", id(c1))
asyncio.run(in_loop_1())
# Loop 2: replaces the slot without awaiting the L1 client's aclose()
async def in_loop_2():
c2 = _get_async_client()
print("l2 id:", id(c2)) # different from c1
asyncio.run(in_loop_2())
# L1's AsyncClient is now unreachable, but its transport / sockets
# leaked. Under load (a serve host that spins short-lived subtasks
# through scoped bridges), one leaked client per loop switch.Fix — key the cache by loop identity
Store one client per loop id, and register a task-done / loop-close hook to drain the old one. Sketch:
# src/praisonai/praisonai/capabilities/passthrough.py
_async_clients: dict[int, Any] = {} # loop id -> httpx.AsyncClient
_async_clients_lock = threading.Lock()
def _get_async_client() -> Any:
"""Return the async client for the *current* loop, constructing lazily."""
import httpx
loop = asyncio.get_running_loop()
key = id(loop)
with _async_clients_lock:
client = _async_clients.get(key)
if client is None:
client = httpx.AsyncClient()
_async_clients[key] = client
# Best-effort teardown: when the loop is torn down normally,
# drain the client on that loop so its pool closes cleanly.
def _drain(client=client, key=key):
try:
coro = client.aclose()
loop.run_until_complete(coro) if not loop.is_closed() else None
except Exception:
pass
_async_clients.pop(key, None)
try:
loop.call_soon_threadsafe(lambda: None) # cheap check loop is alive
except RuntimeError:
pass
return client
async def aclose_clients() -> None:
"""Await-close *every* per-loop client from within its own loop."""
current = asyncio.get_running_loop()
with _async_clients_lock:
# Only close the current loop's client here; other loops' clients
# will drain when their own loop tears down, or via aclose_all().
client = _async_clients.pop(id(current), None)
if client is not None:
await client.aclose()(An even simpler variant: don't cache the async client at all — build one per request. httpx.AsyncClient() construction is cheap; the shared pool argument is nullified the moment two loops exist. Only cache the sync client.)
Validation — grep for _async_client in src/praisonai/praisonai/**, confirm no bare module-level slot survives, and add a regression test that runs two back-to-back asyncio.run(apassthrough(...)) calls and asserts no httpx warnings about un-closed transports.
3. FrameworkAdapterRegistry.is_available memoises transient failures permanently — one flaky probe pins a framework to "unavailable" for the whole process
Where src/praisonai/praisonai/framework_adapters/registry.py:211-252
# src/praisonai/praisonai/framework_adapters/registry.py
def is_available(self, name: str) -> bool:
key = name.lower()
with self._avail_lock:
cached = self._avail_cache.get(key)
if cached is not None:
return cached
try:
adapter = self.create(name)
ok = bool(adapter.is_available())
except (ValueError, TypeError, ImportError):
ok = False
except Exception:
logger.warning("is_available() raised for adapter %r", name, exc_info=True)
ok = False
with self._avail_lock:
self._avail_cache[key] = ok # ← negative result cached forever
return okWhy it's a real bug
The docstring above says the probe "is memoised per process so hot-path callers like pick_default … do not re-probe." But negative results are cached with the same lifetime as positive ones — for the entire process. The probe can be False for reasons that are transient in the exact sense that matters:
- An
importlib.metadataentry-point scan racing a pluginpip install --user(entry-point file lands after we probe). - An adapter's constructor touching a lazily-imported optional dep whose first import fails once (e.g. an on-disk cache being warmed by another process, a
.pthfile not yet flushed, apkg_resourcesrefresh mid-scan). - An adapter whose
.is_available()probe hits the network (requests.get(...)to a hosted LLM's healthcheck) and times out on the first call because DNS wasn't yet resolvable at process boot. - An adapter that reads a config file whose write is still in flight from a supervisor script.
All four fall into the except Exception: branch. ok = False is committed to the cache. From then on:
pick_default()will never select that framework — even if the very next call would succeed.list_available_frameworks()hides it — users looking at the CLI'spraisonai frameworkscommand see it as gone.- The only escape hatch (
invalidate_availability) requires the caller to know the miss happened, which is precisely the information the silent memoisation destroys.
The commit history shows this cache has already burned the project multiple times — #5117 (test isolation), #5133 (probe path leak), #5123 (adapter resolver shadowing). Those all fix the test-side; the production-side footgun is still here.
The right primitive already exists in the wrapper: praisonai._lazy_cache.LazyCache distinguishes transient exceptions (_TRANSIENT_EXCEPTIONS) from durable ones and retries on the former. is_available is a hand-rolled cache that doesn't use it.
Fix — cache positives forever, retry negatives with a cool-down
# src/praisonai/praisonai/framework_adapters/registry.py
import time
_NEG_CACHE_TTL = 60.0 # seconds; probe again after a minute of "no"
def is_available(self, name: str) -> bool:
key = name.lower()
now = time.monotonic()
with self._avail_lock:
entry = self._avail_cache.get(key)
if entry is not None:
ok, expires_at = entry
if ok or now < expires_at:
return ok
# negative & expired -> fall through to reprobe
try:
adapter = self.create(name)
ok = bool(adapter.is_available())
expires_at = float("inf") # positive: cache forever
except (ValueError, TypeError, ImportError):
# Structural "not installed" — cache forever; a new install invalidates
# by re-probing after the wrapper's plugin refresh (kept explicit).
ok, expires_at = False, float("inf")
except Exception:
# Transient — probe raised on something that isn't a durable
# "unavailable" signal. Cache short, so the next call retries.
logger.warning("is_available() raised for adapter %r; will retry",
name, exc_info=True)
ok, expires_at = False, now + _NEG_CACHE_TTL
with self._avail_lock:
self._avail_cache[key] = (ok, expires_at)
return okValidation:
- Existing test fixture that resets
_avail_cachestill works (structure change only). - Add a regression test: subclass
FrameworkAdapter.is_availableto raiseRuntimeErroronce then returnTrue; assertregistry.is_available("x")returnsFalsefirst, thenTrueafter_NEG_CACHE_TTL. - Grep
src/praisonai/praisonai/**for hand-rolled_avail_cache[key] = okpatterns to confirm this is the only site (llm/registry.py,endpoints/registry.py,observability/hooks.pyall keep their own defaults but don't cache negative probes — this is the single hot-path memoiser).
Method / how I validated
Beyond reading the files, I ran targeted greps across src/praisonai/praisonai/:
except Exception:\s*pass— 24 sites; most are around import guards for optional deps and non-actionable log emissions. Only the three called out above are on a data-safety / correctness path.asyncio.run(in module bodies vs.def main/CLI entrypoints — everyasyncio.runI could find is at a top-of-CLI boundary, not nested in a loop-owning path. The_async_bridge.pymachinery correctly refuses nesting.- Module-level
_singleton,_default_*,_instance— the pattern is hand-rolled in ≥6 subsystems (endpoints/registry.py:115,integrations/registry.py:154,framework_adapters/registry.py:271,llm/registry.py:212,observability/hooks.py:483,api/agent_invoke.py:198). This is real but lower-severity than the three above; it belongs in a follow-up "consolidate default-registry lazy singletons on_lazy_cache.LazyCache" issue rather than in this shortlist. yaml.load(/pickle.load(/shell=True/eval(/exec(on config strings — no unsafe calls in the wrapper hot path.
The three findings above are the ones where a single production event ends in silent data corruption, socket leak / wrong-loop crash, or a permanently degraded framework selection. All three are one small PR each; none of them require touching the SDK.
Source: MervinPraison/PraisonAI