diff --git a/agent/agent_runtime_helpers.py b/agent/agent_runtime_helpers.py index 92d521b16d8..7303b7e921a 100644 --- a/agent/agent_runtime_helpers.py +++ b/agent/agent_runtime_helpers.py @@ -32,6 +32,7 @@ from pathlib import Path from typing import Any, Dict, List, Optional from hermes_cli.timeouts import get_provider_request_timeout +from agent.memory_write_bridge import collect_memory_write_notifications from agent.prompt_builder import format_steer_marker from agent.tool_dispatch_helpers import _trajectory_normalize_msg, make_tool_result_message from agent.trajectory import convert_scratchpad_to_think @@ -1838,29 +1839,24 @@ def invoke_tool(agent, function_name: str, function_args: dict, effective_task_i operations=operations, store=agent._memory_store, ) - # Bridge: notify external memory provider of built-in memory writes. - # Covers both the single-op shape and each add/replace inside a batch. + # Bridge: notify external memory providers of successful built-in + # memory writes. Covers the single-op shape and each mutating op + # inside a successful batch. if agent._memory_manager: - if operations: - _mem_ops = [ - op for op in operations - if isinstance(op, dict) and op.get("action") in {"add", "replace"} - ] - else: - _mem_ops = ( - [{"action": next_args.get("action"), "content": next_args.get("content")}] - if next_args.get("action") in {"add", "replace"} else [] - ) + _mem_ops = collect_memory_write_notifications(result, next_args) for _op in _mem_ops: try: + metadata = agent._build_memory_write_metadata( + task_id=effective_task_id, + tool_call_id=tool_call_id, + ) + if _op.get("old_text"): + metadata["old_text"] = _op["old_text"] agent._memory_manager.on_memory_write( _op.get("action", ""), - target, + _op.get("target", target), _op.get("content", "") or "", - metadata=agent._build_memory_write_metadata( - task_id=effective_task_id, - tool_call_id=tool_call_id, - ), + metadata=metadata, ) except Exception: pass diff --git a/agent/memory_write_bridge.py b/agent/memory_write_bridge.py new file mode 100644 index 00000000000..eefe0e1b478 --- /dev/null +++ b/agent/memory_write_bridge.py @@ -0,0 +1,61 @@ +"""Helpers for mirroring built-in memory writes to external providers.""" + +from __future__ import annotations + +import json +from typing import Any, Dict, List + +_MIRRORED_MEMORY_ACTIONS = {"add", "replace", "remove"} + + +def _memory_tool_result_succeeded(result: Any) -> bool: + if isinstance(result, str): + try: + result = json.loads(result) + except Exception: + return False + + if isinstance(result, dict): + if result.get("success") is False: + return False + if result.get("staged") is True: + return False + if "error" in result and result.get("success") is not True: + return False + + return True + + +def collect_memory_write_notifications( + tool_result: Any, + tool_args: Dict[str, Any], +) -> List[Dict[str, str]]: + """Return provider notifications for a successful built-in memory write.""" + if not _memory_tool_result_succeeded(tool_result): + return [] + + target = str(tool_args.get("target") or "memory") + operations = tool_args.get("operations") + if isinstance(operations, list) and operations: + raw_operations = operations + else: + raw_operations = [{ + "action": tool_args.get("action"), + "content": tool_args.get("content"), + "old_text": tool_args.get("old_text"), + }] + + notifications: List[Dict[str, str]] = [] + for op in raw_operations: + if not isinstance(op, dict): + continue + action = str(op.get("action") or "") + if action not in _MIRRORED_MEMORY_ACTIONS: + continue + notifications.append({ + "action": action, + "target": target, + "content": str(op.get("content") or ""), + "old_text": str(op.get("old_text") or ""), + }) + return notifications diff --git a/agent/tool_executor.py b/agent/tool_executor.py index b79c29767e8..99706317786 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -29,6 +29,7 @@ from agent.display import ( _detect_tool_failure, ) from agent.tool_guardrails import ToolGuardrailDecision +from agent.memory_write_bridge import collect_memory_write_notifications from agent.tool_dispatch_helpers import ( _is_destructive_command, _is_multimodal_tool_result, @@ -1046,29 +1047,24 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe operations=operations, store=agent._memory_store, ) - # Bridge: notify external memory provider of built-in memory writes. - # Covers both the single-op shape and each add/replace inside a batch. + # Bridge: notify external memory providers of successful built-in + # memory writes. Covers the single-op shape and each mutating op + # inside a successful batch. if agent._memory_manager: - if operations: - _mem_ops = [ - op for op in operations - if isinstance(op, dict) and op.get("action") in {"add", "replace"} - ] - else: - _mem_ops = ( - [{"action": next_args.get("action"), "content": next_args.get("content")}] - if next_args.get("action") in {"add", "replace"} else [] - ) + _mem_ops = collect_memory_write_notifications(result, next_args) for _op in _mem_ops: try: + metadata = agent._build_memory_write_metadata( + task_id=effective_task_id, + tool_call_id=getattr(tool_call, "id", None), + ) + if _op.get("old_text"): + metadata["old_text"] = _op["old_text"] agent._memory_manager.on_memory_write( _op.get("action", ""), - target, + _op.get("target", target), _op.get("content", "") or "", - metadata=agent._build_memory_write_metadata( - task_id=effective_task_id, - tool_call_id=getattr(tool_call, "id", None), - ), + metadata=metadata, ) except Exception: pass diff --git a/plugins/memory/openviking/README.md b/plugins/memory/openviking/README.md index 17f658d350d..4c98e3d0a09 100644 --- a/plugins/memory/openviking/README.md +++ b/plugins/memory/openviking/README.md @@ -47,5 +47,37 @@ Hermes sends `OPENVIKING_ACCOUNT` and `OPENVIKING_USER` as identity headers. | `viking_search` | Semantic search with fast/deep/auto modes | | `viking_read` | Read content at a viking:// URI (abstract/overview/full) | | `viking_browse` | Filesystem-style navigation (list/tree/stat) | -| `viking_remember` | Store a fact for extraction on session commit | +| `viking_remember` | Store a fact directly with OpenViking `content/write` | +| `viking_forget` | Delete one exact `viking://` memory file URI | | `viking_add_resource` | Ingest URLs/docs into the knowledge base | + +## Memory Writes And Deletes + +`viking_remember` writes directly to OpenViking with `POST /api/v1/content/write` +and `mode=create`. It creates peer-scoped memory files under +`viking://user/peers/${OPENVIKING_AGENT}/memories/...`; OpenViking may return a +canonical user-scoped form such as +`viking://user/default/peers/${OPENVIKING_AGENT}/memories/...` in API-key mode. +Explicit remembers do not depend on session commit extraction. + +Hermes built-in `memory` tool additions are mirrored to OpenViking after the +local memory operation succeeds: + +| Hermes action | OpenViking operation | +|---------------|----------------------| +| `add` | `content/write` with `mode=create` under the configured peer memory namespace | + +Built-in `replace` and `remove` operations are not mirrored because Hermes +native memory entries do not yet carry stable OpenViking file URIs. Use +`viking_forget` when the user explicitly asks to delete a specific OpenViking +memory URI. + +`viking_forget` is intentionally narrow. It only accepts concrete user memory +file URIs, such as +`viking://user/peers/hermes/memories/preferences/mem_abc123.md` or the canonical +`viking://user/default/peers/hermes/memories/preferences/mem_abc123.md`. Files +directly under `memories/`, such as `viking://user/default/memories/profile.md`, +are also allowed because OpenViking supports them. The tool rejects directories, +resources, skills, sessions, generated summary files, and URIs with query +strings or fragments. Use OpenViking's MCP, CLI, or admin APIs for broader +resource and directory cleanup. diff --git a/plugins/memory/openviking/__init__.py b/plugins/memory/openviking/__init__.py index 2beaeb26c2a..c3b652c3d22 100644 --- a/plugins/memory/openviking/__init__.py +++ b/plugins/memory/openviking/__init__.py @@ -91,6 +91,13 @@ _MEMORY_WRITE_TARGET_SUBDIR_MAP = { "user": "preferences", "memory": "patterns", } +_DERIVED_MEMORY_FILENAMES = { + ".abstract.md", + ".overview.md", + ".read.md", + ".full.md", + ".relations.json", +} _LOCAL_OPENVIKING_HOSTS = {"localhost", "127.0.0.1", "::1"} _LOCAL_OPENVIKING_AUTOSTART_TIMEOUT = 60.0 _OPENVIKING_SERVER_LOG_RELATIVE_PATH = Path("logs") / "openviking-server.log" @@ -320,6 +327,13 @@ class _VikingClient: ) ) + def delete(self, path: str, **kwargs) -> dict: + return self._send_with_trusted_identity_retry( + lambda headers: self._httpx.delete( + self._url(path), headers=headers, timeout=_TIMEOUT, **kwargs + ) + ) + def upload_temp_file(self, file_path: Path) -> str: mime_type = mimetypes.guess_type(file_path.name)[0] or "application/octet-stream" @@ -460,6 +474,26 @@ REMEMBER_SCHEMA = { }, } +FORGET_SCHEMA = { + "name": "viking_forget", + "description": ( + "Delete one OpenViking memory file by exact viking:// URI. " + "Use only when the user explicitly asks to forget or delete a specific " + "memory and you have the exact memory file URI. Resources, skills, " + "sessions, directories, generated summaries, and broad deletes are rejected." + ), + "parameters": { + "type": "object", + "properties": { + "uri": { + "type": "string", + "description": "Exact viking:// memory file URI ending in .md.", + }, + }, + "required": ["uri"], + }, +} + ADD_RESOURCE_SCHEMA = { "name": "viking_add_resource", "description": ( @@ -552,6 +586,46 @@ def _is_remote_resource_source(value: str) -> bool: return value.startswith(_REMOTE_RESOURCE_PREFIXES) +def _memory_segment_index(parts: List[str]) -> Optional[int]: + if len(parts) >= 2 and parts[0] == "user" and parts[1] == "memories": + return 1 + if len(parts) >= 3 and parts[0] == "user" and parts[2] == "memories": + return 2 + if len(parts) >= 4 and parts[0] == "user" and parts[1] == "peers" and parts[3] == "memories": + return 3 + if len(parts) >= 5 and parts[0] == "user" and parts[2] == "peers" and parts[4] == "memories": + return 4 + return None + + +def _validate_forget_memory_uri(raw_uri: Any) -> tuple[Optional[str], Optional[str]]: + if not isinstance(raw_uri, str): + return None, "uri is required" + + uri = raw_uri.strip() + if not uri: + return None, "uri is required" + + parsed = urlparse(uri) + if parsed.scheme != "viking" or not uri.startswith("viking://"): + return None, "viking_forget only accepts viking:// memory file URIs" + if parsed.query or parsed.fragment: + return None, "viking_forget requires an exact URI without query or fragment" + if uri.endswith("/") or not uri.endswith(".md"): + return None, "viking_forget only deletes concrete .md memory files" + + parts = [part for part in uri[len("viking://") :].split("/") if part] + memories_idx = _memory_segment_index(parts) + if memories_idx is None or len(parts) < memories_idx + 2: + return None, "viking_forget only deletes user memory file URIs" + + filename = uri.rsplit("/", 1)[-1] + if filename in _DERIVED_MEMORY_FILENAMES: + return None, "viking_forget cannot delete generated memory summary files" + + return uri, None + + def _is_local_path_reference(value: str) -> bool: if not value or "\n" in value or "\r" in value: return False @@ -2047,7 +2121,8 @@ class OpenVikingMemoryProvider(MemoryProvider): f"Active. Endpoint: {self._endpoint}\n" "Use viking_search to find information, viking_read for details " "(abstract/overview/full), viking_browse to explore.\n" - "Use viking_remember to store facts, viking_add_resource to index URLs/docs." + "Use viking_remember to store facts, viking_forget to delete exact memory " + "file URIs, and viking_add_resource to index URLs/docs." ) except Exception as e: logger.warning("OpenViking system_prompt_block failed: %s", e) @@ -2055,7 +2130,7 @@ class OpenVikingMemoryProvider(MemoryProvider): "# OpenViking Knowledge Base\n" f"Active. Endpoint: {self._endpoint}\n" "Use viking_search, viking_read, viking_browse, " - "viking_remember, viking_add_resource." + "viking_remember, viking_forget, viking_add_resource." ) def prefetch(self, query: str, *, session_id: str = "") -> str: @@ -2806,7 +2881,7 @@ class OpenVikingMemoryProvider(MemoryProvider): content: str, metadata: Optional[Dict[str, Any]] = None, ) -> None: - """Mirror built-in memory writes to OpenViking via content/write.""" + """Mirror successful built-in memory additions to OpenViking.""" if not self._client or action != "add" or not content: return @@ -2831,7 +2906,14 @@ class OpenVikingMemoryProvider(MemoryProvider): t.start() def get_tool_schemas(self) -> List[Dict[str, Any]]: - return [SEARCH_SCHEMA, READ_SCHEMA, BROWSE_SCHEMA, REMEMBER_SCHEMA, ADD_RESOURCE_SCHEMA] + return [ + SEARCH_SCHEMA, + READ_SCHEMA, + BROWSE_SCHEMA, + REMEMBER_SCHEMA, + FORGET_SCHEMA, + ADD_RESOURCE_SCHEMA, + ] def handle_tool_call(self, tool_name: str, args: dict, **kwargs) -> str: if not self._client: @@ -2846,6 +2928,8 @@ class OpenVikingMemoryProvider(MemoryProvider): return self._tool_browse(args) elif tool_name == "viking_remember": return self._tool_remember(args) + elif tool_name == "viking_forget": + return self._tool_forget(args) elif tool_name == "viking_add_resource": return self._tool_add_resource(args) return tool_error(f"Unknown tool: {tool_name}") @@ -3097,6 +3181,31 @@ class OpenVikingMemoryProvider(MemoryProvider): logger.error("OpenViking content/write failed: %s", e) return tool_error(f"Failed to store memory: {e}") + def _tool_forget(self, args: dict) -> str: + uri, error = _validate_forget_memory_uri(args.get("uri")) + if error: + return tool_error(error) + + resp = self._client.delete( + "/api/v1/fs", + params={"uri": uri, "recursive": False}, + ) + result = self._unwrap_result(resp) + payload: Dict[str, Any] = {"status": "deleted", "uri": uri} + if isinstance(result, dict): + payload["uri"] = result.get("uri") or uri + for key in ( + "estimated_deleted_count", + "memory_cleanup", + "semantic_root_uri", + "semantic_status", + "queue_status", + ): + if key in result: + payload[key] = result[key] + + return json.dumps(payload, ensure_ascii=False) + def _tool_add_resource(self, args: dict) -> str: url = args.get("url", "") if not url: diff --git a/tests/agent/test_memory_provider.py b/tests/agent/test_memory_provider.py index 57f8f39fc7d..bacb8911600 100644 --- a/tests/agent/test_memory_provider.py +++ b/tests/agent/test_memory_provider.py @@ -1172,16 +1172,12 @@ class TestOnMemoryWriteBridge: mgr.on_memory_write("replace", "user", "updated pref") assert p.memory_writes == [("replace", "user", "updated pref")] - def test_on_memory_write_remove_not_bridged(self): - """The bridge intentionally skips 'remove' — only add/replace notify.""" - # This tests the contract that run_agent.py checks: - # function_args.get("action") in ("add", "replace") + def test_on_memory_write_remove_supported_by_manager(self): + """The manager forwards remove actions when a caller elects to bridge them.""" mgr = MemoryManager() p = FakeMemoryProvider("ext") mgr.add_provider(p) - # Manager itself doesn't filter — run_agent.py does. - # But providers should handle remove gracefully. mgr.on_memory_write("remove", "memory", "old fact") assert p.memory_writes == [("remove", "memory", "old fact")] diff --git a/tests/agent/test_memory_write_bridge.py b/tests/agent/test_memory_write_bridge.py new file mode 100644 index 00000000000..053ad8c8aa0 --- /dev/null +++ b/tests/agent/test_memory_write_bridge.py @@ -0,0 +1,84 @@ +import json + +from agent.memory_write_bridge import collect_memory_write_notifications + + +def test_collect_notifications_includes_remove_with_old_text_after_success(): + notifications = collect_memory_write_notifications( + json.dumps({"success": True}), + { + "action": "remove", + "target": "memory", + "old_text": "stale preference entry", + }, + ) + + assert notifications == [ + { + "action": "remove", + "target": "memory", + "content": "", + "old_text": "stale preference entry", + } + ] + + +def test_collect_notifications_skips_failed_memory_write(): + notifications = collect_memory_write_notifications( + json.dumps({"success": False, "error": "No entry matched"}), + { + "action": "remove", + "target": "memory", + "old_text": "stale preference entry", + }, + ) + + assert notifications == [] + + +def test_collect_notifications_skips_staged_memory_write(): + notifications = collect_memory_write_notifications( + json.dumps({"success": True, "staged": True, "pending_id": "abc123"}), + { + "action": "remove", + "target": "memory", + "old_text": "stale preference entry", + }, + ) + + assert notifications == [] + + +def test_collect_notifications_preserves_old_text_for_replace_and_remove_batch(): + notifications = collect_memory_write_notifications( + json.dumps({"success": True}), + { + "target": "user", + "operations": [ + {"action": "replace", "old_text": "old preference", "content": "updated"}, + {"action": "remove", "old_text": "obsolete preference"}, + {"action": "add", "content": "new fact"}, + ], + }, + ) + + assert notifications == [ + { + "action": "replace", + "target": "user", + "content": "updated", + "old_text": "old preference", + }, + { + "action": "remove", + "target": "user", + "content": "", + "old_text": "obsolete preference", + }, + { + "action": "add", + "target": "user", + "content": "new fact", + "old_text": "", + }, + ] diff --git a/tests/plugins/memory/test_openviking_provider.py b/tests/plugins/memory/test_openviking_provider.py index 28f2d8e9d46..d5b5f347994 100644 --- a/tests/plugins/memory/test_openviking_provider.py +++ b/tests/plugins/memory/test_openviking_provider.py @@ -1459,6 +1459,115 @@ def test_tool_add_resource_sends_git_remote_sources_as_path(url): }) +def test_get_tool_schemas_includes_narrow_forget_tool(): + provider = OpenVikingMemoryProvider() + + names = [schema["name"] for schema in provider.get_tool_schemas()] + + assert "viking_forget" in names + + +def test_handle_tool_call_forget_deletes_exact_memory_file_uri(): + uri = "viking://user/peers/hermes/memories/preferences/mem_abc123.md" + provider = OpenVikingMemoryProvider() + provider._client = MagicMock() + provider._client.delete.return_value = { + "status": "ok", + "result": {"uri": uri, "estimated_deleted_count": 1}, + } + + result = json.loads(provider.handle_tool_call("viking_forget", {"uri": uri})) + + provider._client.delete.assert_called_once_with( + "/api/v1/fs", + params={"uri": uri, "recursive": False}, + ) + assert result == { + "status": "deleted", + "uri": uri, + "estimated_deleted_count": 1, + } + + +def test_handle_tool_call_forget_deletes_exact_memory_file_under_memories_root(): + uri = "viking://user/default/memories/profile.md" + provider = OpenVikingMemoryProvider() + provider._client = MagicMock() + provider._client.delete.return_value = { + "status": "ok", + "result": {"uri": uri, "estimated_deleted_count": 1}, + } + + result = json.loads(provider.handle_tool_call("viking_forget", {"uri": uri})) + + provider._client.delete.assert_called_once_with( + "/api/v1/fs", + params={"uri": uri, "recursive": False}, + ) + assert result == { + "status": "deleted", + "uri": uri, + "estimated_deleted_count": 1, + } + + +@pytest.mark.parametrize("uri", [ + "", + "https://example.com/mem.md", + "viking:/user/memories/preferences/mem_abc123.md", + "viking://resources/project/doc.md", + "viking://resources/project/memories/mem_abc123.md", + "viking://memories/preferences/mem_abc123.md", + "viking://agent/hermes/memories/preferences/mem_abc123.md", + "viking://user/skills/example/SKILL.md", + "viking://user/sessions/session-1/messages.jsonl", + "viking://user/memories/preferences/", + "viking://user/memories/preferences/.overview.md", + "viking://user/memories/preferences/.abstract.md", + "viking://user/memories/preferences/mem_abc123.md?recursive=true", +]) +def test_handle_tool_call_forget_rejects_non_memory_file_uris(uri): + provider = OpenVikingMemoryProvider() + provider._client = MagicMock() + + result = json.loads(provider.handle_tool_call("viking_forget", {"uri": uri})) + + assert "error" in result + provider._client.delete.assert_not_called() + + +def test_viking_client_delete_uses_identity_headers(monkeypatch): + client = _VikingClient( + "https://example.com", + api_key="test-key", + account="acct", + user="alice", + agent="hermes", + ) + captured = {} + + def capture_delete(url, **kwargs): + captured["url"] = url + captured["kwargs"] = kwargs + return SimpleNamespace( + status_code=200, + text="", + json=lambda: {"status": "ok", "result": {"uri": "viking://user/memories/x.md"}}, + raise_for_status=lambda: None, + ) + + monkeypatch.setattr(client._httpx, "delete", capture_delete) + + assert client.delete("/api/v1/fs", params={"uri": "viking://user/memories/x.md"}) == { + "status": "ok", + "result": {"uri": "viking://user/memories/x.md"}, + } + assert captured["url"] == "https://example.com/api/v1/fs" + assert captured["kwargs"]["params"] == {"uri": "viking://user/memories/x.md"} + assert captured["kwargs"]["headers"]["Authorization"] == "Bearer test-key" + assert captured["kwargs"]["headers"]["X-OpenViking-Actor-Peer"] == "hermes" + + def test_viking_client_upload_temp_file_uses_multipart_identity_headers(tmp_path, monkeypatch): sample = tmp_path / "sample.md" sample.write_text("# Local resource\n", encoding="utf-8") @@ -2637,6 +2746,46 @@ def test_on_memory_write_uses_content_write_independent_of_session_rotation(): ) +@pytest.mark.parametrize( + ("action", "content"), + [ + ("replace", "updated memory"), + ("remove", ""), + ("forget", ""), + ("delete", ""), + ], +) +def test_on_memory_write_ignores_non_add_actions(action, content, monkeypatch): + provider = OpenVikingMemoryProvider() + provider._client = MagicMock() + provider._endpoint = "http://test" + provider._api_key = "" + provider._account = "acct" + provider._user = "usr" + provider._agent = "hermes" + uri = "viking://user/peers/hermes/memories/preferences/mem_abc123.md" + spawned = [] + + class StubThread: + def __init__(self, *args, **kwargs): + spawned.append((args, kwargs)) + + def start(self): + raise AssertionError("non-URI remove should not spawn a mirror thread") + + import plugins.memory.openviking as _mod + monkeypatch.setattr(_mod.threading, "Thread", StubThread) + + provider.on_memory_write( + action, + "memory", + content, + metadata={"uri": uri, "old_text": "stale fact"}, + ) + + assert spawned == [] + + # --------------------------------------------------------------------------- # Prefetch staleness: a prefetch worker that finishes AFTER a session switch # must drop its result instead of repopulating the new session with stale diff --git a/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index 2b45654aac2..ca798e2340c 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -2082,6 +2082,41 @@ class TestExecuteToolCalls: assert messages[0]["role"] == "tool" assert "search result" in messages[0]["content"] + def test_sequential_memory_remove_notifies_provider_with_tool_result(self, agent): + old_text = "stale preference entry" + tc = _mock_tool_call( + name="memory", + arguments=json.dumps({ + "action": "remove", + "target": "memory", + "old_text": old_text, + }), + call_id="mem-1", + ) + mock_msg = _mock_assistant_msg(content="", tool_calls=[tc]) + messages = [] + calls = [] + + class FakeMemoryManager: + def has_tool(self, name): + return False + + def on_memory_write(self, action, target, content, metadata=None): + calls.append((action, target, content, metadata or {})) + + agent._memory_manager = FakeMemoryManager() + agent._memory_store = object() + + with patch("tools.memory_tool.memory_tool", return_value=json.dumps({"success": True})): + agent._execute_tool_calls_sequential(mock_msg, messages, "task-1") + + assert len(calls) == 1 + action, target, content, metadata = calls[0] + assert (action, target, content) == ("remove", "memory", "") + assert metadata["old_text"] == old_text + assert metadata["tool_call_id"] == "mem-1" + assert messages[-1]["tool_call_id"] == "mem-1" + def test_keyboard_interrupt_emits_cancelled_post_tool_hook(self, agent, monkeypatch): tc = _mock_tool_call(name="web_search", arguments='{"q":"test"}', call_id="c1") mock_msg = _mock_assistant_msg(content="", tool_calls=[tc]) @@ -2797,6 +2832,63 @@ class TestConcurrentToolExecution: assert json.loads(result) == {"error": "Blocked"} assert agent._turns_since_memory == 5 + def test_invoke_tool_memory_remove_notifies_provider_with_old_text(self, agent, monkeypatch): + monkeypatch.setattr( + "hermes_cli.plugins.get_pre_tool_call_block_message", + lambda *args, **kwargs: None, + ) + calls = [] + + class FakeMemoryManager: + def has_tool(self, name): + return False + + def on_memory_write(self, action, target, content, metadata=None): + calls.append((action, target, content, metadata or {})) + + old_text = "stale preference entry" + agent._memory_manager = FakeMemoryManager() + agent._memory_store = object() + + with patch("tools.memory_tool.memory_tool", return_value=json.dumps({"success": True})): + agent._invoke_tool( + "memory", + {"action": "remove", "target": "memory", "old_text": old_text}, + "task-1", + tool_call_id="mem-1", + ) + + assert len(calls) == 1 + action, target, content, metadata = calls[0] + assert (action, target, content) == ("remove", "memory", "") + assert metadata["old_text"] == old_text + assert metadata["tool_call_id"] == "mem-1" + + def test_invoke_tool_memory_failed_remove_skips_provider_notification(self, agent, monkeypatch): + monkeypatch.setattr( + "hermes_cli.plugins.get_pre_tool_call_block_message", + lambda *args, **kwargs: None, + ) + manager = SimpleNamespace( + has_tool=lambda name: False, + on_memory_write=MagicMock(side_effect=AssertionError("should not notify")), + ) + agent._memory_manager = manager + agent._memory_store = object() + + with patch( + "tools.memory_tool.memory_tool", + return_value=json.dumps({"success": False, "error": "No entry matched"}), + ): + agent._invoke_tool( + "memory", + {"action": "remove", "target": "memory", "old_text": "missing"}, + "task-1", + tool_call_id="mem-1", + ) + + manager.on_memory_write.assert_not_called() + def test_concurrent_blocked_write_skips_checkpoint(self, agent, monkeypatch): """Concurrent path: blocked write_file should not trigger checkpoint.""" tc1 = _mock_tool_call(name="write_file",