fix(memory): recover from missing old_text on single-op replace/remove (#49997)

Single-op replace/remove failed with a dead-end 'old_text is required'
error when a structured-output client omitted the optional old_text field
(it can't be schema-required without a top-level if/then combinator that
OpenAI's Codex backend 400s on). The model couldn't recover.

Now a missing old_text returns the current entry inventory plus a retry
instruction (mirroring the batch path's _batch_error), so the model can
reissue the call with old_text set. Also sharpens the old_text schema
description to state it's required for replace/remove.

Fixes #49466, #43412.

tests/tools/test_memory_tool.py

@@ -435,12 +435,33 @@ class TestMemoryToolDispatcher:
         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

@@ -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",
@@ -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.
@@ -971,7 +1009,7 @@ MEMORY_SCHEMA = {
             },
             "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",