From 027cb649ef8018e6027edcead9423ad654888dd4 Mon Sep 17 00:00:00 2001 From: Hao Zhe Date: Mon, 22 Jun 2026 13:30:43 +0800 Subject: [PATCH] fix(memory): fail closed on unclear write results --- agent/memory_write_bridge.py | 11 +++------- plugins/memory/openviking/__init__.py | 9 ++++---- tests/agent/test_memory_write_bridge.py | 16 ++++++++++++++ .../memory/test_openviking_provider.py | 22 +++++++++++++++++++ 4 files changed, 45 insertions(+), 13 deletions(-) diff --git a/agent/memory_write_bridge.py b/agent/memory_write_bridge.py index eefe0e1b478..f09bfc6d42c 100644 --- a/agent/memory_write_bridge.py +++ b/agent/memory_write_bridge.py @@ -15,15 +15,10 @@ def _memory_tool_result_succeeded(result: Any) -> bool: 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 + if not isinstance(result, dict): + return False - return True + return result.get("success") is True and result.get("staged") is not True def collect_memory_write_notifications( diff --git a/plugins/memory/openviking/__init__.py b/plugins/memory/openviking/__init__.py index 030f6a59aa1..5c5de5d65f7 100644 --- a/plugins/memory/openviking/__init__.py +++ b/plugins/memory/openviking/__init__.py @@ -91,12 +91,11 @@ _MEMORY_WRITE_TARGET_SUBDIR_MAP = { "user": "preferences", "memory": "patterns", } -_DERIVED_MEMORY_FILENAMES = { +# OpenViking-generated markdown summaries. Non-.md sidecars such as +# .relations.json are rejected earlier by the exact memory-file check. +_GENERATED_MEMORY_SUMMARY_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 @@ -620,7 +619,7 @@ def _validate_forget_memory_uri(raw_uri: Any) -> tuple[Optional[str], Optional[s return None, "viking_forget only deletes user memory file URIs" filename = uri.rsplit("/", 1)[-1] - if filename in _DERIVED_MEMORY_FILENAMES: + if filename in _GENERATED_MEMORY_SUMMARY_FILENAMES: return None, "viking_forget cannot delete generated memory summary files" return uri, None diff --git a/tests/agent/test_memory_write_bridge.py b/tests/agent/test_memory_write_bridge.py index 053ad8c8aa0..b87da176d61 100644 --- a/tests/agent/test_memory_write_bridge.py +++ b/tests/agent/test_memory_write_bridge.py @@ -1,5 +1,7 @@ import json +import pytest + from agent.memory_write_bridge import collect_memory_write_notifications @@ -49,6 +51,20 @@ def test_collect_notifications_skips_staged_memory_write(): assert notifications == [] +@pytest.mark.parametrize("tool_result", [None, [], object()]) +def test_collect_notifications_skips_unrecognized_tool_result_shape(tool_result): + notifications = collect_memory_write_notifications( + tool_result, + { + "action": "add", + "target": "memory", + "content": "new fact", + }, + ) + + assert notifications == [] + + def test_collect_notifications_preserves_old_text_for_replace_and_remove_batch(): notifications = collect_memory_write_notifications( json.dumps({"success": True}), diff --git a/tests/plugins/memory/test_openviking_provider.py b/tests/plugins/memory/test_openviking_provider.py index f176492ca95..777afd2b43f 100644 --- a/tests/plugins/memory/test_openviking_provider.py +++ b/tests/plugins/memory/test_openviking_provider.py @@ -1511,6 +1511,28 @@ def test_handle_tool_call_forget_deletes_exact_memory_file_under_memories_root() } +def test_handle_tool_call_forget_allows_non_generated_dot_md_memory_file(): + uri = "viking://user/default/memories/preferences/.full.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",