From 1a7f73b8ea72109b9a8432d90fbb411309cdfb4b Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 29 Jul 2026 09:02:14 -0700 Subject: [PATCH] refactor: migrate hand-rolled error envelopes to shared tool_error() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace json.dumps({"error": ...}) boilerplate with the documented tools/registry.py tool_error() helper across 13 files. Migrated: 59 sites (58 code sites + 1 docstring example in path_security.py), incl. multi-key envelopes passed via kwargs (available_actions, path/already_read, pattern/already_searched, parameters/hint, needs_reauth/server, error_type/tool/result_type). Also removed 2 now-redundant local tool_error imports in mcp_tool.py in favor of a module-level import. Skipped (not byte/shape-compatible with tool_error): - {"success": false, "error": ...} envelopes (browser_tool, browser_camofox, browser_dialog_tool, web_tools, tts_tool, skills_tool, image_generation_tool, project_tools, memory_tool, cronjob_tools, x_search_tool, xai_video_tools) — leading keys differ; key order would change. - terminal_tool/code_execution_tool envelopes carrying output/ exit_code/status leading keys. - tool_search.py:912-area multi-key success paths (non-error). - mcp_tool.py MCPSampling._error — returns MCP-spec ErrorData object, not a JSON string; incompatible. - send_message_tool._error — returns a dict (not str) and applies secret redaction; return type must be preserved. Behavior note: sites that previously omitted ensure_ascii=False now emit raw UTF-8 (tool_error's canonical behavior) — JSON-equivalent. Tests: 23 targeted files (tool_search, discord, file_tools/read guards/operations, registry, clarify, homeassistant, code_execution, send_message, delegate, terminal, mcp, model_tools, sanitize_tool_error, retaindb plugin) — all pass. ruff clean. --- model_tools.py | 23 +++---- tools/clarify_tool.py | 10 +-- tools/code_execution_tool.py | 50 ++++++-------- tools/delegate_tool.py | 16 ++--- tools/discord_tool.py | 34 +++++----- tools/file_tools.py | 80 ++++++++++------------ tools/homeassistant_tool.py | 6 +- tools/mcp_tool.py | 128 +++++++++++++---------------------- tools/path_security.py | 2 +- tools/registry.py | 16 ++--- tools/send_message_tool.py | 18 ++--- tools/terminal_tool.py | 13 ++-- tools/tool_search.py | 36 +++++----- 13 files changed, 181 insertions(+), 251 deletions(-) diff --git a/model_tools.py b/model_tools.py index 47f6ee18e74..9c354194b85 100644 --- a/model_tools.py +++ b/model_tools.py @@ -29,7 +29,7 @@ import threading import time from typing import Dict, Any, List, Optional, Tuple -from tools.registry import discover_builtin_tools, registry +from tools.registry import discover_builtin_tools, registry, tool_error from toolsets import resolve_toolset, validate_toolset logger = logging.getLogger(__name__) @@ -1170,8 +1170,7 @@ def handle_function_call( if function_name == _ts_mod.TOOL_CALL_NAME: underlying_name, underlying_args, err = _ts_mod.resolve_underlying_call(function_args or {}) if err or not underlying_name: - return json.dumps({"error": err or "tool_call could not be resolved"}, - ensure_ascii=False) + return tool_error(err or "tool_call could not be resolved") # Defense in depth: the underlying tool MUST be in the session's # scoped deferrable catalog. resolve_underlying_call() only checks # that the name is deferrable in the global registry; this gate @@ -1180,12 +1179,10 @@ def handle_function_call( # the bridge even if the catalog scoping above regressed. _scoped_deferrable = _ts_mod.scoped_deferrable_names(current_defs) if underlying_name not in _scoped_deferrable: - return json.dumps({ - "error": ( - f"'{underlying_name}' is not available in this session. " - "Use tool_search to find tools you can call." - ), - }, ensure_ascii=False) + return tool_error( + f"'{underlying_name}' is not available in this session. " + "Use tool_search to find tools you can call." + ) # Probe-validate against the deferred tool's schema (ironclaw#5149): # a blind call missing required arguments returns the parameter # schema instead of dispatching into an opaque downstream failure. @@ -1232,7 +1229,7 @@ def handle_function_call( try: if function_name in _AGENT_LOOP_TOOLS: - return json.dumps({"error": f"{function_name} must be handled by the agent loop"}) + return tool_error(f"{function_name} must be handled by the agent loop") # Check plugin hooks for a block/approve directive (unless caller # already checked — e.g. run_agent._invoke_tool passes skip=True to @@ -1263,7 +1260,7 @@ def handle_function_call( logger.debug("pre_tool_call hook error: %s", _hook_err) if block_message is not None: - result = json.dumps({"error": block_message}, ensure_ascii=False) + result = tool_error(block_message) _emit_post_tool_call_hook( function_name=function_name, function_args=function_args, @@ -1292,7 +1289,7 @@ def handle_function_call( except Exception as _edit_approval_err: logger.debug("ACP edit approval guard error: %s", _edit_approval_err) if function_name in {"write_file", "patch"}: - return json.dumps({"error": "Edit approval denied: approval guard failed"}, ensure_ascii=False) + return tool_error("Edit approval denied: approval guard failed") # Notify the read-loop tracker when a non-read/search tool runs, # so the *consecutive* counter resets (reads after other work are fine). @@ -1419,7 +1416,7 @@ def handle_function_call( except Exception as e: error_msg = f"Error executing {function_name}: {str(e)}" logger.exception(error_msg) - return json.dumps({"error": _sanitize_tool_error(error_msg)}, ensure_ascii=False) + return tool_error(_sanitize_tool_error(error_msg)) # ============================================================================= diff --git a/tools/clarify_tool.py b/tools/clarify_tool.py index 88e9762ca78..98bc9e4b42f 100644 --- a/tools/clarify_tool.py +++ b/tools/clarify_tool.py @@ -157,18 +157,12 @@ def clarify_tool( choices = None # empty list → open-ended if callback is None: - return json.dumps( - {"error": "Clarify tool is not available in this execution context."}, - ensure_ascii=False, - ) + return tool_error("Clarify tool is not available in this execution context.") try: raw_response = _invoke_callback(callback, question, choices, multi_select) except Exception as exc: - return json.dumps( - {"error": f"Failed to get user input: {exc}"}, - ensure_ascii=False, - ) + return tool_error(f"Failed to get user input: {exc}") if multi_select and choices is not None: user_response = _parse_multi_select_response(raw_response) diff --git a/tools/code_execution_tool.py b/tools/code_execution_tool.py index c69b2d5fdb2..af9c41a015d 100644 --- a/tools/code_execution_tool.py +++ b/tools/code_execution_tool.py @@ -621,7 +621,7 @@ def _rpc_server_loop( # sandbox-script-supplied JSON. str(request.get("token") or "").encode(), rpc_token.encode() ): - resp = json.dumps({"error": "Unauthorized RPC request"}) + resp = tool_error("Unauthorized RPC request") conn.sendall((resp + "\n").encode()) continue @@ -631,23 +631,19 @@ def _rpc_server_loop( # Enforce the allow-list if tool_name not in allowed_tools: available = ", ".join(sorted(allowed_tools)) - resp = json.dumps({ - "error": ( - f"Tool '{tool_name}' is not available in execute_code. " - f"Available: {available}" - ) - }) + resp = tool_error( + f"Tool '{tool_name}' is not available in execute_code. " + f"Available: {available}" + ) conn.sendall((resp + "\n").encode()) continue # Enforce tool call limit if tool_call_counter[0] >= max_tool_calls: - resp = json.dumps({ - "error": ( - f"Tool call limit reached ({max_tool_calls}). " - "No more tool calls allowed in this execution." - ) - }) + resp = tool_error( + f"Tool call limit reached ({max_tool_calls}). " + "No more tool calls allowed in this execution." + ) conn.sendall((resp + "\n").encode()) continue @@ -923,20 +919,16 @@ def _rpc_poll_loop( # Enforce allow-list if tool_name not in allowed_tools: available = ", ".join(sorted(allowed_tools)) - tool_result = json.dumps({ - "error": ( - f"Tool '{tool_name}' is not available in execute_code. " - f"Available: {available}" - ) - }) + tool_result = tool_error( + f"Tool '{tool_name}' is not available in execute_code. " + f"Available: {available}" + ) # Enforce tool call limit elif tool_call_counter[0] >= max_tool_calls: - tool_result = json.dumps({ - "error": ( - f"Tool call limit reached ({max_tool_calls}). " - "No more tool calls allowed in this execution." - ) - }) + tool_result = tool_error( + f"Tool call limit reached ({max_tool_calls}). " + "No more tool calls allowed in this execution." + ) else: # Strip forbidden terminal parameters if tool_name == "terminal" and isinstance(tool_args, dict): @@ -1207,10 +1199,10 @@ def execute_code( JSON string with execution results. """ if not SANDBOX_AVAILABLE: - return json.dumps({ - "error": "execute_code sandbox is unavailable in this environment. " - "Use normal tool calls (terminal, read_file, write_file, ...) instead." - }) + return tool_error( + "execute_code sandbox is unavailable in this environment. " + "Use normal tool calls (terminal, read_file, write_file, ...) instead." + ) if not code or not code.strip(): return tool_error("No code provided.") diff --git a/tools/delegate_tool.py b/tools/delegate_tool.py index 2513fa80408..9d1e9930228 100644 --- a/tools/delegate_tool.py +++ b/tools/delegate_tool.py @@ -2828,16 +2828,12 @@ def delegate_task( depth = getattr(parent_agent, "_delegate_depth", 0) max_spawn = _get_max_spawn_depth() if depth >= max_spawn: - return json.dumps( - { - "error": ( - f"Delegation depth limit reached (depth={depth}, " - f"max_spawn_depth={max_spawn}). Raise " - f"delegation.max_spawn_depth in config.yaml if deeper " - f"nesting is required (no hard ceiling, but each level " - f"multiplies API cost)." - ) - } + return tool_error( + f"Delegation depth limit reached (depth={depth}, " + f"max_spawn_depth={max_spawn}). Raise " + f"delegation.max_spawn_depth in config.yaml if deeper " + f"nesting is required (no hard ceiling, but each level " + f"multiplies API cost)." ) # Load config diff --git a/tools/discord_tool.py b/tools/discord_tool.py index 34d60d86dbe..e279f43d5f9 100644 --- a/tools/discord_tool.py +++ b/tools/discord_tool.py @@ -34,7 +34,7 @@ import urllib.parse import urllib.request from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple -from tools.registry import registry +from tools.registry import registry, tool_error if TYPE_CHECKING: from pathlib import Path @@ -1003,26 +1003,24 @@ def _run_discord_action( """Shared handler logic for both discord tools.""" token = _get_bot_token() if not token: - return json.dumps({"error": "DISCORD_BOT_TOKEN not configured."}) + return tool_error("DISCORD_BOT_TOKEN not configured.") action_fn = valid_actions.get(action) if not action_fn: - return json.dumps({ - "error": f"Unknown action: {action}", - "available_actions": list(valid_actions.keys()), - }) + return tool_error( + f"Unknown action: {action}", + available_actions=list(valid_actions.keys()), + ) # Config-level allowlist gate (defense in depth — schema already filtered, # but a stale cached schema from a prior config should not let denied # actions through). allowlist = _load_allowed_actions_config() if allowlist is not None and action not in allowlist: - return json.dumps({ - "error": ( - f"Action '{action}' is disabled by config (discord.server_actions). " - f"Allowed: {', '.join(allowlist) if allowlist else ''}" - ), - }) + return tool_error( + f"Action '{action}' is disabled by config (discord.server_actions). " + f"Allowed: {', '.join(allowlist) if allowlist else ''}" + ) local_vars = { "guild_id": guild_id, @@ -1036,9 +1034,9 @@ def _run_discord_action( missing = [p for p in _REQUIRED_PARAMS.get(action, []) if not local_vars.get(p)] if missing: - return json.dumps({ - "error": f"Missing required parameters for '{action}': {', '.join(missing)}", - }) + return tool_error( + f"Missing required parameters for '{action}': {', '.join(missing)}" + ) try: return action_fn( @@ -1058,11 +1056,11 @@ def _run_discord_action( except DiscordAPIError as e: logger.warning("Discord API error in %s action '%s': %s", tool_label, action, e) if e.status == 403: - return json.dumps({"error": _enrich_403(action, e.body)}) - return json.dumps({"error": str(e)}) + return tool_error(_enrich_403(action, e.body)) + return tool_error(str(e)) except Exception as e: logger.exception("Unexpected error in %s action '%s'", tool_label, action) - return json.dumps({"error": f"Unexpected error: {e}"}) + return tool_error(f"Unexpected error: {e}") def discord_core(action: str, **kwargs) -> str: diff --git a/tools/file_tools.py b/tools/file_tools.py index 15ddd0e3655..8ea335d002d 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -1116,12 +1116,10 @@ def read_file_tool(path: str, offset: int = 1, limit: int = 500, task_id: str = # blocking on input). Pure path check — no I/O. device_base = None if Path(path).expanduser().is_absolute() else _resolve_base_dir(task_id) if _is_blocked_device(path, base_dir=device_base): - return json.dumps({ - "error": ( - f"Cannot read '{path}': this is a device file that would " - "block or produce infinite output." - ), - }) + return tool_error( + f"Cannot read '{path}': this is a device file that would " + "block or produce infinite output." + ) _resolved = _resolve_path_for_task(path, task_id) @@ -1188,12 +1186,10 @@ def read_file_tool(path: str, offset: int = 1, limit: int = 500, task_id: str = # Block binary files by extension (no I/O). if has_binary_extension(str(_resolved)): _ext = _resolved.suffix.lower() - return json.dumps({ - "error": ( - f"Cannot read binary file '{path}' ({_ext}). " - "Use vision_analyze for images, or terminal to inspect binary files." - ), - }) + return tool_error( + f"Cannot read binary file '{path}' ({_ext}). " + "Use vision_analyze for images, or terminal to inspect binary files." + ) # ── Hermes internal path guard ──────────────────────────────── # Prevent prompt injection via catalog or hub metadata files, @@ -1204,7 +1200,7 @@ def read_file_tool(path: str, offset: int = 1, limit: int = 500, task_id: str = # the Python process cwd, which can differ. block_error = get_read_block_error(str(_resolved)) if block_error: - return json.dumps({"error": block_error}) + return tool_error(block_error) # ── Dedup check ─────────────────────────────────────────────── # If we already read this exact (path, offset, limit) and the @@ -1242,19 +1238,17 @@ def read_file_tool(path: str, offset: int = 1, limit: int = 500, task_id: str = _cap_read_tracker_data(task_data) if hits >= 2: - return json.dumps({ - "error": ( - f"BLOCKED: You have called read_file on this " - f"exact region {hits + 1} times and the file " - "has NOT changed. STOP calling read_file for " - "this path — the content from your earlier " - "read_file result in this conversation is " - "still current. Proceed with your task using " - "the information you already have." - ), - "path": path, - "already_read": hits + 1, - }, ensure_ascii=False) + return tool_error( + f"BLOCKED: You have called read_file on this " + f"exact region {hits + 1} times and the file " + "has NOT changed. STOP calling read_file for " + "this path — the content from your earlier " + "read_file result in this conversation is " + "still current. Proceed with your task using " + "the information you already have.", + path=path, + already_read=hits + 1, + ) return json.dumps({ "status": "unchanged", @@ -1380,15 +1374,13 @@ def read_file_tool(path: str, offset: int = 1, limit: int = 500, task_id: str = if count >= 4: # Hard block: stop returning content to break the loop - return json.dumps({ - "error": ( - f"BLOCKED: You have read this exact file region {count} times in a row. " - "The content has NOT changed. You already have this information. " - "STOP re-reading and proceed with your task." - ), - "path": path, - "already_read": count, - }, ensure_ascii=False) + return tool_error( + f"BLOCKED: You have read this exact file region {count} times in a row. " + "The content has NOT changed. You already have this information. " + "STOP re-reading and proceed with your task.", + path=path, + already_read=count, + ) elif count >= 3: result_dict["_warning"] = ( f"You have read this exact file region {count} times consecutively. " @@ -1878,15 +1870,13 @@ def search_tool(pattern: str, target: str = "content", path: str = ".", count = task_data["consecutive"] if count >= 4: - return json.dumps({ - "error": ( - f"BLOCKED: You have run this exact search {count} times in a row. " - "The results have NOT changed. You already have this information. " - "STOP re-searching and proceed with your task." - ), - "pattern": pattern, - "already_searched": count, - }, ensure_ascii=False) + return tool_error( + f"BLOCKED: You have run this exact search {count} times in a row. " + "The results have NOT changed. You already have this information. " + "STOP re-searching and proceed with your task.", + pattern=pattern, + already_searched=count, + ) try: resolved_path = _resolve_path_for_task(path, task_id) @@ -1894,7 +1884,7 @@ def search_tool(pattern: str, target: str = "content", path: str = ".", resolved_path = None block_error = get_read_block_error(str(resolved_path) if resolved_path else path) if block_error: - return json.dumps({"error": block_error}, ensure_ascii=False) + return tool_error(block_error) file_ops = _get_file_ops(task_id) result = file_ops.search( diff --git a/tools/homeassistant_tool.py b/tools/homeassistant_tool.py index 2e698a45908..27ff985afe5 100644 --- a/tools/homeassistant_tool.py +++ b/tools/homeassistant_tool.py @@ -264,10 +264,10 @@ def _handle_call_service(args: dict, **kw) -> str: return tool_error(f"Invalid service format: {service!r}") if domain in _BLOCKED_DOMAINS: - return json.dumps({ - "error": f"Service domain '{domain}' is blocked for security. " + return tool_error( + f"Service domain '{domain}' is blocked for security. " f"Blocked domains: {', '.join(sorted(_BLOCKED_DOMAINS))}" - }) + ) entity_id = args.get("entity_id") if entity_id and not _ENTITY_ID_RE.match(entity_id): diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index 0ae26b4d3b4..af7fc6b20ec 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -111,6 +111,8 @@ from datetime import datetime from typing import Any, Coroutine, Dict, List, Optional from urllib.parse import urlparse +from tools.registry import tool_error + logger = logging.getLogger(__name__) # Upper bound for the OSV malware preflight during stdio MCP startup. The @@ -3899,16 +3901,14 @@ def _handle_auth_error_and_retry( # needs_reauth error. Bumps the circuit breaker so the model stops # retrying the tool. _bump_server_error(server_name) - return json.dumps({ - "error": ( - f"MCP server '{server_name}' requires re-authentication. " - f"Run `hermes mcp login {server_name}` (or delete the tokens " - f"file under ~/.hermes/mcp-tokens/ and restart). Do NOT retry " - f"this tool — ask the user to re-authenticate." - ), - "needs_reauth": True, - "server": server_name, - }, ensure_ascii=False) + return tool_error( + f"MCP server '{server_name}' requires re-authentication. " + f"Run `hermes mcp login {server_name}` (or delete the tokens " + f"file under ~/.hermes/mcp-tokens/ and restart). Do NOT retry " + f"this tool — ask the user to re-authenticate.", + needs_reauth=True, + server=server_name, + ) # Substrings (lower-cased match) that indicate the MCP server rejected @@ -4498,9 +4498,7 @@ def _run_on_mcp_loop(coro_or_factory, timeout: float = 30): def _interrupted_call_result() -> str: """Standardized JSON error for a user-interrupted MCP tool call.""" - return json.dumps({ - "error": "MCP call interrupted: user sent a new message" - }, ensure_ascii=False) + return tool_error("MCP call interrupted: user sent a new message") # --------------------------------------------------------------------------- @@ -4696,23 +4694,19 @@ def _make_tool_handler(server_name: str, tool_name: str, tool_timeout: float): age = time.monotonic() - opened_at if age < _CIRCUIT_BREAKER_COOLDOWN_SEC: remaining = max(1, int(_CIRCUIT_BREAKER_COOLDOWN_SEC - age)) - return json.dumps({ - "error": ( - f"MCP server '{server_name}' is unreachable after " - f"{_server_error_counts[server_name]} consecutive " - f"failures. Auto-retry available in ~{remaining}s. " - f"Do NOT retry this tool yet — use alternative " - f"approaches or ask the user to check the MCP server." - ) - }, ensure_ascii=False) + return tool_error( + f"MCP server '{server_name}' is unreachable after " + f"{_server_error_counts[server_name]} consecutive " + f"failures. Auto-retry available in ~{remaining}s. " + f"Do NOT retry this tool yet — use alternative " + f"approaches or ask the user to check the MCP server." + ) # Cooldown elapsed → fall through as a half-open probe. server = _get_connected_server_for_call(server_name) if not server: _bump_server_error(server_name) - return json.dumps({ - "error": f"MCP server '{server_name}' is not connected" - }, ensure_ascii=False) + return tool_error(f"MCP server '{server_name}' is not connected") if not server.session: # No live session. A reconnect may already be completing (the @@ -4737,16 +4731,12 @@ def _make_tool_handler(server_name: str, tool_name: str, tool_timeout: float): # _reset_server_error). _bump_server_error(server_name) if _signal_reconnect(server): - return json.dumps({ - "error": ( - f"MCP server '{server_name}' transport is down; " - f"reconnect requested. Do NOT retry this tool " - f"immediately — give it a few seconds to come back." - ) - }, ensure_ascii=False) - return json.dumps({ - "error": f"MCP server '{server_name}' is not connected" - }, ensure_ascii=False) + return tool_error( + f"MCP server '{server_name}' transport is down; " + f"reconnect requested. Do NOT retry this tool " + f"immediately — give it a few seconds to come back." + ) + return tool_error(f"MCP server '{server_name}' is not connected") async def _call(): _mark_server_call_started(server) @@ -4779,11 +4769,9 @@ def _make_tool_handler(server_name: str, tool_name: str, tool_timeout: float): res_text = getattr(getattr(block, "resource", None), "text", None) if res_text: error_text += str(res_text) - return json.dumps({ - "error": _sanitize_error( - error_text or "MCP tool returned an error" - ) - }, ensure_ascii=False) + return tool_error(_sanitize_error( + error_text or "MCP tool returned an error" + )) # Collect text from content blocks. MCP tool results can also # include ImageContent blocks (screenshot / Blockbench / Playwright @@ -4891,11 +4879,9 @@ def _make_tool_handler(server_name: str, tool_name: str, tool_timeout: float): "MCP tool %s/%s call failed: %s", server_name, tool_name, exc, ) - return json.dumps({ - "error": _sanitize_error( - f"MCP call failed: {type(exc).__name__}: {_exc_str(exc)}" - ) - }, ensure_ascii=False) + return tool_error(_sanitize_error( + f"MCP call failed: {type(exc).__name__}: {_exc_str(exc)}" + )) return _handler @@ -4906,9 +4892,7 @@ def _make_list_resources_handler(server_name: str, tool_timeout: float): def _handler(args: dict, **kwargs) -> str: server = _get_connected_server_for_call(server_name) if not server or not server.session: - return json.dumps({ - "error": f"MCP server '{server_name}' is not connected" - }, ensure_ascii=False) + return tool_error(f"MCP server '{server_name}' is not connected") async def _call(): _mark_server_call_started(server) @@ -4951,11 +4935,9 @@ def _make_list_resources_handler(server_name: str, tool_timeout: float): logger.error( "MCP %s/list_resources failed: %s", server_name, exc, ) - return json.dumps({ - "error": _sanitize_error( - f"MCP call failed: {type(exc).__name__}: {_exc_str(exc)}" - ) - }, ensure_ascii=False) + return tool_error(_sanitize_error( + f"MCP call failed: {type(exc).__name__}: {_exc_str(exc)}" + )) return _handler @@ -4964,13 +4946,9 @@ def _make_read_resource_handler(server_name: str, tool_timeout: float): """Return a sync handler that reads a resource by URI from an MCP server.""" def _handler(args: dict, **kwargs) -> str: - from tools.registry import tool_error - server = _get_connected_server_for_call(server_name) if not server or not server.session: - return json.dumps({ - "error": f"MCP server '{server_name}' is not connected" - }, ensure_ascii=False) + return tool_error(f"MCP server '{server_name}' is not connected") uri = args.get("uri") if not uri: @@ -5018,11 +4996,9 @@ def _make_read_resource_handler(server_name: str, tool_timeout: float): logger.error( "MCP %s/read_resource failed: %s", server_name, exc, ) - return json.dumps({ - "error": _sanitize_error( - f"MCP call failed: {type(exc).__name__}: {_exc_str(exc)}" - ) - }, ensure_ascii=False) + return tool_error(_sanitize_error( + f"MCP call failed: {type(exc).__name__}: {_exc_str(exc)}" + )) return _handler @@ -5033,9 +5009,7 @@ def _make_list_prompts_handler(server_name: str, tool_timeout: float): def _handler(args: dict, **kwargs) -> str: server = _get_connected_server_for_call(server_name) if not server or not server.session: - return json.dumps({ - "error": f"MCP server '{server_name}' is not connected" - }, ensure_ascii=False) + return tool_error(f"MCP server '{server_name}' is not connected") async def _call(): _mark_server_call_started(server) @@ -5083,11 +5057,9 @@ def _make_list_prompts_handler(server_name: str, tool_timeout: float): logger.error( "MCP %s/list_prompts failed: %s", server_name, exc, ) - return json.dumps({ - "error": _sanitize_error( - f"MCP call failed: {type(exc).__name__}: {_exc_str(exc)}" - ) - }, ensure_ascii=False) + return tool_error(_sanitize_error( + f"MCP call failed: {type(exc).__name__}: {_exc_str(exc)}" + )) return _handler @@ -5096,13 +5068,9 @@ def _make_get_prompt_handler(server_name: str, tool_timeout: float): """Return a sync handler that gets a prompt by name from an MCP server.""" def _handler(args: dict, **kwargs) -> str: - from tools.registry import tool_error - server = _get_connected_server_for_call(server_name) if not server or not server.session: - return json.dumps({ - "error": f"MCP server '{server_name}' is not connected" - }, ensure_ascii=False) + return tool_error(f"MCP server '{server_name}' is not connected") name = args.get("name") if not name: @@ -5154,11 +5122,9 @@ def _make_get_prompt_handler(server_name: str, tool_timeout: float): logger.error( "MCP %s/get_prompt failed: %s", server_name, exc, ) - return json.dumps({ - "error": _sanitize_error( - f"MCP call failed: {type(exc).__name__}: {_exc_str(exc)}" - ) - }, ensure_ascii=False) + return tool_error(_sanitize_error( + f"MCP call failed: {type(exc).__name__}: {_exc_str(exc)}" + )) return _handler diff --git a/tools/path_security.py b/tools/path_security.py index 828011e5d77..9d73a613c52 100644 --- a/tools/path_security.py +++ b/tools/path_security.py @@ -23,7 +23,7 @@ def validate_within_dir(path: Path, root: Path) -> Optional[str]: error = validate_within_dir(user_path, allowed_root) if error: - return json.dumps({"error": error}) + return tool_error(error) """ try: resolved = path.resolve() diff --git a/tools/registry.py b/tools/registry.py index 471f54fb2ee..fb760b519e9 100644 --- a/tools/registry.py +++ b/tools/registry.py @@ -667,12 +667,12 @@ class ToolRegistry: name, result_type, ) - return json.dumps({ - "error": f"Tool handler returned unsupported result type: {result_type}", - "error_type": "tool_result_contract", - "tool": name, - "result_type": result_type, - }, ensure_ascii=False) + return tool_error( + f"Tool handler returned unsupported result type: {result_type}", + error_type="tool_result_contract", + tool=name, + result_type=result_type, + ) def dispatch(self, name: str, args: dict, **kwargs) -> str | dict: """Execute a tool handler by name. @@ -685,7 +685,7 @@ class ToolRegistry: """ entry = self.get_entry(name) if not entry: - return json.dumps({"error": f"Unknown tool: {name}"}) + return tool_error(f"Unknown tool: {name}") try: if entry.is_async: from model_tools import _run_async @@ -704,7 +704,7 @@ class ToolRegistry: sanitized = _sanitize_tool_error(raw) except Exception: sanitized = raw # defensive: never let the sanitizer block error propagation - return json.dumps({"error": sanitized}) + return tool_error(sanitized) # ------------------------------------------------------------------ # Query helpers (replace redundant dicts in model_tools.py) diff --git a/tools/send_message_tool.py b/tools/send_message_tool.py index f6b7dc37d97..d1b71a2eb33 100644 --- a/tools/send_message_tool.py +++ b/tools/send_message_tool.py @@ -383,15 +383,15 @@ def _handle_send(args): if resolved: chat_id, thread_id, _ = _parse_target_ref(platform_name, resolved) else: - return json.dumps({ - "error": f"Could not resolve '{target_ref}' on {platform_name}. " + return tool_error( + f"Could not resolve '{target_ref}' on {platform_name}. " f"Use send_message(action='list') to see available targets." - }) + ) except Exception: - return json.dumps({ - "error": f"Could not resolve '{target_ref}' on {platform_name}. " + return tool_error( + f"Could not resolve '{target_ref}' on {platform_name}. " f"Try using a numeric channel ID instead." - }) + ) from tools.interrupt import is_interrupted if is_interrupted(): @@ -460,11 +460,11 @@ def _handle_send(args): home_env = _HOME_CHANNEL_ENV_OVERRIDES.get( platform_name, f"{platform_name.upper()}_HOME_CHANNEL" ) - return json.dumps({ - "error": f"No home channel set for {platform_name} to determine where to send the message. " + return tool_error( + f"No home channel set for {platform_name} to determine where to send the message. " f"Either specify a channel directly with '{platform_name}:CHANNEL_NAME', " f"or set a home channel via: hermes config set {home_env} " - }) + ) duplicate_skip = _maybe_skip_cron_duplicate_send(platform_name, chat_id, thread_id) if duplicate_skip: diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index 476582d03e1..e6f4bb05350 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -56,6 +56,7 @@ logger = logging.getLogger(__name__) # long-running subprocesses immediately instead of blocking until timeout. # --------------------------------------------------------------------------- from tools.interrupt import is_interrupted, _interrupt_event # noqa: F401 — re-exported +from tools.registry import tool_error # display_hermes_home imported lazily at call site (stale-module safety during hermes update) @@ -2209,13 +2210,11 @@ def terminal_tool( # Reject foreground commands where the model explicitly requests # a timeout above FOREGROUND_MAX_TIMEOUT — nudge it toward background. if not background and timeout and timeout > FOREGROUND_MAX_TIMEOUT: - return json.dumps({ - "error": ( - f"Foreground timeout {timeout}s exceeds the maximum of " - f"{FOREGROUND_MAX_TIMEOUT}s. Use background=true with " - f"notify_on_complete=true for long-running commands." - ), - }, ensure_ascii=False) + return tool_error( + f"Foreground timeout {timeout}s exceeds the maximum of " + f"{FOREGROUND_MAX_TIMEOUT}s. Use background=true with " + f"notify_on_complete=true for long-running commands." + ) # Guardrail: long-lived server/watch commands should run as managed # background sessions, not foreground shell hacks. diff --git a/tools/tool_search.py b/tools/tool_search.py index df1e29b3c03..e286fa06867 100644 --- a/tools/tool_search.py +++ b/tools/tool_search.py @@ -44,6 +44,8 @@ import re from dataclasses import dataclass, field from typing import Any, Dict, Iterable, List, Optional, Tuple +from tools.registry import tool_error + logger = logging.getLogger("tools.tool_search") @@ -868,7 +870,7 @@ def dispatch_tool_search(args: Dict[str, Any], config = load_config() query = str(args.get("query") or "").strip() if not query: - return json.dumps({"error": "query is required"}, ensure_ascii=False) + return tool_error("query is required") raw_limit = args.get("limit") if raw_limit is None: @@ -892,14 +894,12 @@ def dispatch_tool_describe(args: Dict[str, Any], """Execute the ``tool_describe`` bridge tool. Returns a JSON string.""" name = str(args.get("name") or "").strip() if not name: - return json.dumps({"error": "name is required"}, ensure_ascii=False) + return tool_error("name is required") if not is_deferrable_tool_name(name): - return json.dumps({ - "error": ( - f"'{name}' is not a deferrable tool. If you see it in the tools list " - "already, call it directly; otherwise check the spelling against tool_search." - ), - }, ensure_ascii=False) + return tool_error( + f"'{name}' is not a deferrable tool. If you see it in the tools list " + "already, call it directly; otherwise check the spelling against tool_search." + ) _, deferrable = classify_tools(current_tool_defs) for td in deferrable: fn = td.get("function") or {} @@ -909,9 +909,9 @@ def dispatch_tool_describe(args: Dict[str, Any], "description": fn.get("description", ""), "parameters": fn.get("parameters", {}), }, ensure_ascii=False) - return json.dumps({ - "error": f"'{name}' is not currently available. Re-run tool_search to refresh.", - }, ensure_ascii=False) + return tool_error( + f"'{name}' is not currently available. Re-run tool_search to refresh." + ) def scoped_deferrable_names(tool_defs: List[Dict[str, Any]]) -> frozenset[str]: @@ -973,17 +973,15 @@ def validate_deferred_call_args(name: str, args: Dict[str, Any]) -> Optional[str missing = [r for r in required if isinstance(r, str) and r not in args] if not missing: return None - return json.dumps({ - "error": ( - f"tool_call to '{name}' is missing required argument(s): " - f"{', '.join(missing)}. The tool was NOT invoked." - ), - "parameters": params, - "hint": ( + return tool_error( + f"tool_call to '{name}' is missing required argument(s): " + f"{', '.join(missing)}. The tool was NOT invoked.", + parameters=params, + hint=( "Retry tool_call with 'arguments' matching the parameters " "schema above." ), - }, ensure_ascii=False) + ) except Exception: # pragma: no cover — never block dispatch on validator bugs logger.debug("validate_deferred_call_args failed for %s", name, exc_info=True) return None