mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-27 17:58:07 +00:00
feat(tool_search): probe-validate blind tool_call args against the deferred schema
Port from nearai/ironclaw#5149 (the describe-first live-hardening fix in their progressive tool disclosure work): when a model invokes a deferred tool through the tool_call bridge without the schema-required arguments, return the tool's parameter schema instead of dispatching blind. Pre-fix, a blind call produced an opaque downstream failure ("[TOOL_ERROR] Tool execution failed: KeyError: 'document_id'") that teaches the model nothing about what the tool expects — IronClaw observed cheap models looping ~30 identical invalid calls until the iteration budget died. Post-fix, the model repairs the call in one round-trip. - tools/tool_search.py: new validate_deferred_call_args() — key-absence check of schema 'required' fields only; no type checking (coerce_tool_args already repairs types downstream); fails open on any validator error so it can never block a legitimate dispatch. - model_tools.py: probe after the scope gate in the bridge dispatch. - agent/tool_executor.py: probe in both unwrap sites (concurrent + sequential) before the underlying tool replaces the bridge; sequential path flattens the payload to match its {"error": str} wrapping. - tests: TestDeferredCallSchemaProbe — blind call returns schema (not KeyError), valid/optional calls dispatch, unvalidatable tools fail open, out-of-scope rejection unchanged.
This commit is contained in:
parent
9b97dea1e6
commit
8fbe2e388f
4 changed files with 199 additions and 4 deletions
|
|
@ -431,8 +431,15 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe
|
|||
_underlying, _underlying_args, _err = _ts.resolve_underlying_call(function_args)
|
||||
if not _err and _underlying:
|
||||
if _underlying in _tool_search_scoped_names(agent):
|
||||
function_name = _underlying
|
||||
function_args = _underlying_args
|
||||
# Probe-validate before unwrapping (ironclaw#5149):
|
||||
# missing required args return the parameter schema
|
||||
# instead of dispatching into an opaque failure.
|
||||
_probe_err = _ts.validate_deferred_call_args(_underlying, _underlying_args)
|
||||
if _probe_err is not None:
|
||||
_ts_scope_block = _probe_err
|
||||
else:
|
||||
function_name = _underlying
|
||||
function_args = _underlying_args
|
||||
else:
|
||||
_ts_scope_block = json.dumps({
|
||||
"error": (
|
||||
|
|
@ -1113,8 +1120,25 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe
|
|||
_underlying, _underlying_args, _err = _ts.resolve_underlying_call(function_args)
|
||||
if not _err and _underlying:
|
||||
if _underlying in _tool_search_scoped_names(agent):
|
||||
function_name = _underlying
|
||||
function_args = _underlying_args
|
||||
# Probe-validate before unwrapping (ironclaw#5149):
|
||||
# missing required args return the parameter schema
|
||||
# instead of dispatching into an opaque failure.
|
||||
_probe_err = _ts.validate_deferred_call_args(_underlying, _underlying_args)
|
||||
if _probe_err is not None:
|
||||
# This path wraps _block_msg in {"error": ...} —
|
||||
# flatten the probe payload to one plain string.
|
||||
try:
|
||||
_probe = json.loads(_probe_err)
|
||||
_ts_scope_block = (
|
||||
f"{_probe.get('error', '')} Parameters schema: "
|
||||
f"{json.dumps(_probe.get('parameters', {}), ensure_ascii=False)}. "
|
||||
f"{_probe.get('hint', '')}"
|
||||
).strip()
|
||||
except Exception:
|
||||
_ts_scope_block = _probe_err
|
||||
else:
|
||||
function_name = _underlying
|
||||
function_args = _underlying_args
|
||||
else:
|
||||
_ts_scope_block = (
|
||||
f"'{_underlying}' is not available in this session. "
|
||||
|
|
|
|||
|
|
@ -1185,6 +1185,12 @@ def handle_function_call(
|
|||
"Use tool_search to find tools you can call."
|
||||
),
|
||||
}, ensure_ascii=False)
|
||||
# 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.
|
||||
_probe_err = _ts_mod.validate_deferred_call_args(underlying_name, underlying_args)
|
||||
if _probe_err is not None:
|
||||
return _probe_err
|
||||
# Recurse with the underlying tool. All hooks fire against the
|
||||
# real tool name. The bridge is invisible to hooks by design.
|
||||
return handle_function_call(
|
||||
|
|
|
|||
|
|
@ -739,3 +739,112 @@ class TestCatalogListing:
|
|||
assert result.activated
|
||||
search = next(t for t in result.tool_defs if t["function"]["name"] == "tool_search")
|
||||
assert "mcp_x_0" not in search["function"]["description"]
|
||||
|
||||
|
||||
class TestDeferredCallSchemaProbe:
|
||||
"""Blind tool_call invocations missing required arguments must return
|
||||
the tool's parameter schema instead of dispatching into an opaque
|
||||
downstream failure (port of nearai/ironclaw#5149's describe-first fix).
|
||||
|
||||
A deferred tool's schema is invisible until tool_describe is called, so
|
||||
models routinely invoke deferred tools by name alone. Pre-fix, that
|
||||
produced ``KeyError: 'document_id'``-style errors that teach the model
|
||||
nothing; post-fix, the probe returns the schema so the model repairs
|
||||
the call in one round-trip. Valid calls dispatch untouched.
|
||||
"""
|
||||
|
||||
@staticmethod
|
||||
def _register(name, toolset, required=("document_id",)):
|
||||
from tools.registry import registry
|
||||
|
||||
def _handler(args, task_id=None, **kw):
|
||||
# Simulates a tool that crashes opaquely on a missing required arg.
|
||||
return json.dumps({"ok": True, "doc": args["document_id"]})
|
||||
|
||||
params = {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"document_id": {"type": "string", "description": "Doc id"},
|
||||
"format": {"type": "string"},
|
||||
},
|
||||
"required": list(required),
|
||||
}
|
||||
registry.register(
|
||||
name=name,
|
||||
handler=_handler,
|
||||
schema={"type": "function",
|
||||
"function": {"name": name, "description": f"desc {name}",
|
||||
"parameters": params}},
|
||||
toolset=toolset,
|
||||
)
|
||||
|
||||
def test_validator_returns_schema_for_missing_required(self):
|
||||
from tools.tool_search import validate_deferred_call_args
|
||||
|
||||
self._register("mcp_probe_docs_get", "mcp-probe")
|
||||
err = validate_deferred_call_args("mcp_probe_docs_get", {})
|
||||
assert err is not None
|
||||
parsed = json.loads(err)
|
||||
assert "document_id" in parsed["error"]
|
||||
assert "NOT invoked" in parsed["error"]
|
||||
assert parsed["parameters"]["required"] == ["document_id"]
|
||||
assert "document_id" in parsed["parameters"]["properties"]
|
||||
|
||||
def test_validator_passes_valid_and_optional_only_calls(self):
|
||||
from tools.tool_search import validate_deferred_call_args
|
||||
|
||||
self._register("mcp_probe_docs_get2", "mcp-probe")
|
||||
# All required present → dispatch.
|
||||
assert validate_deferred_call_args(
|
||||
"mcp_probe_docs_get2", {"document_id": "abc"}) is None
|
||||
# Extra optional args don't matter.
|
||||
assert validate_deferred_call_args(
|
||||
"mcp_probe_docs_get2", {"document_id": "abc", "format": "md"}) is None
|
||||
|
||||
def test_validator_never_blocks_unvalidatable_tools(self):
|
||||
from tools.tool_search import validate_deferred_call_args
|
||||
|
||||
# Unknown tool → no schema → dispatch (downstream scope gate handles it).
|
||||
assert validate_deferred_call_args("mcp_no_such_tool_xyz", {}) is None
|
||||
|
||||
def test_validator_no_required_list_dispatches(self):
|
||||
from tools.tool_search import validate_deferred_call_args
|
||||
from tools.registry import registry
|
||||
|
||||
registry.register(
|
||||
name="mcp_probe_norequired",
|
||||
handler=lambda args, task_id=None, **kw: json.dumps({"ok": True}),
|
||||
schema={"type": "function",
|
||||
"function": {"name": "mcp_probe_norequired",
|
||||
"description": "d",
|
||||
"parameters": {"type": "object", "properties": {}}}},
|
||||
toolset="mcp-probe",
|
||||
)
|
||||
assert validate_deferred_call_args("mcp_probe_norequired", {}) is None
|
||||
|
||||
def test_blind_tool_call_returns_schema_not_keyerror(self):
|
||||
import model_tools
|
||||
|
||||
self._register("mcp_probe_blind_op", "mcp-probe-blind")
|
||||
result = json.loads(model_tools.handle_function_call(
|
||||
function_name="tool_call",
|
||||
function_args={"name": "mcp_probe_blind_op", "arguments": {}},
|
||||
enabled_toolsets=["mcp-probe-blind"],
|
||||
))
|
||||
assert "error" in result
|
||||
assert "KeyError" not in result["error"]
|
||||
assert "missing required argument" in result["error"]
|
||||
assert result["parameters"]["required"] == ["document_id"]
|
||||
|
||||
def test_valid_tool_call_still_dispatches(self):
|
||||
import model_tools
|
||||
|
||||
self._register("mcp_probe_valid_op", "mcp-probe-valid")
|
||||
result = json.loads(model_tools.handle_function_call(
|
||||
function_name="tool_call",
|
||||
function_args={"name": "mcp_probe_valid_op",
|
||||
"arguments": {"document_id": "abc"}},
|
||||
enabled_toolsets=["mcp-probe-valid"],
|
||||
))
|
||||
assert result.get("ok") is True
|
||||
assert result.get("doc") == "abc"
|
||||
|
|
|
|||
|
|
@ -934,6 +934,61 @@ def scoped_deferrable_names(tool_defs: List[Dict[str, Any]]) -> frozenset[str]:
|
|||
return frozenset(names)
|
||||
|
||||
|
||||
def validate_deferred_call_args(name: str, args: Dict[str, Any]) -> Optional[str]:
|
||||
"""Probe-validate ``tool_call`` arguments against the deferred tool's schema.
|
||||
|
||||
A deferred tool's parameter schema is invisible to the model until it
|
||||
calls ``tool_describe`` — so models routinely invoke deferred tools
|
||||
"blind" by name alone, omitting required arguments. Dispatching such a
|
||||
call produces an opaque downstream failure (``KeyError: 'document_id'``)
|
||||
that tells the model nothing about what the tool expects, and cheap
|
||||
models loop on it until the iteration budget dies.
|
||||
|
||||
Port of the describe-first probe-validation fix from nearai/ironclaw#5149:
|
||||
when required arguments are missing, return the tool's parameter schema
|
||||
instead of dispatching blind — the model repairs the call in one
|
||||
round-trip. Valid calls (and any call we can't confidently validate)
|
||||
dispatch untouched, so this can never block a legitimate invocation.
|
||||
|
||||
Only *key absence* of schema-``required`` fields counts as invalid.
|
||||
No type checking, no null rejection — nullable/typed edge cases are the
|
||||
tool's own business, and ``coerce_tool_args`` already handles type repair
|
||||
downstream. Returns a JSON error string when invalid, ``None`` when the
|
||||
call should dispatch.
|
||||
"""
|
||||
try:
|
||||
from tools.registry import registry as _registry
|
||||
schema = _registry.get_schema(name)
|
||||
if not isinstance(schema, dict):
|
||||
return None
|
||||
fn = schema.get("function") if schema.get("type") == "function" else schema
|
||||
if not isinstance(fn, dict):
|
||||
return None
|
||||
params = fn.get("parameters")
|
||||
if not isinstance(params, dict):
|
||||
return None
|
||||
required = params.get("required")
|
||||
if not isinstance(required, list) or not required:
|
||||
return None
|
||||
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": (
|
||||
"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
|
||||
|
||||
|
||||
def resolve_underlying_call(args: Dict[str, Any]) -> Tuple[Optional[str], Dict[str, Any], Optional[str]]:
|
||||
"""Parse a ``tool_call`` invocation into (underlying_name, args, error_msg).
|
||||
|
||||
|
|
@ -992,4 +1047,5 @@ __all__ = [
|
|||
"dispatch_tool_describe",
|
||||
"resolve_underlying_call",
|
||||
"scoped_deferrable_names",
|
||||
"validate_deferred_call_args",
|
||||
]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue