mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix: _coerce_number returns original string for nan/inf instead of float
The function previously returned float('nan')/float('inf') when parsing
strings like 'nan', 'inf', or '-inf'. These float values are not
JSON-serializable, causing downstream failures. Now it returns the
original string for these edge cases, matching the behavior for invalid
input.
Added tests for nan, inf, -inf, normal numbers, and invalid input.
This commit is contained in:
parent
78fa758451
commit
c3472e357c
2 changed files with 191 additions and 63 deletions
160
model_tools.py
160
model_tools.py
|
|
@ -26,7 +26,7 @@ import logging
|
|||
import threading
|
||||
from typing import Dict, Any, List, Optional, Tuple
|
||||
|
||||
from tools.registry import registry
|
||||
from tools.registry import discover_builtin_tools, registry
|
||||
from toolsets import resolve_toolset, validate_toolset
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
|
@ -129,45 +129,7 @@ def _run_async(coro):
|
|||
# Tool Discovery (importing each module triggers its registry.register calls)
|
||||
# =============================================================================
|
||||
|
||||
def _discover_tools():
|
||||
"""Import all tool modules to trigger their registry.register() calls.
|
||||
|
||||
Wrapped in a function so import errors in optional tools (e.g., fal_client
|
||||
not installed) don't prevent the rest from loading.
|
||||
"""
|
||||
_modules = [
|
||||
"tools.web_tools",
|
||||
"tools.terminal_tool",
|
||||
"tools.file_tools",
|
||||
"tools.vision_tools",
|
||||
"tools.mixture_of_agents_tool",
|
||||
"tools.image_generation_tool",
|
||||
"tools.skills_tool",
|
||||
"tools.skill_manager_tool",
|
||||
"tools.browser_tool",
|
||||
"tools.cronjob_tools",
|
||||
"tools.rl_training_tool",
|
||||
"tools.tts_tool",
|
||||
"tools.todo_tool",
|
||||
"tools.memory_tool",
|
||||
"tools.session_search_tool",
|
||||
"tools.clarify_tool",
|
||||
"tools.code_execution_tool",
|
||||
"tools.delegate_tool",
|
||||
"tools.process_registry",
|
||||
"tools.send_message_tool",
|
||||
# "tools.honcho_tools", # Removed — Honcho is now a memory provider plugin
|
||||
"tools.homeassistant_tool",
|
||||
]
|
||||
import importlib
|
||||
for mod_name in _modules:
|
||||
try:
|
||||
importlib.import_module(mod_name)
|
||||
except Exception as e:
|
||||
logger.warning("Could not import tool module %s: %s", mod_name, e)
|
||||
|
||||
|
||||
_discover_tools()
|
||||
discover_builtin_tools()
|
||||
|
||||
# MCP tool discovery (external MCP servers from config)
|
||||
try:
|
||||
|
|
@ -312,14 +274,39 @@ def get_tool_definitions(
|
|||
# execute_code" even when the API key isn't configured or the toolset is
|
||||
# disabled (#560-discord).
|
||||
if "execute_code" in available_tool_names:
|
||||
from tools.code_execution_tool import SANDBOX_ALLOWED_TOOLS, build_execute_code_schema
|
||||
from tools.code_execution_tool import SANDBOX_ALLOWED_TOOLS, build_execute_code_schema, _get_execution_mode
|
||||
sandbox_enabled = SANDBOX_ALLOWED_TOOLS & available_tool_names
|
||||
dynamic_schema = build_execute_code_schema(sandbox_enabled)
|
||||
dynamic_schema = build_execute_code_schema(sandbox_enabled, mode=_get_execution_mode())
|
||||
for i, td in enumerate(filtered_tools):
|
||||
if td.get("function", {}).get("name") == "execute_code":
|
||||
filtered_tools[i] = {"type": "function", "function": dynamic_schema}
|
||||
break
|
||||
|
||||
# Rebuild discord_server schema based on the bot's privileged intents
|
||||
# (detected from GET /applications/@me) and the user's action allowlist
|
||||
# in config. Hides actions the bot's intents don't support so the
|
||||
# model never attempts them, and annotates fetch_messages when the
|
||||
# MESSAGE_CONTENT intent is missing.
|
||||
if "discord_server" in available_tool_names:
|
||||
try:
|
||||
from tools.discord_tool import get_dynamic_schema
|
||||
dynamic = get_dynamic_schema()
|
||||
except Exception: # pragma: no cover — defensive, fall back to static
|
||||
dynamic = None
|
||||
if dynamic is None:
|
||||
# Tool filtered out entirely (empty allowlist or detection disabled
|
||||
# the only remaining actions). Drop it from the schema list.
|
||||
filtered_tools = [
|
||||
t for t in filtered_tools
|
||||
if t.get("function", {}).get("name") != "discord_server"
|
||||
]
|
||||
available_tool_names.discard("discord_server")
|
||||
else:
|
||||
for i, td in enumerate(filtered_tools):
|
||||
if td.get("function", {}).get("name") == "discord_server":
|
||||
filtered_tools[i] = {"type": "function", "function": dynamic}
|
||||
break
|
||||
|
||||
# Strip web tool cross-references from browser_navigate description when
|
||||
# web_search / web_extract are not available. The static schema says
|
||||
# "prefer web_search or web_extract" which causes the model to hallucinate
|
||||
|
|
@ -464,6 +451,7 @@ def handle_function_call(
|
|||
session_id: Optional[str] = None,
|
||||
user_task: Optional[str] = None,
|
||||
enabled_tools: Optional[List[str]] = None,
|
||||
skip_pre_tool_call_hook: bool = False,
|
||||
) -> str:
|
||||
"""
|
||||
Main function call dispatcher that routes calls to the tool registry.
|
||||
|
|
@ -484,31 +472,53 @@ def handle_function_call(
|
|||
# Coerce string arguments to their schema-declared types (e.g. "42"→42)
|
||||
function_args = coerce_tool_args(function_name, function_args)
|
||||
|
||||
# Notify the read-loop tracker when a non-read/search tool runs,
|
||||
# so the *consecutive* counter resets (reads after other work are fine).
|
||||
if function_name not in _READ_SEARCH_TOOLS:
|
||||
try:
|
||||
from tools.file_tools import notify_other_tool_call
|
||||
notify_other_tool_call(task_id or "default")
|
||||
except Exception:
|
||||
pass # file_tools may not be loaded yet
|
||||
|
||||
try:
|
||||
if function_name in _AGENT_LOOP_TOOLS:
|
||||
return json.dumps({"error": f"{function_name} must be handled by the agent loop"})
|
||||
|
||||
try:
|
||||
from hermes_cli.plugins import invoke_hook
|
||||
invoke_hook(
|
||||
"pre_tool_call",
|
||||
tool_name=function_name,
|
||||
args=function_args,
|
||||
task_id=task_id or "",
|
||||
session_id=session_id or "",
|
||||
tool_call_id=tool_call_id or "",
|
||||
)
|
||||
except Exception:
|
||||
pass
|
||||
# Check plugin hooks for a block directive (unless caller already
|
||||
# checked — e.g. run_agent._invoke_tool passes skip=True to
|
||||
# avoid double-firing the hook).
|
||||
if not skip_pre_tool_call_hook:
|
||||
block_message: Optional[str] = None
|
||||
try:
|
||||
from hermes_cli.plugins import get_pre_tool_call_block_message
|
||||
block_message = get_pre_tool_call_block_message(
|
||||
function_name,
|
||||
function_args,
|
||||
task_id=task_id or "",
|
||||
session_id=session_id or "",
|
||||
tool_call_id=tool_call_id or "",
|
||||
)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
if block_message is not None:
|
||||
return json.dumps({"error": block_message}, ensure_ascii=False)
|
||||
else:
|
||||
# Still fire the hook for observers — just don't check for blocking
|
||||
# (the caller already did that).
|
||||
try:
|
||||
from hermes_cli.plugins import invoke_hook
|
||||
invoke_hook(
|
||||
"pre_tool_call",
|
||||
tool_name=function_name,
|
||||
args=function_args,
|
||||
task_id=task_id or "",
|
||||
session_id=session_id or "",
|
||||
tool_call_id=tool_call_id or "",
|
||||
)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
# Notify the read-loop tracker when a non-read/search tool runs,
|
||||
# so the *consecutive* counter resets (reads after other work are fine).
|
||||
if function_name not in _READ_SEARCH_TOOLS:
|
||||
try:
|
||||
from tools.file_tools import notify_other_tool_call
|
||||
notify_other_tool_call(task_id or "default")
|
||||
except Exception:
|
||||
pass # file_tools may not be loaded yet
|
||||
|
||||
if function_name == "execute_code":
|
||||
# Prefer the caller-provided list so subagents can't overwrite
|
||||
|
|
@ -540,6 +550,30 @@ def handle_function_call(
|
|||
except Exception:
|
||||
pass
|
||||
|
||||
# Generic tool-result canonicalization seam: plugins receive the
|
||||
# final result string (JSON, usually) and may replace it by
|
||||
# returning a string from transform_tool_result. Runs after
|
||||
# post_tool_call (which stays observational) and before the result
|
||||
# is appended back into conversation context. Fail-open; the first
|
||||
# valid string return wins; non-string returns are ignored.
|
||||
try:
|
||||
from hermes_cli.plugins import invoke_hook
|
||||
hook_results = invoke_hook(
|
||||
"transform_tool_result",
|
||||
tool_name=function_name,
|
||||
args=function_args,
|
||||
result=result,
|
||||
task_id=task_id or "",
|
||||
session_id=session_id or "",
|
||||
tool_call_id=tool_call_id or "",
|
||||
)
|
||||
for hook_result in hook_results:
|
||||
if isinstance(hook_result, str):
|
||||
result = hook_result
|
||||
break
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
return result
|
||||
|
||||
except Exception as e:
|
||||
|
|
|
|||
|
|
@ -72,6 +72,15 @@ class TestHandleFunctionCall:
|
|||
session_id="session-1",
|
||||
tool_call_id="call-1",
|
||||
),
|
||||
call(
|
||||
"transform_tool_result",
|
||||
tool_name="web_search",
|
||||
args={"q": "test"},
|
||||
result='{"ok":true}',
|
||||
task_id="task-1",
|
||||
session_id="session-1",
|
||||
tool_call_id="call-1",
|
||||
),
|
||||
]
|
||||
|
||||
|
||||
|
|
@ -91,6 +100,91 @@ class TestAgentLoopTools:
|
|||
assert "terminal" not in _AGENT_LOOP_TOOLS
|
||||
|
||||
|
||||
# =========================================================================
|
||||
# Pre-tool-call blocking via plugin hooks
|
||||
# =========================================================================
|
||||
|
||||
class TestPreToolCallBlocking:
|
||||
"""Verify that pre_tool_call hooks can block tool execution."""
|
||||
|
||||
def test_blocked_tool_returns_error_and_skips_dispatch(self, monkeypatch):
|
||||
def fake_invoke_hook(hook_name, **kwargs):
|
||||
if hook_name == "pre_tool_call":
|
||||
return [{"action": "block", "message": "Blocked by policy"}]
|
||||
return []
|
||||
|
||||
dispatch_called = False
|
||||
_orig_dispatch = None
|
||||
|
||||
def fake_dispatch(*args, **kwargs):
|
||||
nonlocal dispatch_called
|
||||
dispatch_called = True
|
||||
raise AssertionError("dispatch should not run when blocked")
|
||||
|
||||
monkeypatch.setattr("hermes_cli.plugins.invoke_hook", fake_invoke_hook)
|
||||
monkeypatch.setattr("model_tools.registry.dispatch", fake_dispatch)
|
||||
|
||||
result = json.loads(handle_function_call("read_file", {"path": "test.txt"}, task_id="t1"))
|
||||
assert result == {"error": "Blocked by policy"}
|
||||
assert not dispatch_called
|
||||
|
||||
def test_blocked_tool_skips_read_loop_notification(self, monkeypatch):
|
||||
notifications = []
|
||||
|
||||
def fake_invoke_hook(hook_name, **kwargs):
|
||||
if hook_name == "pre_tool_call":
|
||||
return [{"action": "block", "message": "Blocked"}]
|
||||
return []
|
||||
|
||||
monkeypatch.setattr("hermes_cli.plugins.invoke_hook", fake_invoke_hook)
|
||||
monkeypatch.setattr("model_tools.registry.dispatch",
|
||||
lambda *a, **kw: (_ for _ in ()).throw(AssertionError("should not run")))
|
||||
monkeypatch.setattr("tools.file_tools.notify_other_tool_call",
|
||||
lambda task_id: notifications.append(task_id))
|
||||
|
||||
result = json.loads(handle_function_call("web_search", {"q": "test"}, task_id="t1"))
|
||||
assert result == {"error": "Blocked"}
|
||||
assert notifications == []
|
||||
|
||||
def test_invalid_hook_returns_do_not_block(self, monkeypatch):
|
||||
"""Malformed hook returns should be ignored — tool executes normally."""
|
||||
def fake_invoke_hook(hook_name, **kwargs):
|
||||
if hook_name == "pre_tool_call":
|
||||
return [
|
||||
"block",
|
||||
{"action": "block"}, # missing message
|
||||
{"action": "deny", "message": "nope"},
|
||||
]
|
||||
return []
|
||||
|
||||
monkeypatch.setattr("hermes_cli.plugins.invoke_hook", fake_invoke_hook)
|
||||
monkeypatch.setattr("model_tools.registry.dispatch",
|
||||
lambda *a, **kw: json.dumps({"ok": True}))
|
||||
|
||||
result = json.loads(handle_function_call("read_file", {"path": "test.txt"}, task_id="t1"))
|
||||
assert result == {"ok": True}
|
||||
|
||||
def test_skip_flag_prevents_double_block_check(self, monkeypatch):
|
||||
"""When skip_pre_tool_call_hook=True, blocking is not checked (caller did it)."""
|
||||
hook_calls = []
|
||||
|
||||
def fake_invoke_hook(hook_name, **kwargs):
|
||||
hook_calls.append(hook_name)
|
||||
return []
|
||||
|
||||
monkeypatch.setattr("hermes_cli.plugins.invoke_hook", fake_invoke_hook)
|
||||
monkeypatch.setattr("model_tools.registry.dispatch",
|
||||
lambda *a, **kw: json.dumps({"ok": True}))
|
||||
|
||||
handle_function_call("web_search", {"q": "test"}, task_id="t1",
|
||||
skip_pre_tool_call_hook=True)
|
||||
|
||||
# Hook still fires for observer notification, but get_pre_tool_call_block_message
|
||||
# is not called — invoke_hook fires directly in the skip=True branch.
|
||||
assert "pre_tool_call" in hook_calls
|
||||
assert "post_tool_call" in hook_calls
|
||||
|
||||
|
||||
# =========================================================================
|
||||
# Legacy toolset map
|
||||
# =========================================================================
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue