mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-08 03:01:47 +00:00
fix(memory): remove dead allOf schema block at the source
PR #21238 introduced top-level `allOf: [{if/then/required}]` blocks in the built-in memory tool's parameters schema as conditional-required hints. Two problems: 1. OpenAI's Codex backend (chatgpt.com/backend-api/codex, gpt-5.x) rejects top-level `allOf`/`anyOf`/`oneOf`/`enum`/`not` outright with a non-retryable 400 — affected every user on openai-codex/gpt-5.x. 2. The `if/then` hints were silently ignored by every other provider (Chat Completions doesn't honour them on function schemas), so they never actually enforced anything anywhere. The runtime handler in `memory_tool()` already validates the per-action required fields and returns actionable error messages, so removing the block changes nothing behaviourally. Paired with the defense-in-depth sanitizer in the previous commit, this closes the bug both at the source (schema no longer emits the forbidden form) and at the wire boundary (sanitizer strips it if anything else re-introduces it). - Rewrites `tests/tools/test_memory_tool_schema.py` to guard against regressing the forbidden-combinator shape instead of asserting it. - Adds AUTHOR_MAP entry for @hrkzogw (author of the sanitizer fix).
This commit is contained in:
parent
3924cb408b
commit
5a3e5b23d2
3 changed files with 38 additions and 50 deletions
|
|
@ -1,38 +1,48 @@
|
|||
"""Schema-shape tests for the built-in memory tool.
|
||||
|
||||
The memory tool previously used ``allOf: [{if: ..., then: {required: ...}}]``
|
||||
at the top level of ``parameters`` to hint per-action required fields. That
|
||||
form was:
|
||||
|
||||
1. Ignored by every provider (Chat Completions doesn't honour ``if/then``
|
||||
on function schemas), so it never actually enforced anything.
|
||||
2. **Rejected outright by strict backends** — OpenAI's Codex endpoint
|
||||
(``chatgpt.com/backend-api/codex``, gpt-5.x) returns
|
||||
``Invalid schema for function 'memory': schema must have type 'object'
|
||||
and not have 'oneOf'/'anyOf'/'allOf'/'enum'/'not' at the top level``.
|
||||
|
||||
We now rely on the runtime handler (``memory_tool()`` in ``tools/memory_tool.py``)
|
||||
to validate required fields per action and return actionable error messages.
|
||||
These tests guard the schema against regressing back to a shape strict
|
||||
backends reject.
|
||||
"""
|
||||
|
||||
import json
|
||||
|
||||
from tools.memory_tool import MEMORY_SCHEMA
|
||||
|
||||
|
||||
def test_memory_schema_requires_content_and_old_text_for_replace_action():
|
||||
schema = MEMORY_SCHEMA["parameters"]
|
||||
assert schema["required"] == ["action", "target"]
|
||||
|
||||
all_of = schema.get("allOf")
|
||||
assert all_of, "memory schema should use conditional requirements"
|
||||
|
||||
replace_requirements = [
|
||||
branch["then"].get("required", [])
|
||||
for branch in all_of
|
||||
if branch.get("if", {}).get("properties", {}).get("action", {}).get("const") == "replace"
|
||||
]
|
||||
assert replace_requirements == [["old_text", "content"]]
|
||||
_FORBIDDEN_TOP_LEVEL_KEYS = ("allOf", "anyOf", "oneOf", "enum", "not")
|
||||
|
||||
|
||||
def test_memory_schema_requires_content_for_add_action():
|
||||
add_requirements = [
|
||||
branch["then"].get("required", [])
|
||||
for branch in MEMORY_SCHEMA["parameters"].get("allOf", [])
|
||||
if branch.get("if", {}).get("properties", {}).get("action", {}).get("const") == "add"
|
||||
]
|
||||
assert add_requirements == [["content"]]
|
||||
def test_memory_schema_has_no_forbidden_top_level_combinators():
|
||||
"""OpenAI's Codex backend rejects these at the top level of parameters."""
|
||||
params = MEMORY_SCHEMA["parameters"]
|
||||
for key in _FORBIDDEN_TOP_LEVEL_KEYS:
|
||||
assert key not in params, (
|
||||
f"top-level {key!r} in memory tool parameters will break the "
|
||||
"Codex backend (chatgpt.com/backend-api/codex). Per-action "
|
||||
"required-field checks belong in the runtime handler, not the schema."
|
||||
)
|
||||
|
||||
|
||||
def test_memory_schema_requires_old_text_for_remove_action():
|
||||
remove_requirements = [
|
||||
branch["then"].get("required", [])
|
||||
for branch in MEMORY_SCHEMA["parameters"].get("allOf", [])
|
||||
if branch.get("if", {}).get("properties", {}).get("action", {}).get("const") == "remove"
|
||||
]
|
||||
assert remove_requirements == [["old_text"]]
|
||||
def test_memory_schema_is_well_formed():
|
||||
params = MEMORY_SCHEMA["parameters"]
|
||||
assert params["type"] == "object"
|
||||
assert params["required"] == ["action", "target"]
|
||||
# Nested ``enum`` on property values is fine — only top-level is forbidden.
|
||||
assert params["properties"]["action"]["enum"] == ["add", "replace", "remove"]
|
||||
assert params["properties"]["target"]["enum"] == ["memory", "user"]
|
||||
|
||||
|
||||
def test_memory_schema_is_json_serializable():
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue