Skip to content

Commit 6a9a8e2

Browse files
fix: resolve external agent UI issues found by code reviewers
- Fix P1 global state contamination in default_app.py (session isolation) - Fix external agent tools bypassing tools_enabled guard in chat.py - Fix cache invalidation to include external agent selection - Fix Chainlit Select API usage (use MultiSelect for multi-selection) - Fix type annotations (use Optional[list[str]] instead of list) - Fix mention detection regex for better accuracy - Fix agent.start_async call (should be agent.achat) - Prevent mutation of cached tools list Addresses critical issues found by Greptile, CodeRabbit, and Copilot reviewers. Maintains backward compatibility while fixing runtime crashes and isolation bugs. Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
1 parent e6452e4 commit 6a9a8e2

3 files changed

Lines changed: 95 additions & 71 deletions

File tree

src/praisonai/praisonai/ui/chat.py

Lines changed: 34 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
# Standard library imports (minimal at top level)
1818
import os
1919
import logging
20+
from typing import Optional
2021

2122
# Set up minimal logging first
2223
logger = logging.getLogger(__name__)
@@ -31,7 +32,7 @@
3132

3233
# Chainlit must be imported early (required by decorators)
3334
import chainlit as cl
34-
from chainlit.input_widget import TextInput, Switch, Select
35+
from chainlit.input_widget import TextInput, Switch, Select, MultiSelect
3536
from chainlit.types import ThreadDict
3637
import chainlit.data as cl_data
3738

@@ -447,7 +448,7 @@ def auth_callback(username: str, password: str):
447448
logger.warning(f"Login failed for user: {username}")
448449
return None
449450

450-
def _get_or_create_agent(model_name: str, tools_enabled: bool = True, selected_external_agents: list = None):
451+
def _get_or_create_agent(model_name: str, tools_enabled: bool = True, selected_external_agents: Optional[list[str]] = None):
451452
"""Get or create a reusable agent for the session."""
452453
Agent = _get_praisonai_agent()
453454
if Agent is None:
@@ -456,32 +457,35 @@ def _get_or_create_agent(model_name: str, tools_enabled: bool = True, selected_e
456457
# Get cached agent from session
457458
cached_agent = cl.user_session.get("_cached_agent")
458459
cached_model = cl.user_session.get("_cached_agent_model")
460+
cached_external_agents = cl.user_session.get("_cached_external_agents", [])
459461

460-
# Reuse if model matches
461-
if cached_agent is not None and cached_model == model_name:
462+
# Reuse if model and external agents match
463+
if (cached_agent is not None and
464+
cached_model == model_name and
465+
cached_external_agents == (selected_external_agents or [])):
462466
return cached_agent
463467

464468
# Create new agent with interactive tools
465469
_profile_start("create_agent")
466470
tools = []
467471
if tools_enabled:
468-
tools = _get_interactive_tools()
472+
tools = list(_get_interactive_tools()) # Copy to avoid mutation
469473
# Add Tavily if available
470474
if os.getenv("TAVILY_API_KEY"):
471475
tools.append(tavily_web_search)
472-
473-
# Add external agent tools
474-
if selected_external_agents:
475-
handler = _get_external_agents_handler()
476-
if handler:
477-
for agent_name in selected_external_agents:
478-
try:
479-
integration = handler.get_integration(agent_name)
480-
if integration and integration.is_available:
481-
tools.append(integration.as_tool())
482-
logger.info(f"Added external agent tool: {agent_name}")
483-
except Exception as e:
484-
logger.warning(f"Could not load external agent {agent_name}: {e}")
476+
477+
# Add external agent tools (only when tools are enabled)
478+
if selected_external_agents:
479+
handler = _get_external_agents_handler()
480+
if handler:
481+
for agent_name in selected_external_agents:
482+
try:
483+
integration = handler.get_integration(agent_name)
484+
if integration and integration.is_available:
485+
tools.append(integration.as_tool())
486+
logger.info(f"Added external agent tool: {agent_name}")
487+
except Exception as e:
488+
logger.warning(f"Could not load external agent {agent_name}: {e}")
485489

486490
# Build instructions based on available tools
487491
instructions = """You are a helpful AI assistant with access to powerful tools.
@@ -534,11 +538,12 @@ async def start():
534538

535539
# Get available external agents for settings
536540
available_external_agents = _check_available_external_agents()
537-
external_agent_options = []
541+
# Build agent options dict for MultiSelect
542+
external_agent_options = {}
538543
for agent_name, is_available in available_external_agents.items():
539544
if is_available:
540545
description = _EXTERNAL_AGENT_DESCRIPTIONS.get(agent_name, agent_name)
541-
external_agent_options.append(cl.SelectOption(label=description, value=agent_name))
546+
external_agent_options[description] = agent_name
542547

543548
settings_widgets = [
544549
TextInput(
@@ -557,12 +562,11 @@ async def start():
557562
# Add external agents selector if any are available
558563
if external_agent_options:
559564
settings_widgets.append(
560-
Select(
565+
MultiSelect(
561566
id="external_agents",
562567
label="External AI Agents (Select multiple)",
563-
options=external_agent_options,
564-
initial=selected_external_agents,
565-
multiple=True
568+
items=external_agent_options,
569+
initial=selected_external_agents
566570
)
567571
)
568572

@@ -882,11 +886,12 @@ async def on_chat_resume(thread: ThreadDict):
882886
logger.debug(f"Model name: {model_name}")
883887
# Get available external agents for settings
884888
available_external_agents = _check_available_external_agents()
885-
external_agent_options = []
889+
# Build agent options dict for MultiSelect
890+
external_agent_options = {}
886891
for agent_name, is_available in available_external_agents.items():
887892
if is_available:
888893
description = _EXTERNAL_AGENT_DESCRIPTIONS.get(agent_name, agent_name)
889-
external_agent_options.append(cl.SelectOption(label=description, value=agent_name))
894+
external_agent_options[description] = agent_name
890895

891896
settings_widgets = [
892897
TextInput(
@@ -904,12 +909,11 @@ async def on_chat_resume(thread: ThreadDict):
904909

905910
if external_agent_options:
906911
settings_widgets.append(
907-
Select(
912+
MultiSelect(
908913
id="external_agents",
909914
label="External AI Agents (Select multiple)",
910-
options=external_agent_options,
911-
initial=selected_external_agents,
912-
multiple=True
915+
items=external_agent_options,
916+
initial=selected_external_agents
913917
)
914918
)
915919

src/praisonai/praisonai/ui/code.py

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
# Standard library imports (minimal at top level)
1212
import os
1313
import logging
14+
from typing import Optional
1415

1516
# Set up minimal logging first
1617
logger = logging.getLogger(__name__)
@@ -25,7 +26,7 @@
2526

2627
# Chainlit must be imported early (required by decorators)
2728
import chainlit as cl
28-
from chainlit.input_widget import TextInput, Switch, Select
29+
from chainlit.input_widget import TextInput, Switch, Select, MultiSelect
2930
from chainlit.types import ThreadDict
3031
import chainlit.data as cl_data
3132

@@ -461,7 +462,7 @@ def _check_available_external_agents():
461462
return handler.check_availability()
462463
return {}
463464

464-
def _get_or_create_agent(model_name: str, tools_enabled: bool = True, claude_code_enabled: bool = False, selected_external_agents: list = None):
465+
def _get_or_create_agent(model_name: str, tools_enabled: bool = True, claude_code_enabled: bool = False, selected_external_agents: Optional[list[str]] = None):
465466
"""Get or create a reusable agent for the session."""
466467
Agent = _get_praisonai_agent()
467468
if Agent is None:
@@ -561,11 +562,12 @@ async def start():
561562

562563
# Get available external agents for settings
563564
available_external_agents = _check_available_external_agents()
564-
external_agent_options = []
565+
# Build agent options dict for MultiSelect
566+
external_agent_options = {}
565567
for agent_name, is_available in available_external_agents.items():
566568
if is_available:
567569
description = _EXTERNAL_AGENT_DESCRIPTIONS.get(agent_name, agent_name)
568-
external_agent_options.append(cl.SelectOption(label=description, value=agent_name))
570+
external_agent_options[description] = agent_name
569571

570572
settings_widgets = [
571573
TextInput(
@@ -588,12 +590,11 @@ async def start():
588590

589591
if external_agent_options:
590592
settings_widgets.append(
591-
Select(
593+
MultiSelect(
592594
id="external_agents",
593595
label="External AI Agents (Select multiple)",
594-
options=external_agent_options,
595-
initial=selected_external_agents,
596-
multiple=True
596+
items=external_agent_options,
597+
initial=selected_external_agents
597598
)
598599
)
599600

@@ -856,11 +857,12 @@ async def on_chat_resume(thread: ThreadDict):
856857
logger.debug(f"Model name: {model_name}")
857858
# Get available external agents for settings
858859
available_external_agents = _check_available_external_agents()
859-
external_agent_options = []
860+
# Build agent options dict for MultiSelect
861+
external_agent_options = {}
860862
for agent_name, is_available in available_external_agents.items():
861863
if is_available:
862864
description = _EXTERNAL_AGENT_DESCRIPTIONS.get(agent_name, agent_name)
863-
external_agent_options.append(cl.SelectOption(label=description, value=agent_name))
865+
external_agent_options[description] = agent_name
864866

865867
settings_widgets = [
866868
TextInput(
@@ -883,12 +885,11 @@ async def on_chat_resume(thread: ThreadDict):
883885

884886
if external_agent_options:
885887
settings_widgets.append(
886-
Select(
888+
MultiSelect(
887889
id="external_agents",
888890
label="External AI Agents (Select multiple)",
889-
options=external_agent_options,
890-
initial=selected_external_agents,
891-
multiple=True
891+
items=external_agent_options,
892+
initial=selected_external_agents
892893
)
893894
)
894895

src/praisonai/praisonai/ui_chat/default_app.py

Lines changed: 46 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,8 @@
2222
aiui.set_theme(preset="blue", dark_mode=True, radius="lg")
2323
aiui.set_pages(["chat"])
2424

25-
# Global state for external agents
26-
_available_agents = {}
27-
_selected_agents = []
28-
_agent_instance = None
25+
# Module-level cache for available agents (read-only)
26+
_available_agents_cache = {}
2927

3028

3129
def _get_external_agents_handler():
@@ -47,12 +45,17 @@ def _check_available_agents() -> Dict[str, bool]:
4745

4846
def _get_agent_with_tools(model: str, selected_agents: List[str]):
4947
"""Create or get cached PraisonAI agent with external agent tools."""
50-
global _agent_instance
48+
# Get session-scoped state
49+
if not hasattr(aiui, 'session_state'):
50+
session_state = {}
51+
else:
52+
session_state = aiui.session_state
5153

5254
# Simple caching based on model and selected agents
5355
cache_key = f"{model}:{','.join(sorted(selected_agents))}"
54-
if _agent_instance and getattr(_agent_instance, '_cache_key', '') == cache_key:
55-
return _agent_instance
56+
cached_agent = session_state.get('agent_instance')
57+
if cached_agent and getattr(cached_agent, '_cache_key', '') == cache_key:
58+
return cached_agent
5659

5760
try:
5861
from praisonaiagents import Agent
@@ -78,7 +81,7 @@ def _get_agent_with_tools(model: str, selected_agents: List[str]):
7881
tools=tools if tools else None
7982
)
8083
agent._cache_key = cache_key
81-
_agent_instance = agent
84+
session_state['agent_instance'] = agent
8285
return agent
8386

8487
except ImportError:
@@ -89,13 +92,18 @@ def _get_agent_with_tools(model: str, selected_agents: List[str]):
8992
@aiui.sidebar
9093
async def external_agents_settings():
9194
"""Render external agents selection in sidebar."""
92-
global _available_agents, _selected_agents
95+
# Get session-scoped state
96+
if not hasattr(aiui, 'session_state'):
97+
# Fallback if session_state not available
98+
session_state = {}
99+
else:
100+
session_state = aiui.session_state
93101

94-
# Check available agents (cached)
95-
if not _available_agents:
96-
_available_agents = _check_available_agents()
102+
# Check available agents (cached at module level for performance)
103+
if not _available_agents_cache:
104+
_available_agents_cache.update(_check_available_agents())
97105

98-
if not _available_agents:
106+
if not any(_available_agents_cache.values()):
99107
aiui.markdown("**No External Agents**")
100108
aiui.markdown("No external AI agents are installed. Install them to enable delegation:")
101109
aiui.markdown("- `curl -fsSL https://claude.ai/install.sh | sh` (Claude Code)")
@@ -113,11 +121,14 @@ async def external_agents_settings():
113121
"cursor": "Cursor CLI (IDE tasks)"
114122
}
115123

124+
# Get current selection from session state
125+
current_selection = session_state.get('selected_agents', [])
116126
updated_selection = []
117-
for agent_name, is_available in _available_agents.items():
127+
128+
for agent_name, is_available in _available_agents_cache.items():
118129
if is_available:
119130
description = agent_descriptions.get(agent_name, agent_name)
120-
is_selected = agent_name in _selected_agents
131+
is_selected = agent_name in current_selection
121132

122133
# Create toggle button as checkbox alternative
123134
if is_selected:
@@ -134,12 +145,11 @@ async def external_agents_settings():
134145
# Button clicked - add to selection (toggle on)
135146
updated_selection.append(agent_name)
136147

137-
# Update global selection if changed
138-
if updated_selection != _selected_agents:
139-
_selected_agents = updated_selection
148+
# Update session-scoped selection if changed
149+
if updated_selection != current_selection:
150+
session_state['selected_agents'] = updated_selection
140151
# Clear agent cache to recreate with new tools
141-
global _agent_instance
142-
_agent_instance = None
152+
session_state['agent_instance'] = None
143153

144154
if updated_selection:
145155
aiui.success(f"External agents enabled: {', '.join(updated_selection)}")
@@ -172,8 +182,15 @@ async def on_message(message: str):
172182

173183
model = os.getenv("PRAISONAI_MODEL", "gpt-4o-mini")
174184

185+
# Get session-scoped state
186+
if not hasattr(aiui, 'session_state'):
187+
session_state = {}
188+
else:
189+
session_state = aiui.session_state
190+
175191
# Try to use PraisonAI agent with external agents
176-
agent = _get_agent_with_tools(model, _selected_agents)
192+
selected_agents = session_state.get('selected_agents', [])
193+
agent = _get_agent_with_tools(model, selected_agents)
177194
if agent:
178195
try:
179196
# Check for @mentions for direct delegation
@@ -185,18 +202,20 @@ async def on_message(message: str):
185202
}
186203

187204
mentioned_agent = None
205+
import re
188206
for mention, agent_name in agent_mentions.items():
189-
if mention in message.lower() and agent_name in _selected_agents:
190-
mentioned_agent = agent_name
191-
# Remove mention from message
192-
message = message.replace(mention, "").replace(mention.title(), "").strip()
193-
break
207+
if agent_name in selected_agents:
208+
pattern = re.compile(rf'(?<!\w){re.escape(mention)}\b', re.IGNORECASE)
209+
if pattern.search(message):
210+
mentioned_agent = agent_name
211+
message = pattern.sub("", message).strip()
212+
break
194213

195214
if mentioned_agent:
196215
await aiui.say(f"🤖 Delegating to {mentioned_agent}...")
197216

198217
# Use PraisonAI agent (supports async)
199-
response = await agent.start_async(message)
218+
response = await agent.achat(message)
200219
if hasattr(response, 'content'):
201220
content = response.content
202221
elif hasattr(response, 'result'):

0 commit comments

Comments
 (0)