Skip to content

fix(tool_search): accept sanitized keys in deferred tool_call probe - #72650

Open
Adolanium wants to merge 1 commit into
NousResearch:mainfrom
Adolanium:fix/tool-search-probe-sanitized-keys
Open

fix(tool_search): accept sanitized keys in deferred tool_call probe#72650
Adolanium wants to merge 1 commit into
NousResearch:mainfrom
Adolanium:fix/tool-search-probe-sanitized-keys

Conversation

@Adolanium

Copy link
Copy Markdown
Contributor

What does this PR do?

The Tool Search blind-call probe (validate_deferred_call_args) checked registry wire required names against the nested tool_call arguments. After schema sanitization, the model and tool_describe use renamed keys (for example issue_class_neq for Cloudflare-style issue_class~neq). A complete call with those model-facing keys was treated as missing required fields and never reached unrename_tool_args / dispatch.

That contradicts the probe's contract that valid calls are never blocked, and it breaks deferred MCP tools whose property keys need sanitizer renames (the Cloudflare path #67034 already handles at dispatch).

This PR reverse-maps args with the same unrename_tool_args path coerce_tool_args uses before the key-absence check. Probe error payloads list model-facing missing names and the sanitized parameters object so a repair round-trip matches tool_describe. Fail-open on unrename/validator errors is unchanged. Clean-key tools are unaffected.

Related Issue

Fixes #72648

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Security fix
  • Documentation update
  • Tests (adding or improving test coverage)
  • Refactor (no behavior change)
  • New skill (bundled or hub)

Changes Made

  • tools/tool_search.py: unrename deferred tool_call args before required-key checks in validate_deferred_call_args. Build model-facing parameters for probe errors via the schema sanitizer so error text and parameters match tool_describe.
  • tests/tools/test_tool_search.py: add TestDeferredCallSchemaProbe cases for sanitized required keys (accept, still-block missing, end-to-end dispatch, blind-call error naming), using the flat MCP schema shape.

How to Test

  1. python -m pytest tests/tools/test_tool_search.py::TestDeferredCallSchemaProbe -q - 10 passed
  2. python -m pytest tests/tools/test_tool_search.py tests/tools/test_schema_sanitizer.py -q - 112 passed
  3. Proof of the bug on main: register an MCP-shaped tool with required issue_class~neq, call handle_function_call("tool_call", arguments={"issue_class_neq": "...", "zone_id": "..."}). On main the probe returns missing issue_class~neq. With this branch the tool dispatches.
  4. Existing clean-key cases still block empty blind calls and still dispatch when document_id is present.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature
  • I've run pytest tests/tools/test_tool_search.py tests/tools/test_schema_sanitizer.py -q and all tests pass (112). Full suite not re-run here. Change is isolated to tool_search probe + tests.
  • I've added tests for my changes
  • I've tested on my platform: Windows 11

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) - or N/A (docstring update only in the probe)
  • I've updated cli-config.yaml.example if I added/changed config keys - or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows - or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide - or N/A (pure Python key mapping)
  • I've updated tool descriptions/schemas if I changed tool behavior - or N/A (dispatch shape unchanged for successful calls)

Screenshots / Logs

# Before (main): sanitized keys blocked
{"error": "tool_call to 'mcp_cf_probe_filter' is missing required argument(s): issue_class~neq. The tool was NOT invoked.", ...}

# After: same call dispatches
{"ok": true, "issue_class_neq": "bug", "zone_id": "z1"}

@alt-glitch alt-glitch added type/bug Something isn't working comp/tools Tool registry, model_tools, toolsets tool/mcp MCP client and OAuth P2 Medium — degraded but workaround exists labels Jul 27, 2026
@Adolanium
Adolanium force-pushed the fix/tool-search-probe-sanitized-keys branch from daccf9a to e2e4ea7 Compare July 27, 2026 16:24
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression fix. Current main still has the key-namespace mismatch: tools/tool_search.py:973 checks registry-wire required names against model arguments, while schemas are sanitized before Tool Search assembly (model_tools.py:546-547) and are reverse-mapped only at dispatch (model_tools.py:734-740). The shared validator is also used by both agent executor paths (agent/tool_executor.py:713 and agent/tool_executor.py:1387).

Suggested changes

  • During salvage, retain current main's tool_error(...) envelope at tools/tool_search.py:976-984 (introduced by 1a7f73b8ea) while applying the proposed model-facing parameter/error mapping.
  • Restore the added regression cases beside TestDeferredCallSchemaProbe at tests/tools/test_tool_search.py:450-518; 39975613b1 pruned the former insertion context.

This is an automated hermes-sweeper review.

The blind-call probe checked registry wire required names, while the model
and tool_describe use schema-sanitizer keys (e.g. issue_class_neq for
issue_class~neq). Unrename args before the key-absence check and return
model-facing parameters in the probe error so repair matches tool_describe.
@Adolanium
Adolanium force-pushed the fix/tool-search-probe-sanitized-keys branch from e2e4ea7 to dcbb69c Compare July 30, 2026 14:32
@Adolanium

Copy link
Copy Markdown
Contributor Author

Salvaged onto current main in dcbb69c. The probe unrenames model args with the same unrename_tool_args path coerce_tool_args uses before the required-key check, so a call that matches tool_describe no longer gets rejected as missing required fields. The error keeps the tool_error envelope from 1a7f73b, now carrying the model-facing parameter schema and sanitized missing names so a repair round-trip matches what tool_describe shows.

The regression cases are restored beside TestDeferredCallSchemaProbe. tests/tools/test_tool_search.py passes 31/31.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 30, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary

One PR addresses #72648. #72650 fixes the reported namespace mismatch by reverse-mapping sanitized model-facing keys before required-field validation and returning model-facing names and parameters in repair errors.

Related pull requests

Suggested consolidation

Keep #72650 open with a salvage path: retain the reverse-mapping and model-facing error-schema changes in tools/tool_search.py together with the four regression cases in tests/tools/test_tool_search.py. It is the only PR in this complex, directly addresses #72648, and has no duplicate PRs to close.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I72648(["issue #72648 (open)"])
    P72650["PR #72650 (open)"]
    P72650 -->|best fix| I72648
    class I72648 open
    class P72650 open
    class P72650 best
    class P72650 target
    click I72648 "https://github.com/NousResearch/hermes-agent/issues/72648"
    click P72650 "https://github.com/NousResearch/hermes-agent/pull/72650"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 10 kB of PR diffs, 9 kB of issue/PR text, <1 kB of discussion (1 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform tool/mcp MCP client and OAuth type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: tool_call probe rejects valid sanitized property keys after schema rename

4 participants