feat: enable tool_arg_contract by default for native tools (Closes #1530) - #1534
Merged
Merged
Conversation
Flips tool_arg_contract_enabled() default from OFF to ON. The #1528 audit confirmed every native tool (terminal, search_files, tool_call, write_file, patch, browser_console, execute_code, process) ships a safe schema (required arrays + type declarations), and the check itself is fail-open on any schema without a structured contract (no parameters/properties/required), so flipping the default ON only ever blocks calls that genuinely violate the schema the model was given — it cannot invent a stricter contract than the tool declared. This is the capstone (child C) of the #1487 decomposition targeting the 484 parse-error events/7d root cause: the contract check now catches missing-required / invalid-enum / type-mismatch calls in practice rather than only when manually opted in. Escape hatches preserved (in priority order): - HERMES_TOOL_ARG_CONTRACT env var: 0/false/no/off disables; truthy ON - tool_arg_contract.enabled: false in config.yaml disables persistently - config load error -> ON (the check is fail-open, so ON is lower-risk) Test updates: - tests/agent/test_tool_arg_contract.py: gate tests now assert default ON, plus config-disable + config-load-error-defaults-on coverage - tests/test_model_tools.py: contract integration tests reflect the new default-ON blocking; an autouse fixture disables the gate for the hook/middleware tests that reuse web_search's real schema with mocked dispatch and a pre-existing q/query arg mismatch (unrelated to the contract behavior under test) Sibling agent.verify_policy.verify_policy_enabled() stays default-OFF (unchanged) — only tool_arg_contract flips, per the #1528 safe-schema audit. Closes #1530 Co-Authored-By: Hermes Evolution <evolution@hermes.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Capstone (child C) of the #1487 decomposition. Flips
tool_arg_contract_enabled()default from OFF to ON, so the deterministic argument-contract check finally catches the 484 parse-error events/7d failure class in practice rather than only when manually opted in.The #1528 audit (PR #1531) confirmed every native tool ships a safe schema (
requiredarrays +typedeclarations), and the check itself is fail-open on any schema without a structured contract — so flipping the default ON only ever blocks calls that genuinely violate the schema the model was given. It cannot invent a stricter contract than the tool declared.Changes
agent/tool_arg_contract.py(+47, -6):tool_arg_contract_enabled()now returnsTrueby default. Env var (HERMES_TOOL_ARG_CONTRACT=0) and config (tool_arg_contract.enabled: false) escape hatches preserved. Config-load error now falls back to ON (the lower-risk choice given the check is fail-open). Module + function docstrings updated.model_tools.py(+6, -2): in-line gating comment updated to reflect default-ON since [FIX] Enable tool_arg_contract by default for native tools with safe schemas (#1487 child C) #1530.tests/agent/test_tool_arg_contract.py(+41): gate tests now assert default-ON, plus config-disable and config-load-error-defaults-on coverage.tests/test_model_tools.py(+69, -10): contract integration tests reflect the new default-ON blocking behavior; an autouse fixture disables the gate for the hook/middleware tests that reuseweb_search's real schema with a mocked dispatch and a pre-existingq/queryarg mismatch unrelated to the contract behavior under test.Escape hatches (priority order)
HERMES_TOOL_ARG_CONTRACTenv var —0/false/no/offdisables; truthy forces ONtool_arg_contract.enabled: falseinconfig.yaml— persistent disableVerification
ruff checkclean on all 4 changed filestests/agent/test_tool_arg_contract.py+tests/test_model_tools.py: 90 passedtest_dispatch_session_id,test_tool_arg_coercion,test_sanitize_tool_error,test_transform_tool_result_hook,test_tool_circuit_breaker_gate,test_malformed_tool_arguments): 123 passedScope
agent.verify_policy.verify_policy_enabled()stays default-OFF (unchanged) — onlytool_arg_contractflips, per the [FIX] Audit native tool schemas for contract-check compatibility (#1487 child A) #1528 safe-schema audit.env.examplereference the old gate, so no doc updates neededCloses #1530