# NousResearch/hermes-agent · Memory tool parameter naming and schema validation bugs

## Open now

- 3 issues, 2 PRs open on GitHub.
- #20735: Inconsistent parameter naming: \`old_text\` vs \`old_string\` across tools
- #55095: \[Bug\] Holographic memory plugin: on_memory_write only mirrors "add", silently drops "replace" and "remove"
- #59772: \[API Consistency\] memory, patch, and skill_manage use different field names for the same semantic operation
- PR #32425: fix(tools): require send_message delivery fields in schema
- PR #55098: fix(holographic): handle replace/remove in on_memory_write mirror

Most members concern mismatches between parameter names (old_text vs old_string), missing schema constraints for replace/remove actions, and broken memory tool operations due to unrecognized parameters. A smaller cluster addresses send_message schema fields not matching runtime enforcement.

## TL;DR

- 29 member(s): 3 open issue(s), 7 closed issue(s), 2 open PR(s), 2 merged PR(s), 15 PR(s) closed without merging.
- A fix was merged to main in PR #21238. Verify it addresses your case.
- **Recommended:** read PR #21238 (and any competing PRs below) before filing. Confirm the merged change covers your symptom.

## Maintainer resolution verdict

- Internal-tracker verdict over this cluster's issue members: `partially_resolved` · deciding PR(s): `pr-21238,pr-49997`. This is distinct from the fix status above. Weigh it yourself.

## How this cluster was grouped

- Link evidence (`STRUCTURAL_PARTIAL`): some members are connected by structural evidence (shared changed files, or a closing/superseded reference); the rest are joined by an automated relevance judgement over their text.
- Members outside that structural core may belong to a different underlying problem.
- Pairwise link density `0.07`: the share of member pairs that carry a link, counting a structural link in full and a relevance-only link at 0.36. `1.00` means every pair is linked.

## Open related issues

- #20735: Inconsistent parameter naming: \`old_text\` vs \`old_string\` across tools
- #55095: \[Bug\] Holographic memory plugin: on_memory_write only mirrors "add", silently drops "replace" and "remove"
- #59772: \[API Consistency\] memory, patch, and skill_manage use different field names for the same semantic operation

## Closed / duplicate issues

- #15843: \[Bug\]: Memory Tool Cannot Delete Old Data - memory tool remove/replace operations failing
- #19171: memory tool: replace action schema missing old_text/content as required
- #21844: Bug: memory tool replace/remove operations fail — old_text parameter not recognized
- #23381: memory tool replace/remove fails with "old_text is required" even when old_text is provided
- #32376: \[Bug\]: send_message schema declares \`required: \[\]\` but handler enforces \`target\`/\`message\` at runtime
- #43412: \[bug\] memory tool remove action rejects valid old_text arg with "old_text is required" error
- #49466: bug: memory remove and replace actions broken in v0.17.0

## Prior & competing PRs

- **#15859: fix(memory): accept common LLM aliases for action / new_text**. closed without merge · fix tier `suggested`
  - maintainer direction: category `uncategorized`; implication `needs_llm`; refs `#15843`
- **#15957: fix(tools): align memory tool handler parameter name with schema (old_text → old_string)**. closed without merge · fix tier `suggested`
  - maintainer direction: category `uncategorized`; implication `needs_llm`
- **#21238: fix(memory): require schema fields by action (salvage #19180)**. merged to main · fix tier `verified`
  - maintainer direction: category `merged`; implication `superseded`
  - files: `tests/tools/test_memory_tool_schema.py`, `tools/memory_tool.py`
  - fix hunks:
  - `tests/tools/test_memory_tool_schema.py`

    ```diff
    @@ -0,0 +1,39 @@
    +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"]]
    +
    +
    +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_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_json_serializable():
    +    json.dumps(MEMORY_SCHEMA)
    ```
  - `tools/memory_tool.py`

    ```diff
    @@ -560,6 +560,29 @@ def check_memory_requirements() -> bool:
                 },
             },
             "required": ["action", "target"],
    +        "allOf": [
    +            {
    +                "if": {
    +                    "properties": {"action": {"const": "add"}},
    +                    "required": ["action"],
    +                },
    +                "then": {"required": ["content"]},
    +            },
    +            {
    +                "if": {
    +                    "properties": {"action": {"const": "replace"}},
    +                    "required": ["action"],
    +                },
    +                "then": {"required": ["old_text", "content"]},
    +            },
    +            {
    +                "if": {
    +                    "properties": {"action": {"const": "remove"}},
    +                    "required": ["action"],
    +                },
    +                "then": {"required": ["old_text"]},
    +            },
    +        ],
         },
     }
     
    ```
- **#21450: fix(memory): accept old_string as old_text alias**. closed without merge · fix tier `suggested`
  - maintainer direction: category `uncategorized`; implication `needs_llm`
  - _suggested-tier fix detail withheld; re-request with `?tier=all` to include it._
- **#21875: fix(memory): normalize replace/remove alias forwarding**. closed without merge · fix tier `suggested`
  - maintainer direction: category `uncategorized`; implication `needs_llm`; refs `#21844`
- **#23399: fix(memory): add if/then schema constraint requiring old_text for rep…**. closed without merge · fix tier `suggested`
  - maintainer direction: category `implemented_elsewhere`; implication `superseded`; refs `c6bf6bda90a2bba718f94dc8fc69dcaf7828819e`, `5a3e5b23d251829629736641284bce2d5be7132a`
- **#32381: fix(tools): validate send_message send args in schema (#32376)**. closed without merge · fix tier `suggested`
  - maintainer direction: category `implemented_elsewhere`; implication `superseded`; refs `c6c8abbadb802dc389d8488a8847f9aa93bf9350`
  - _suggested-tier fix detail withheld; re-request with `?tier=all` to include it._
- **#32385: fix(send_message): declare required params in schema so model-side validator catches malformed calls**. closed without merge · fix tier `suggested`
  - maintainer direction: category `uncategorized`; implication `needs_llm`; refs `#32381`, `#32376`, `#21311`, `#21316`
  - _suggested-tier fix detail withheld; re-request with `?tier=all` to include it._
- **#32577: fix(memory): accept old_string as alias for old_text in memory tool**. closed without merge · fix tier `suggested`
  - maintainer direction: category `uncategorized`; implication `needs_llm`; refs `4cd1790db965`
- **#43526: fix(memory): add old_text to schema required list (#43412)**. closed without merge · fix tier `suggested`
  - maintainer direction: category `superseded_by_pr`; implication `superseded`; refs `#43426`, `#43412`
  - _suggested-tier fix detail withheld; re-request with `?tier=all` to include it._
- **#49997: fix(memory): recover from missing old_text on single-op replace/remove**. merged to main · fix tier `verified`
  - maintainer direction: category `merged`; implication `superseded`
  - files: `tests/tools/test_memory_tool.py`, `tools/memory_tool.py`
  - fix hunks:
  - `tests/tools/test_memory_tool.py`

    ```diff
    @@ -435,12 +435,33 @@ def test_add_via_tool(self, store):
             assert result["success"] is True
     
         def test_replace_requires_old_text(self, store):
    +        # Missing old_text on a single-op replace is recoverable, not a dead-end:
    +        # return the current inventory + a retry instruction so the model can
    +        # reissue with old_text set. (issues #43412, #49466)
    +        store.add("memory", "fact A")
    +        store.add("memory", "fact B")
             result = json.loads(memory_tool(action="replace", content="new", store=store))
             assert result["success"] is False
    +        assert "old_text" in result["error"]
    +        assert result["current_entries"] == ["fact A", "fact B"]
    +        assert "usage" in result
     
         def test_remove_requires_old_text(self, store):
    +        store.add("memory", "fact A")
             result = json.loads(memory_tool(action="remove", store=store))
             assert result["success"] is False
    +        assert "old_text" in result["error"]
    +        assert result["current_entries"] == ["fact A"]
    +        assert "usage" in result
    +
    +    def test_replace_missing_content_still_distinct_error(self, store):
    +        # When old_text IS present but content is missing, keep the original
    +        # content-specific error (don't route through the old_text recovery path).
    +        store.add("memory", "fact A")
    +        result = json.loads(memory_tool(action="replace", old_text="fact A", store=store))
    +        assert result["success"] is False
    +        assert "content is required" in result["error"]
    +        assert "current_entries" not in result
     
     
     class TestMemoryBatch:
    ```
  - `tools/memory_tool.py`

    ```diff
    @@ -835,6 +835,38 @@ def _apply_batch_write_gate(target: str, operations: List[Dict[str, Any]]) -> Op
         )
     
     
    +def _missing_old_text_error(store: "MemoryStore", target: str, action: str) -> str:
    +    """Build a recoverable error for a replace/remove call that arrived without
    +    ``old_text``.
    +
    +    ``replace``/``remove`` are inherently targeted -- without ``old_text`` there
    +    is no entry to act on, so we cannot fulfil the call. But returning a bare
    +    "old_text is required" is a dead-end: some structured-output clients omit the
    +    optional ``old_text`` field (it isn't, and can't be, schema-required without
    +    a top-level combinator the Codex backend rejects -- see
    +    tests/tools/test_memory_tool_schema.py). So instead we return the current
    +    entry inventory plus an explicit retry instruction, letting the model reissue
    +    the call with ``old_text`` set to a unique substring of the entry it means.
    +    Mirrors the batch path's ``_batch_error`` shape. (issues #43412, #49466)
    +    """
    +    entries = store._entries_for(target)
    +    current = store._char_count(target)
    +    limit = store._char_limit(target)
    +    return json.dumps(
    +        {
    +            "success": False,
    +            "error": (
    +                f"'{action}' needs old_text -- a short unique substring of the entry "
    +                f"to {action}. None was provided. Reissue the {action} with old_text "
    +                f"set to part of one of the current_entries below."
    +            ),
    +            "current_entries": entries,
    +            "usage": f"{current:,}/{limit:,}",
    +        },
    +        ensure_ascii=False,
    +    )
    +
    +
     def memory_tool(
         action: str = None,
         target: str = "memory",
    ```

    ```diff
    @@ -876,9 +908,15 @@ def memory_tool(
             return tool_error("Content is required for 'add' action.", success=False)
         if action == "replace" and (not old_text or not content):
             missing = "old_text" if not old_text else "content"
    +        if not old_text:
    +            # The client/model omitted old_text. Replace is inherently targeted
    +            # -- we can't guess which entry. Return the current inventory plus a
    +            # retry instruction so the model can reissue with old_text set,
    +            # instead of hitting a dead-end error. (issues #43412, #49466)
    +            return _missing_old_text_error(store, target, "replace")
             return tool_error(f"{missing} is required for 'replace' action.", success=False)
         if action == "remove" and not old_text:
    -        return tool_error("old_text is required for 'remove' action.", success=False)
    +        return _missing_old_text_error(store, target, "remove")
     
         # Approval gate: when on, stages the write (background/gateway) or prompts
         # inline (interactive CLI); when off (default) passes straight through.
    ```

    ```diff
    @@ -971,7 +1009,7 @@ def apply_memory_pending(payload: Dict[str, Any], store: "MemoryStore") -> Dict[
                 },
                 "old_text": {
                     "type": "string",
    -                "description": "Short unique substring identifying the entry to replace or remove (single-op shape)."
    +                "description": "REQUIRED for 'replace' and 'remove' (single-op shape): a short unique substring identifying the existing entry to modify. Omit only for 'add'."
                 },
                 "operations": {
                     "type": "array",
    ```
- **#19180: fix: require memory schema fields by action**. closed without merge
  - maintainer direction: category `duplicate_of`; implication `duplicate`; refs `#12604`, `#15957`
- **#21896: fix(tools): resolve old_string/new_text parameter aliases in memory tool**. closed without merge
  - maintainer direction: category `wontfix_bydesign`; implication `wontfix`
- **#32425: fix(tools): require send_message delivery fields in schema**. open
  - maintainer direction: category `uncategorized`; implication `needs_llm`; refs `90aaa84942dabc96f6648abb7b20404ffe01b6d5`
  - files: `tests/tools/test_send_message_tool_schema.py`, `tools/send_message_tool.py`
- **#32435: fix(tools): send_message schema required fields match runtime enforcement**. closed without merge
  - maintainer direction: category `superseded_by_pr`; implication `superseded`; refs `#32376`
- **#43426: fix(memory): preserve old_text in memory tool schema (#43412)**. closed without merge
  - maintainer direction: category `implemented_elsewhere`; implication `superseded`; refs `c6bf6bda90a2bba718f94dc8fc69dcaf7828819e`, `#49997`, `#43412`, `#43412`
- **#43484: fix(tools): add old_text to memory tool required schema fields (#43412)**. closed without merge
  - maintainer direction: category `duplicate_of`; implication `duplicate`; refs `#43426`, `#43412`, `#43426`
- **#55098: fix(holographic): handle replace/remove in on_memory_write mirror**. open
  - maintainer direction: category `implemented_elsewhere`; implication `superseded`; refs `#56274`, `#55095`, `#56274`
  - files: `plugins/memory/holographic/__init__.py`, `tests/plugins/memory/test_holographic_memory_write.py`
- **#56274: fix(memory): handle replace and remove in holographic on_memory_write**. closed without merge
  - maintainer direction: category `duplicate_of`; implication `duplicate`; refs `#55098`

## Fix status

- Merged to main in PR #21238 (cited above). No release data is tracked. Verify downstream.
- Fix last verified (merged) `2026-06-21`.
- Subsystem: `agent`