diff --git a/agent.py b/agent.py index 267a908077..e265002a69 100644 --- a/agent.py +++ b/agent.py @@ -91,6 +91,9 @@ def __init__( AgentContext._counter += 1 self.no = AgentContext._counter self.last_message = last_message or datetime.now(timezone.utc) + + # Initialize error tracking counters + self.tool_request_format_error_count = 0 # Track format errors for warnings # initialize agent at last (context is complete now) self.agent0 = agent0 or Agent(0, self.config, self) @@ -727,6 +730,129 @@ def hist_add_tool_result(self, tool_name: str, tool_result: str, **kwargs): extension.call_extensions_sync("hist_add_tool_result", self, data=data) return self.hist_add_message(False, content=data, id=msg_id) + def build_tool_request_error_observation( + self, error_reason: str, invalid_request: dict | None = None + ) -> str: + """Build structured error observation for invalid tool request format. + + Args: + error_reason: Human-readable error description + invalid_request: The invalid request that caused the error + + Returns: + Formatted error observation message for LLM feedback + """ + obs = "**Invalid tool request format**\n\n" + obs += f"**Reason:** {error_reason}\n\n" + + if invalid_request: + obs += "**Your invalid tool request:**\n" + obs += "```json\n" + import json + try: + obs += json.dumps(invalid_request, indent=2) + except: + obs += str(invalid_request) + obs += "\n```\n\n" + + obs += "**Correct required shape:**\n" + obs += "```json\n" + obs += "{\n" + obs += ' "tool_name": "name_of_the_tool",\n' + obs += ' "tool_args": {}\n' + obs += "}\n" + obs += "```\n\n" + + obs += "**Rules:**\n" + obs += "- `tool_name` must be a non-empty string\n" + obs += "- `tool_args` must be a JSON object\n" + obs += "- Use `tool_args` for arguments (not `args` or `arguments`)\n\n" + obs += "**Please retry** with a corrected tool call." + + return obs + + @extension.extensible + def add_tool_request_error_observation( + self, error_reason: str, invalid_request: dict | None = None, id: str = "" + ): + """Add tool request error observation to history for LLM retry. + + Args: + error_reason: Error message to display + invalid_request: The invalid request object + id: Optional message ID for tracking + """ + error_obs = self.build_tool_request_error_observation(error_reason, invalid_request) + return self.hist_add_warning(error_obs, id=id) + + def build_tool_execution_error_observation( + self, + tool_name: str, + tool_args: dict | None = None, + error_type: str = "Unknown", + error_message: str = "", + hint: str = "", + ) -> str: + """Build structured error observation for tool execution failure. + + Args: + tool_name: Name of the tool that failed + tool_args: Arguments passed to the tool + error_type: Type of error (e.g., FileNotFoundError) + error_message: Brief error description + hint: Suggestion for retry + + Returns: + Formatted error observation for LLM feedback + """ + obs = "**Tool execution failed**\n\n" + obs += f"**Tool:** `{tool_name}`\n\n" + + if tool_args: + obs += "**Arguments used:**\n" + obs += "```json\n" + import json + try: + obs += json.dumps(tool_args, indent=2) + except: + obs += str(tool_args) + obs += "\n```\n\n" + + obs += f"**Error type:** {error_type}\n\n" + obs += f"**Error message:** {error_message}\n\n" + + if hint: + obs += f"**Hint:** {hint}\n\n" + + obs += "**Please retry** with corrected tool arguments." + + return obs + + @extension.extensible + def add_tool_execution_error_observation( + self, + tool_name: str, + tool_args: dict | None = None, + error_type: str = "Unknown", + error_message: str = "", + hint: str = "", + id: str = "", + ): + """Add tool execution error observation to history for LLM retry. + + Args: + tool_name: Name of the failed tool + tool_args: Arguments passed to the tool + error_type: Type of error + error_message: Brief error description + hint: Suggestion for retry + id: Optional message ID for tracking + """ + error_obs = self.build_tool_execution_error_observation( + tool_name, tool_args, error_type, error_message, hint + ) + return self.hist_add_warning(error_obs, id=id) + def concat_messages( self, messages ): # TODO add param for message range, topic, history @@ -870,16 +996,48 @@ async def process_tools(self, msg: str): raw_tool_name = "" tool_args = {} + normalization_repairs = [] # Only validate when extraction produced an object; None means no JSON tool # block was found - the misformat warning path below handles that. if tool_request is not None: try: - raw_tool_name, tool_args = extract_tools.normalize_tool_request( + result = extract_tools.normalize_tool_request_with_diagnostics( tool_request ) - except ValueError: - tool_request = None # treat structural validation errors as misformat + raw_tool_name = result.tool_name + tool_args = result.tool_args + normalization_repairs = result.repairs + + # Log auto-repairs for transparency + if normalization_repairs: + repair_msg = "Tool request auto-repaired:\n" + "\n".join( + f"- {r}" for r in normalization_repairs + ) + PrintStyle(font_color="yellow", padding=True).print(repair_msg) + + except ValueError as e: + # Track format error count + if not hasattr(self.context, "tool_request_format_error_count"): + self.context.tool_request_format_error_count = 0 + self.context.tool_request_format_error_count += 1 + + error_reason = str(e) + self.add_tool_request_error_observation(error_reason, tool_request) + + # Warn if errors are repeating + if self.context.tool_request_format_error_count >= 3: + repeat_warning = ( + "**Format Error Repeated**\n\n" + "You have produced invalid tool requests **3+ times**.\n\n" + "Do not call the same invalid format again.\n\n" + "**Either:**\n" + "1. Retry with the exact required JSON shape, or\n" + "2. Use the `response` tool to end this interaction." + ) + self.hist_add_warning(repeat_warning) + + tool_request = None # treat as misformat if tool_request is not None: tool_name = raw_tool_name # Initialize tool_name with raw_tool_name @@ -932,7 +1090,39 @@ async def process_tools(self, msg: str): tool_name=tool_name, ) - response = await tool.execute(**tool_args) + # Execute tool with error handling + try: + response = await tool.execute(**tool_args) + except Exception as exec_error: + # Capture execution error details + error_type = type(exec_error).__name__ + error_msg = str(exec_error)[:200] # Truncate to prevent noise + + # Generate helpful hint based on error type + hint = "" + if error_type == "FileNotFoundError": + hint = "The file or path may not exist. Check for typos or list the directory first." + elif error_type == "ValueError": + hint = "One or more arguments have invalid values. Check the argument types and values." + elif error_type == "TypeError": + hint = "Missing required arguments or wrong argument types." + elif error_type == "PermissionError": + hint = "Insufficient permissions to perform this operation." + + # Add error observation for LLM retry + self.add_tool_execution_error_observation( + tool_name=tool_name, + tool_args=tool_args, + error_type=error_type, + error_message=error_msg, + hint=hint, + ) + + PrintStyle(font_color="red", padding=True).print( + f"Tool '{tool_name}' execution failed: {error_type}: {error_msg}" + ) + return None # Don't break loop, let LLM retry + await self.handle_intervention() # Allow extensions to postprocess tool response diff --git a/helpers/extract_tools.py b/helpers/extract_tools.py index 39e8378553..b1097a0c72 100644 --- a/helpers/extract_tools.py +++ b/helpers/extract_tools.py @@ -3,6 +3,138 @@ import regex, re from helpers.modules import load_classes_from_file, load_classes_from_folder # keep here for backwards compatibility from typing import Any +from dataclasses import dataclass, field + + +@dataclass +class ToolRequestNormalizationResult: + """Result of tool request normalization with diagnostic info""" + tool_name: str + tool_args: dict + repairs: list[str] = field(default_factory=list) # Auto-repairs applied + errors: list[str] = field(default_factory=list) # Hard errors that prevented normalization + + +def normalize_tool_request_with_diagnostics(tool_request: Any) -> ToolRequestNormalizationResult: + """ + Normalize tool request with detailed diagnostics. + + Handles aliases like: + - tool → tool_name + - name → tool_name + - args/arguments → tool_args + - tool_name:action syntax + - method → action + - JSON string → dict parsing + + Returns ToolRequestNormalizationResult with repairs list for LLM feedback. + Raises ValueError only for truly invalid requests (non-dict, missing tool_name). + """ + errors = [] + repairs = [] + + # Validate it's a dict + if not isinstance(tool_request, dict): + errors.append(f"Tool request must be a dictionary, got {type(tool_request).__name__}") + raise ValueError(errors[0]) + + # Extract tool_name with aliases + tool_name = None + + # Try direct tool_name + if "tool_name" in tool_request: + tool_name = tool_request["tool_name"] + # Try tool alias + elif "tool" in tool_request: + tool_name = tool_request["tool"] + repairs.append('Mapped "tool" to "tool_name"') + # Try name alias + elif "name" in tool_request: + tool_name = tool_request["name"] + repairs.append('Mapped "name" to "tool_name"') + + # Validate tool_name + if not tool_name or not isinstance(tool_name, str): + errors.append("Tool request must have tool_name (non-empty string)") + raise ValueError(errors[0]) + + # Extract tool_args with aliases + tool_args = None + + if "tool_args" in tool_request: + tool_args = tool_request["tool_args"] + elif "args" in tool_request: + tool_args = tool_request["args"] + repairs.append('Mapped "args" to "tool_args"') + elif "arguments" in tool_request: + tool_args = tool_request["arguments"] + repairs.append('Mapped "arguments" to "tool_args"') + + # Normalize tool_args + if tool_args is None: + tool_args = {} + elif isinstance(tool_args, str): + # Try to parse JSON string + try: + parsed = DirtyJson.parse_string(tool_args) + if isinstance(parsed, dict): + tool_args = parsed + repairs.append("Parsed tool_args JSON string into object") + else: + errors.append("tool_args must be a JSON object, but got string.") + except Exception as e: + errors.append("tool_args must be a JSON object, but got string.") + elif isinstance(tool_args, list): + errors.append( + f"tool_args must be a JSON object, got array/list" + ) + elif not isinstance(tool_args, dict): + errors.append( + f"tool_args must be a JSON object, got {type(tool_args).__name__}" + ) + + # If we have errors at this point, raise + if errors: + raise ValueError(" | ".join(errors)) + + # Ensure tool_args is dict + tool_args = dict(tool_args) if isinstance(tool_args, dict) else {} + + # Handle tool_name:action syntax + if ":" in tool_name: + parts = tool_name.split(":", 1) + if len(parts) == 2 and parts[0] and parts[1]: + tool_name = parts[0] + action = parts[1] + if "action" not in tool_args: + tool_args["action"] = action + repairs.append(f'Extracted action "{action}"') + else: + errors.append("tool_name:action syntax requires both parts non-empty") + + # Map method → action if action missing + if "action" not in tool_args and "method" in tool_args: + method = tool_args.get("method") + if isinstance(method, str) and method: + tool_args["action"] = method + repairs.append('Mapped "method" to "action"') + + return ToolRequestNormalizationResult( + tool_name=tool_name, + tool_args=tool_args, + repairs=repairs, + errors=errors + ) + + +def normalize_tool_request(tool_request: Any) -> tuple[str, dict]: + """ + Backward compatible wrapper for normalize_tool_request_with_diagnostics. + Returns (tool_name, tool_args) tuple for existing code. + """ + result = normalize_tool_request_with_diagnostics(tool_request) + return result.tool_name, result.tool_args + def json_parse_dirty(json: str) -> dict[str, Any] | None: if not json or not isinstance(json, str): @@ -20,31 +152,6 @@ def json_parse_dirty(json: str) -> dict[str, Any] | None: return None -def normalize_tool_request(tool_request: Any) -> tuple[str, dict]: - if not isinstance(tool_request, dict): - raise ValueError("Tool request must be a dictionary") - tool_name = tool_request.get("tool_name") - if not tool_name or not isinstance(tool_name, str): - tool_name = tool_request.get("tool") - if not tool_name or not isinstance(tool_name, str): - raise ValueError("Tool request must have a tool_name (type string) field") - tool_args = tool_request.get("tool_args") - if not isinstance(tool_args, dict): - tool_args = tool_request.get("args") - if not isinstance(tool_args, dict): - raise ValueError("Tool request must have a tool_args (type dictionary) field") - tool_args = dict(tool_args) - if ":" in tool_name: - tool_name, action = tool_name.split(":", 1) - if not tool_name or not action: - raise ValueError("tool_name method suffix must include tool and action") - tool_args.setdefault("action", action) - method = tool_args.get("method") - if "action" not in tool_args and isinstance(method, str) and method: - tool_args["action"] = method - return tool_name, tool_args - - def extract_json_root_string(content: str) -> str | None: if not content or not isinstance(content, str): return None diff --git a/helpers/plugins.py b/helpers/plugins.py index 45bac7b058..fde58792ab 100644 --- a/helpers/plugins.py +++ b/helpers/plugins.py @@ -12,6 +12,7 @@ Literal, Optional, TYPE_CHECKING, + TypeAlias, TypedDict, ) @@ -44,7 +45,7 @@ ) -type ToggleState = Literal["enabled", "disabled", "advanced"] +ToggleState: TypeAlias = Literal["enabled", "disabled", "advanced"] class PluginAssetFile(TypedDict): diff --git a/helpers/subagents.py b/helpers/subagents.py index f3557a49e8..fc4a416b32 100644 --- a/helpers/subagents.py +++ b/helpers/subagents.py @@ -1,7 +1,7 @@ from helpers import files from helpers import cache from helpers import yaml as yaml_helper -from typing import TypedDict, TYPE_CHECKING, Literal +from typing import TypedDict, TYPE_CHECKING, Literal, TypeAlias from pydantic import BaseModel, model_validator import json import os @@ -14,7 +14,7 @@ cache.toggle_area(PATHS_CACHE_AREA, False) -type Origin = Literal["default", "user", "project", "plugin"] +Origin: TypeAlias = Literal["default", "user", "project", "plugin"] if TYPE_CHECKING: from agent import Agent diff --git a/tests/helpers/test_extract_tools.py b/tests/helpers/test_extract_tools.py new file mode 100644 index 0000000000..49867651a7 --- /dev/null +++ b/tests/helpers/test_extract_tools.py @@ -0,0 +1,262 @@ +"""Unit tests for extract_tools.py normalization""" +import pytest +from helpers.extract_tools import ( + normalize_tool_request, + normalize_tool_request_with_diagnostics, + ToolRequestNormalizationResult, +) + + +class TestNormalizeToolRequestWithDiagnostics: + """Test normalize_tool_request_with_diagnostics function""" + + def test_valid_dict_request(self): + """Test normal valid request""" + result = normalize_tool_request_with_diagnostics({ + "tool_name": "response", + "tool_args": {"text": "hello"} + }) + assert result.tool_name == "response" + assert result.tool_args == {"text": "hello"} + assert result.repairs == [] + assert result.errors == [] + + def test_tool_to_tool_name_alias(self): + """Test tool → tool_name alias""" + result = normalize_tool_request_with_diagnostics({ + "tool": "response", + "tool_args": {"text": "hello"} + }) + assert result.tool_name == "response" + assert 'Mapped "tool" to "tool_name"' in result.repairs + + def test_name_to_tool_name_alias(self): + """Test name → tool_name alias""" + result = normalize_tool_request_with_diagnostics({ + "name": "response", + "tool_args": {"text": "hello"} + }) + assert result.tool_name == "response" + assert 'Mapped "name" to "tool_name"' in result.repairs + + def test_args_to_tool_args_alias(self): + """Test args → tool_args alias""" + result = normalize_tool_request_with_diagnostics({ + "tool_name": "response", + "args": {"text": "hello"} + }) + assert result.tool_args == {"text": "hello"} + assert 'Mapped "args" to "tool_args"' in result.repairs + + def test_arguments_to_tool_args_alias(self): + """Test arguments → tool_args alias""" + result = normalize_tool_request_with_diagnostics({ + "tool_name": "response", + "arguments": {"text": "hello"} + }) + assert result.tool_args == {"text": "hello"} + assert 'Mapped "arguments" to "tool_args"' in result.repairs + + def test_missing_tool_args(self): + """Test missing tool_args defaults to empty dict""" + result = normalize_tool_request_with_diagnostics({ + "tool_name": "response" + }) + assert result.tool_args == {} + assert result.repairs == [] + + def test_null_tool_args(self): + """Test null tool_args becomes empty dict""" + result = normalize_tool_request_with_diagnostics({ + "tool_name": "response", + "tool_args": None + }) + assert result.tool_args == {} + + def test_tool_args_json_string(self): + """Test tool_args as JSON string gets parsed""" + result = normalize_tool_request_with_diagnostics({ + "tool_name": "response", + "tool_args": '{"text": "hello"}' + }) + assert result.tool_args == {"text": "hello"} + assert "Parsed tool_args JSON string into object" in result.repairs + + def test_tool_args_json_string_with_dirty_json(self): + """Test tool_args with single quotes (dirty JSON)""" + result = normalize_tool_request_with_diagnostics({ + "tool_name": "response", + "tool_args": "{'text': 'hello'}" + }) + assert result.tool_args == {"text": "hello"} + assert "Parsed tool_args JSON string into object" in result.repairs + + def test_tool_args_list_error(self): + """Test tool_args as list raises error""" + with pytest.raises(ValueError) as exc_info: + normalize_tool_request_with_diagnostics({ + "tool_name": "response", + "tool_args": ["hello"] + }) + assert "array/list" in str(exc_info.value) + + def test_tool_args_string_non_json_error(self): + """Test tool_args as non-JSON string raises error""" + with pytest.raises(ValueError) as exc_info: + normalize_tool_request_with_diagnostics({ + "tool_name": "response", + "tool_args": "read file.txt" + }) + assert "JSON object" in str(exc_info.value) + + def test_tool_args_number_error(self): + """Test tool_args as number raises error""" + with pytest.raises(ValueError) as exc_info: + normalize_tool_request_with_diagnostics({ + "tool_name": "response", + "tool_args": 42 + }) + assert "int" in str(exc_info.value) + + def test_tool_name_colon_action(self): + """Test tool_name:action syntax""" + result = normalize_tool_request_with_diagnostics({ + "tool_name": "text_editor:read", + "tool_args": {"path": "/file.txt"} + }) + assert result.tool_name == "text_editor" + assert result.tool_args["action"] == "read" + assert any('Extracted action "read"' in repair for repair in result.repairs) + + def test_tool_name_colon_action_with_existing_action(self): + """Test tool_name:action doesn't override existing action""" + result = normalize_tool_request_with_diagnostics({ + "tool_name": "text_editor:read", + "tool_args": {"action": "write"} + }) + assert result.tool_name == "text_editor" + assert result.tool_args["action"] == "write" + + def test_method_to_action(self): + """Test method → action mapping""" + result = normalize_tool_request_with_diagnostics({ + "tool_name": "response", + "tool_args": {"method": "GET"} + }) + assert result.tool_args["action"] == "GET" + assert 'Mapped "method" to "action"' in result.repairs + + def test_method_to_action_no_override(self): + """Test method → action doesn't override existing action""" + result = normalize_tool_request_with_diagnostics({ + "tool_name": "response", + "tool_args": {"action": "POST", "method": "GET"} + }) + assert result.tool_args["action"] == "POST" + + def test_not_dict_error(self): + """Test non-dict input raises error""" + with pytest.raises(ValueError) as exc_info: + normalize_tool_request_with_diagnostics([{"tool_name": "response"}]) + assert "must be a dictionary" in str(exc_info.value) + + def test_missing_tool_name_error(self): + """Test missing tool_name raises error""" + with pytest.raises(ValueError) as exc_info: + normalize_tool_request_with_diagnostics({ + "tool_args": {} + }) + assert "tool_name" in str(exc_info.value) + + def test_empty_tool_name_error(self): + """Test empty tool_name raises error""" + with pytest.raises(ValueError) as exc_info: + normalize_tool_request_with_diagnostics({ + "tool_name": "", + "tool_args": {} + }) + assert "tool_name" in str(exc_info.value) + + def test_tool_name_not_string_error(self): + """Test tool_name as non-string raises error""" + with pytest.raises(ValueError) as exc_info: + normalize_tool_request_with_diagnostics({ + "tool_name": 123, + "tool_args": {} + }) + assert "tool_name" in str(exc_info.value) + + def test_multiple_repairs(self): + """Test multiple repairs are tracked""" + result = normalize_tool_request_with_diagnostics({ + "tool": "text_editor:read", + "args": '{"path": "/file.txt"}' + }) + assert result.tool_name == "text_editor" + assert any('Mapped "tool"' in repair for repair in result.repairs) + assert any('Mapped "args"' in repair for repair in result.repairs) + assert any('Extracted action "read"' in repair for repair in result.repairs) + + def test_empty_args_dict(self): + """Test empty tool_args dict is valid""" + result = normalize_tool_request_with_diagnostics({ + "tool_name": "response", + "tool_args": {} + }) + assert result.tool_args == {} + assert result.repairs == [] + + +class TestNormalizeToolRequestBackwardCompatibility: + """Test backward compatible normalize_tool_request wrapper""" + + def test_returns_tuple(self): + """Test returns (tool_name, tool_args) tuple""" + tool_name, tool_args = normalize_tool_request({ + "tool_name": "response", + "tool_args": {"text": "hello"} + }) + assert tool_name == "response" + assert tool_args == {"text": "hello"} + + def test_handles_alias(self): + """Test wrapper handles alias""" + tool_name, tool_args = normalize_tool_request({ + "tool": "response", + "args": {"text": "hello"} + }) + assert tool_name == "response" + assert tool_args == {"text": "hello"} + + def test_raises_on_error(self): + """Test wrapper raises ValueError on invalid input""" + with pytest.raises(ValueError): + normalize_tool_request({ + "tool_args": {} + }) + + +class TestToolRequestNormalizationResult: + """Test ToolRequestNormalizationResult dataclass""" + + def test_fields_present(self): + """Test result has all required fields""" + result = ToolRequestNormalizationResult( + tool_name="test", + tool_args={}, + repairs=["repair1"], + errors=[] + ) + assert result.tool_name == "test" + assert result.tool_args == {} + assert result.repairs == ["repair1"] + assert result.errors == [] + + def test_default_empty_lists(self): + """Test repairs and errors default to empty lists""" + result = ToolRequestNormalizationResult( + tool_name="test", + tool_args={} + ) + assert result.repairs == [] + assert result.errors == [] diff --git a/tests/test_runtime_rfc_fallback.py b/tests/test_runtime_rfc_fallback.py new file mode 100644 index 0000000000..e63b647ad1 --- /dev/null +++ b/tests/test_runtime_rfc_fallback.py @@ -0,0 +1,25 @@ +import asyncio + +from helpers import runtime + + +def test_call_development_function_falls_back_to_local_when_rfc_password_missing(monkeypatch): + monkeypatch.setattr(runtime, "args", {}, raising=False) + monkeypatch.delenv("RFC_PASSWORD", raising=False) + + def local_func(value): + return value + 1 + + result = asyncio.run(runtime.call_development_function(local_func, 41)) + assert result == 42 + + +def test_call_development_function_falls_back_to_async_local_when_rfc_password_missing(monkeypatch): + monkeypatch.setattr(runtime, "args", {}, raising=False) + monkeypatch.delenv("RFC_PASSWORD", raising=False) + + async def local_func(value): + return value + 1 + + result = asyncio.run(runtime.call_development_function(local_func, 41)) + assert result == 42