fix(agent): validate context/memory tool schemas before wrapping (#47707) - #47712
Closed
Bartok9 wants to merge 1 commit into
Closed
fix(agent): validate context/memory tool schemas before wrapping (#47707)#47712Bartok9 wants to merge 1 commit into
Bartok9 wants to merge 1 commit into
Conversation
Closes NousResearch#47707 Context engines and memory providers expose tool schemas via get_tool_schemas(). agent_init.py wrapped each as {"type":"function","function":_schema} without validating that _schema carries a top-level name. A provider returning an entry already in OpenAI tool form ({"type":"function","function":{...}}) was then double-wrapped into a tool whose function has no name. Strict providers (e.g. DeepSeek) reject the entire request with HTTP 400 'tools[N].function: missing field name', so one malformed schema silently disables the whole toolset and breaks every turn. The schema was also never added to valid_tool_names, so even lenient providers could not call it. Add a shared normalize_tool_schema() helper that unwraps an already-wrapped entry and returns None for anything lacking a resolvable string name. Wire it into the agent_init context-engine loop and all three memory_manager surfaces (inject_memory_provider_tools, add_provider routing index, get_all_tool_schemas), so a single bad plugin schema is skipped with a warning instead of poisoning the request. Verification: 209 targeted agent/memory tests pass (incl. 9 new). New tests assert the unwrap + skip-nameless behavior and fail without the fix.
Contributor
|
Thanks for the thorough fix. This is an automated hermes-sweeper review: the exact implementation has already landed on
|
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
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
Motivation
Closes #47707.
In
agent/agent_init.py, schemas returned by a context engine'sget_tool_schemas()were wrapped as{"type": "function", "function": _schema}without validating that_schemahas a top-levelname. If a provider returns an entry already in OpenAI tool form ({"type":"function","function":{...}}), the result is double-wrapped into a tool whosefunctionhas no top-levelname. Strict providers (e.g. DeepSeek) reject the entire request withtools[N].function: missing field name(HTTP 400), so a single bad tool silently disables the whole toolset and breaks every turn. The tool was also never added tovalid_tool_names, so even lenient providers couldn't call it. The same unguarded pattern lived inagent/memory_manager.py.Fix
Add a shared
normalize_tool_schema()helper inmemory_manager.pythat:Nonefor anything lacking a resolvable top-level stringname.Wire it into:
agent_init.pycontext-engine injection loop (skip-with-warning instead of appending a nameless tool),inject_memory_provider_tools(),MemoryManager.add_provider()routing index, andMemoryManager.get_all_tool_schemas().So a single bad plugin schema is skipped (or, if merely double-wrapped but otherwise valid, correctly unwrapped) instead of poisoning the request, with a clear log line pointing at the offending provider.
Verification
python3 -m pytest tests/agent/test_memory_provider.py— 100 passed (incl. 9 new)python3 -m pytest tests/agent/ -k "memory or tool_schema or context_engine or inject"— 209 passedruff check agent/memory_manager.py agent/agent_init.py— cleanfunction).Real behavior proof
{"type":"function","function":{"name":"x_grep",...}}was blind-wrapped a second time into{"type":"function","function":{"type":"function","function":{...}}}— a tool whosefunctionhas no top-levelname→ DeepSeek HTTP 400.fix/47707-validate-context-tool-schema.tests/agent/test_memory_provider.py::TestMemoryInjectionRejectsMalformedSchema::test_already_wrapped_schema_is_unwrapped_not_poisoned(+test_nameless_schema_is_skipped,test_good_schema_still_injected_alongside_bad, andTestNormalizeToolSchema).