-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: comprehensive managed agents follow-ups (fixes #1425) #1437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,141 @@ | ||
| """ | ||
| Test unified session info schema between managed agent backends. | ||
|
|
||
| Verifies that both AnthropicManagedAgent and LocalManagedAgent return | ||
| consistent session information with the same schema. | ||
| """ | ||
|
|
||
| import pytest | ||
| from unittest.mock import patch, Mock | ||
|
|
||
|
|
||
| def test_session_info_schema_consistency(): | ||
| """Test that both managed agents return identical session schema.""" | ||
| from praisonai.integrations.managed_agents import ManagedAgent | ||
| from praisonai.integrations.managed_local import LocalManagedAgent | ||
| from praisonai.integrations._session_info import SessionInfo | ||
|
|
||
| # Create both backends | ||
| managed = ManagedAgent() | ||
| local = LocalManagedAgent() | ||
|
|
||
| # Mock session data for both | ||
| managed._session_id = "test_session_anthropic" | ||
| local._session_id = "test_session_local" | ||
|
|
||
| # Get session info from both | ||
| with patch.object(managed, '_get_client') as mock_client: | ||
| # Mock Anthropic API response | ||
| mock_session = Mock() | ||
| mock_session.id = "test_session_anthropic" | ||
| mock_session.status = "active" | ||
| mock_session.title = "Test Session" | ||
| mock_usage = Mock() | ||
| mock_usage.input_tokens = 100 | ||
| mock_usage.output_tokens = 50 | ||
| mock_session.usage = mock_usage | ||
|
|
||
| mock_client.return_value.beta.sessions.retrieve.return_value = mock_session | ||
|
|
||
| managed_info = managed.retrieve_session() | ||
|
|
||
| local_info = local.retrieve_session() | ||
|
|
||
| # Both should have exact same keys | ||
| assert set(managed_info.keys()) == set(local_info.keys()) | ||
|
|
||
| # Required keys should be present | ||
| required_keys = ["id", "status", "title", "usage"] | ||
| for key in required_keys: | ||
| assert key in managed_info | ||
| assert key in local_info | ||
|
|
||
| # Usage should have consistent structure | ||
| usage_keys = ["input_tokens", "output_tokens"] | ||
| for key in usage_keys: | ||
| assert key in managed_info["usage"] | ||
| assert key in local_info["usage"] | ||
| assert isinstance(managed_info["usage"][key], int) | ||
| assert isinstance(local_info["usage"][key], int) | ||
|
|
||
|
|
||
| def test_session_info_dataclass(): | ||
| """Test SessionInfo dataclass functionality.""" | ||
| from praisonai.integrations._session_info import SessionInfo, SessionUsage | ||
|
|
||
| # Test default construction | ||
| info = SessionInfo() | ||
| assert info.id == "" | ||
| assert info.status == "unknown" | ||
| assert info.title == "" | ||
| assert info.usage.input_tokens == 0 | ||
| assert info.usage.output_tokens == 0 | ||
|
|
||
| # Test with data | ||
| usage = SessionUsage(input_tokens=100, output_tokens=50) | ||
| info = SessionInfo( | ||
| id="test_session", | ||
| status="active", | ||
| title="Test Session", | ||
| usage=usage | ||
| ) | ||
|
|
||
| # Test to_dict | ||
| data = info.to_dict() | ||
| expected = { | ||
| "id": "test_session", | ||
| "status": "active", | ||
| "title": "Test Session", | ||
| "usage": { | ||
| "input_tokens": 100, | ||
| "output_tokens": 50 | ||
| } | ||
| } | ||
| assert data == expected | ||
|
|
||
| # Test from_dict | ||
| reconstructed = SessionInfo.from_dict(data) | ||
| assert reconstructed.id == info.id | ||
| assert reconstructed.status == info.status | ||
| assert reconstructed.title == info.title | ||
| assert reconstructed.usage.input_tokens == info.usage.input_tokens | ||
| assert reconstructed.usage.output_tokens == info.usage.output_tokens | ||
|
|
||
|
|
||
| def test_session_info_backward_compatibility(): | ||
| """Test that SessionInfo handles missing fields gracefully.""" | ||
| from praisonai.integrations._session_info import SessionInfo | ||
|
|
||
| # Test partial data (old format) | ||
| partial_data = { | ||
| "id": "test_session", | ||
| "status": "active" | ||
| # Missing title and usage | ||
| } | ||
|
|
||
| info = SessionInfo.from_dict(partial_data) | ||
| assert info.id == "test_session" | ||
| assert info.status == "active" | ||
| assert info.title == "" # Default | ||
| assert info.usage.input_tokens == 0 # Default | ||
| assert info.usage.output_tokens == 0 # Default | ||
|
|
||
| # Test empty data | ||
| info = SessionInfo.from_dict({}) | ||
| data = info.to_dict() | ||
|
|
||
| # Should have all required keys with defaults | ||
| required_keys = ["id", "status", "title", "usage"] | ||
| for key in required_keys: | ||
| assert key in data | ||
|
|
||
|
|
||
| def test_managed_backend_protocol_re_export(): | ||
| """Test that ManagedBackendProtocol can be imported from managed module.""" | ||
| # Test lazy import works | ||
| from praisonaiagents.managed import ManagedBackendProtocol | ||
| assert ManagedBackendProtocol is not None | ||
|
|
||
| # Test it's the same as the original | ||
| from praisonaiagents.agent.protocols import ManagedBackendProtocol as OriginalProtocol | ||
| assert ManagedBackendProtocol is OriginalProtocol |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -316,6 +316,31 @@ def sessions_resume( | |
| print(result) | ||
|
|
||
|
|
||
| @sessions_app.command("delete") | ||
| def sessions_delete( | ||
| session_id: str = typer.Argument(..., help="Session ID to delete (sesn_01...)"), | ||
| confirm: bool = typer.Option(False, "--yes", "-y", help="Skip confirmation prompt"), | ||
| ): | ||
| """Delete a managed session permanently. | ||
|
|
||
| Example: | ||
| praisonai managed sessions delete sesn_01AbCdEf | ||
| praisonai managed sessions delete sesn_01AbCdEf --yes | ||
| """ | ||
| if not confirm: | ||
| typer.confirm(f"Delete session {session_id}? This cannot be undone.", abort=True) | ||
|
|
||
| try: | ||
| client = _get_client() | ||
| # Note: Anthropic API may not have delete endpoint yet | ||
| # This is a placeholder for when it becomes available | ||
| typer.echo(f"Session deletion not yet supported by Anthropic API") | ||
| typer.echo(f"Sessions will eventually expire automatically") | ||
| except Exception as e: | ||
| typer.echo(f"Error deleting session: {e}", err=True) | ||
| raise typer.Exit(1) | ||
|
|
||
|
|
||
| # ───────────────────────────────────────────────────────────────────────────── | ||
| # agents sub-commands | ||
| # ───────────────────────────────────────────────────────────────────────────── | ||
|
|
@@ -397,6 +422,29 @@ def agents_update( | |
| typer.echo(f"Updated agent: {updated.id} (v{getattr(updated,'version','')})") | ||
|
|
||
|
|
||
| @agents_app.command("delete") | ||
| def agents_delete( | ||
| agent_id: str = typer.Argument(..., help="Agent ID to delete (agent_01...)"), | ||
| confirm: bool = typer.Option(False, "--yes", "-y", help="Skip confirmation prompt"), | ||
| ): | ||
| """Delete a managed agent permanently. | ||
|
|
||
| Example: | ||
| praisonai managed agents delete agent_01AbCdEf | ||
| praisonai managed agents delete agent_01AbCdEf --yes | ||
| """ | ||
| if not confirm: | ||
| typer.confirm(f"Delete agent {agent_id}? This cannot be undone.", abort=True) | ||
|
|
||
| try: | ||
| client = _get_client() | ||
| client.beta.agents.delete(agent_id) | ||
| typer.echo(f"Agent {agent_id} deleted successfully") | ||
| except Exception as e: | ||
| typer.echo(f"Error deleting agent: {e}", err=True) | ||
| raise typer.Exit(1) | ||
|
Comment on lines
+425
to
+445
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Locate the anthropic version pinned in this repo and inspect other usages of beta.agents / beta.environments.
fd -t f 'pyproject.toml|setup.py|setup.cfg|requirements*.txt' | xargs rg -n -C1 'anthropic'
echo '---'
rg -nP '\bbeta\.(agents|environments|sessions)\.(delete|update|list|retrieve|create)\b' -C1Repository: MervinPraison/PraisonAI Length of output: 12963 🌐 Web query:
💡 Result: No, the Anthropic Python SDK (anthropic >=0.94.0) does not expose client.beta.agents.delete. It exposes client.beta.environments.delete and client.beta.environments.update. The official Claude API Python reference documents client.beta.environments.delete (result 1,6), client.beta.environments.update (result 9), but for agents, only create (result 13,59), list, retrieve, update (result 58), and archive (result 13). No delete method for agents is documented across multiple searches targeting the API reference. The SDK latest release v0.95.0 (result 2) supports these beta APIs as per the generated docs. Citations:
🏁 Script executed: rg -n '\.archive\(' src/praisonai --context 2Repository: MervinPraison/PraisonAI Length of output: 49 🌐 Web query:
💡 Result: To archive an agent using the Anthropic Python SDK instead of deleting it, use the client.beta.agents.archive(agent_id) method. This is a POST request to /v1/agents/{agent_id}/archive that marks the agent as archived, adding an archived_at timestamp while preserving the agent data. Example from official documentation: import os from anthropic import Anthropic client = Anthropic( api_key=os.environ.get("ANTHROPIC_API_KEY"), # This is the default and can be omitted ) beta_managed_agents_agent = client.beta.agents.archive( agent_id="agent_011CZkYpogX7uDKUy...", # Replace with your agent ID ) The method returns a BetaManagedAgentsAgent object with the archived agent details, including archived_at: Optional[datetime]. Optional parameters include betas: Optional[List[AnthropicBetaParam]] for specifying beta versions. This contrasts with deletion (if available), as archiving retains the agent for potential restoration or reference rather than permanent removal. Citations: Replace The Anthropic Python SDK does not expose The 🧰 Tools🪛 Ruff (0.15.10)[warning] 443-443: Do not catch blind exception: (BLE001) [warning] 445-445: Within an (B904) 🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| # ───────────────────────────────────────────────────────────────────────────── | ||
| # envs sub-commands | ||
| # ───────────────────────────────────────────────────────────────────────────── | ||
|
|
@@ -441,6 +489,54 @@ def envs_get( | |
| typer.echo(f"Config: {cfg}") | ||
|
|
||
|
|
||
| @envs_app.command("update") | ||
| def envs_update( | ||
| env_id: str = typer.Argument(..., help="Environment ID to update (env_01...)"), | ||
| name: Optional[str] = typer.Option(None, "--name", help="Update environment name"), | ||
| ): | ||
| """Update an existing environment's configuration. | ||
|
|
||
| Example: | ||
| praisonai managed envs update env_01AbCdEf --name "New Environment Name" | ||
| """ | ||
| client = _get_client() | ||
| kwargs = {} | ||
| if name: | ||
| kwargs["name"] = name | ||
| if not kwargs: | ||
| typer.echo("Nothing to update. Pass --name.") | ||
| raise typer.Exit(0) | ||
| try: | ||
| updated = client.beta.environments.update(env_id, **kwargs) | ||
| typer.echo(f"Updated environment: {updated.id}") | ||
| except Exception as e: | ||
| typer.echo(f"Error updating environment: {e}", err=True) | ||
| raise typer.Exit(1) | ||
|
|
||
|
|
||
| @envs_app.command("delete") | ||
| def envs_delete( | ||
| env_id: str = typer.Argument(..., help="Environment ID to delete (env_01...)"), | ||
| confirm: bool = typer.Option(False, "--yes", "-y", help="Skip confirmation prompt"), | ||
| ): | ||
| """Delete an environment permanently. | ||
|
|
||
| Example: | ||
| praisonai managed envs delete env_01AbCdEf | ||
| praisonai managed envs delete env_01AbCdEf --yes | ||
| """ | ||
| if not confirm: | ||
| typer.confirm(f"Delete environment {env_id}? This cannot be undone.", abort=True) | ||
|
|
||
| try: | ||
| client = _get_client() | ||
| client.beta.environments.delete(env_id) | ||
| typer.echo(f"Environment {env_id} deleted successfully") | ||
| except Exception as e: | ||
| typer.echo(f"Error deleting environment: {e}", err=True) | ||
| raise typer.Exit(1) | ||
|
|
||
|
|
||
| # ───────────────────────────────────────────────────────────────────────────── | ||
| # ids sub-commands (save / restore / show — no Anthropic IDs are user-defined) | ||
| # ───────────────────────────────────────────────────────────────────────────── | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| """ | ||
| Unified session info schema for managed agents. | ||
|
|
||
| Provides consistent session information structure across different managed agent backends. | ||
| """ | ||
|
|
||
| from dataclasses import dataclass, field | ||
| from typing import Dict, Any, Optional | ||
|
|
||
|
|
||
| @dataclass | ||
| class SessionUsage: | ||
| """Token usage information.""" | ||
| input_tokens: int = 0 | ||
| output_tokens: int = 0 | ||
|
|
||
|
|
||
| @dataclass | ||
| class SessionInfo: | ||
| """Unified session information across managed agent backends. | ||
|
|
||
| Provides consistent schema for session metadata returned by | ||
| retrieve_session() methods in both AnthropicManagedAgent and LocalManagedAgent. | ||
|
|
||
| All fields are always present with sensible defaults for backward compatibility. | ||
| """ | ||
| id: str = "" | ||
| status: str = "unknown" | ||
| title: str = "" | ||
| usage: SessionUsage = field(default_factory=SessionUsage) | ||
|
|
||
| def to_dict(self) -> Dict[str, Any]: | ||
| """Convert to dictionary format for backward compatibility.""" | ||
| return { | ||
| "id": self.id, | ||
| "status": self.status, | ||
| "title": self.title, | ||
| "usage": { | ||
| "input_tokens": self.usage.input_tokens, | ||
| "output_tokens": self.usage.output_tokens, | ||
| } | ||
| } | ||
|
|
||
| @classmethod | ||
| def from_dict(cls, data: Dict[str, Any]) -> "SessionInfo": | ||
| """Create from dictionary with defaults for missing fields.""" | ||
| usage_data = data.get("usage", {}) | ||
| usage = SessionUsage( | ||
| input_tokens=usage_data.get("input_tokens", 0), | ||
| output_tokens=usage_data.get("output_tokens", 0) | ||
| ) | ||
|
|
||
| return cls( | ||
| id=data.get("id", ""), | ||
| status=data.get("status", "unknown"), | ||
| title=data.get("title", ""), | ||
| usage=usage | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| """ | ||
| Unified tool alias mapping for managed agents. | ||
|
|
||
| Consolidates the tool mapping logic that was previously split between | ||
| managed_agents.TOOL_MAPPING and managed_local.TOOL_ALIAS_MAP. | ||
| """ | ||
|
|
||
| from typing import List | ||
|
|
||
|
|
||
| # Canonical tool alias mapping for all managed agent backends | ||
| UNIFIED_TOOL_MAPPING = { | ||
| "bash": "execute_command", | ||
| "read": "read_file", | ||
| "write": "write_file", | ||
| "edit": "apply_diff", # Use apply_diff for structured edits | ||
| "glob": "list_files", | ||
| "grep": "search_file", # Use search_file for content search | ||
| "web_fetch": "web_fetch", # Keep as web_fetch for consistency | ||
| "web_crawl": "web_fetch", # Alias for web_crawl | ||
| "search": "search_web", | ||
| "web_search": "search_web", | ||
| } | ||
|
|
||
|
|
||
| def map_managed_tools(managed_tools: List[str]) -> List[str]: | ||
| """Map managed agent tool names to PraisonAI tool names. | ||
|
|
||
| Uses unified mapping that resolves conflicts between Anthropic and Local backends: | ||
| - edit → apply_diff (structured diff edits preferred) | ||
| - grep → search_file (file content search preferred over shell command) | ||
| - web_fetch → web_fetch (canonical name) | ||
|
|
||
| Args: | ||
| managed_tools: List of tool names from managed agent configuration | ||
|
|
||
| Returns: | ||
| List of PraisonAI tool names | ||
| """ | ||
| return [UNIFIED_TOOL_MAPPING.get(tool, tool) for tool in managed_tools] | ||
|
|
||
|
|
||
| def get_tool_alias(tool_name: str) -> str: | ||
| """Get the canonical PraisonAI tool name for a managed tool. | ||
|
|
||
| Args: | ||
| tool_name: Original tool name (e.g. 'bash', 'grep') | ||
|
|
||
| Returns: | ||
| Canonical PraisonAI tool name (e.g. 'execute_command', 'search_file') | ||
| """ | ||
| return UNIFIED_TOOL_MAPPING.get(tool_name, tool_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misleading UX: command prompts to confirm a destructive action, then no-ops.
sessions_deleteasks the user to confirm deletion, then prints that deletion isn't supported. This is backwards — the user has been led to believe something destructive is about to happen, and commits (types "y") before learning nothing will occur. It also exits with status0on the placeholder path, which silently masks the "not implemented" state from scripts/CI.Additionally, Lines 337–338 are flagged by Ruff (F541) as
f-strings with no placeholders.Consider either:
2(or similar), orProposed fix
🧰 Tools
🪛 Ruff (0.15.10)
[error] 337-337: f-string without any placeholders
Remove extraneous
fprefix(F541)
[error] 338-338: f-string without any placeholders
Remove extraneous
fprefix(F541)
[warning] 339-339: Do not catch blind exception:
Exception(BLE001)
[warning] 341-341: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
🤖 Prompt for AI Agents