From 6446e19cb57249f7dd7ff6202075d91b793090ea Mon Sep 17 00:00:00 2001 From: ehz0ah Date: Tue, 1 Sep 2026 14:34:21 +0800 Subject: [PATCH] fix(openviking): route remember through session extraction --- plugins/memory/openviking/README.md | 25 ++-- plugins/memory/openviking/__init__.py | 62 +++++---- tests/openviking_plugin/test_openviking.py | 125 +++++++++++++++++- .../memory/test_openviking_optional_peer.py | 16 ++- 4 files changed, 180 insertions(+), 48 deletions(-) diff --git a/plugins/memory/openviking/README.md b/plugins/memory/openviking/README.md index 7bea6ad3eb..ebbaf2e741 100644 --- a/plugins/memory/openviking/README.md +++ b/plugins/memory/openviking/README.md @@ -120,23 +120,24 @@ changes future writes, not the location of existing memories. | `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 directly with OpenViking `content/write` | +| `viking_remember` | Submit a fact through OpenViking session memory extraction | | `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`. By default it creates files under explicit-uid -`viking://user//memories/...` URIs. When a peer ID is configured, it keeps -the existing `viking://user//peers//memories/...` path. In both cases, -`` is resolved client-side from `/api/v1/system/status` (server-asserted -current user). Hermes caches a confirmed user only for the active connection. -If the probe fails, Hermes uses the configured user, or `default`, for that -operation and retries the probe later. Explicit-uid URIs are canonical and -work under every OpenViking auth mode and version; the `viking://~` alias only -expands for USER/ADMIN roles, not the default dev mode. -Explicit remembers do not depend on session commit extraction. +`viking_remember` creates a one-shot `hermes-remember-` OpenViking +session, adds the fact as one message, and commits the session with no retained +tail. The session remains available in OpenViking for audit. OpenViking then +categorizes, merges, deduplicates, and indexes the result through its normal +memory extraction pipeline. The tool returns the one-shot session ID and the +extraction task ID when the server provides one. Extraction continues +asynchronously after the tool returns. + +The optional category is an extraction hint. The fact is stored as a `user` +message so `viking_remember` produces user memory. The one-shot session is +separate from the live Hermes conversation, so an explicit remember does not +commit or rotate the active conversation session. Hermes built-in `memory` tool additions are mirrored to OpenViking after the local memory operation succeeds: diff --git a/plugins/memory/openviking/__init__.py b/plugins/memory/openviking/__init__.py index 8280977cfe..5996367077 100644 --- a/plugins/memory/openviking/__init__.py +++ b/plugins/memory/openviking/__init__.py @@ -138,15 +138,6 @@ _SESSION_START_LIST_PARAMS = { "node_limit": 512, } -# Maps the viking_remember `category` enum to a viking:// subdirectory. -# Keep in sync with REMEMBER_SCHEMA.parameters.properties.category.enum. -_CATEGORY_SUBDIR_MAP = { - "preference": "preferences", - "entity": "entities", - "event": "events", - "case": "cases", - "pattern": "patterns", -} _DEFAULT_MEMORY_SUBDIR = "preferences" # Maps the built-in memory tool's `target` ("user" vs "memory") to a subdir @@ -623,9 +614,9 @@ BROWSE_SCHEMA = { REMEMBER_SCHEMA = { "name": "viking_remember", "description": ( - "Explicitly store a fact or memory in the OpenViking knowledge base. " + "Explicitly submit a fact or memory to the OpenViking memory pipeline. " "Use for important information the agent should remember long-term. " - "The system automatically categorizes and indexes the memory." + "OpenViking automatically categorizes, merges, and indexes the memory." ), "parameters": { "type": "object", @@ -634,7 +625,9 @@ REMEMBER_SCHEMA = { "category": { "type": "string", "enum": ["preference", "entity", "event", "case", "pattern"], - "description": "Memory category (default: auto-detected).", + "description": ( + "Optional extraction hint. OpenViking makes the final classification." + ), }, }, "required": ["content"], @@ -5267,30 +5260,41 @@ class OpenVikingMemoryProvider(MemoryProvider): if not content: return tool_error("content is required") - category = args.get("category", "") - subdir = _CATEGORY_SUBDIR_MAP.get(category, _DEFAULT_MEMORY_SUBDIR) client = self._ensure_client() if not client: return tool_error("OpenViking server not connected") - uri = self._build_memory_uri(subdir, client=client) - # Write directly via content/write API. - # This creates the file, stores the content, and queues vector indexing - # in a single call — no dependency on session commit / VLM extraction. + category = str(args.get("category") or "").strip() + message_content = f"[Remember — {category}] {content}" if category else content + session_id = f"hermes-remember-{uuid.uuid4().hex[:12]}" + message: Dict[str, Any] = { + "role": "user", + "parts": [self._text_part(message_content)], + } + + # Use a dedicated session so explicit remember does not commit or + # otherwise alter the live Hermes conversation session. try: - result = client.post("/api/v1/content/write", { - "uri": uri, - "content": content, - "mode": "create", - }) - written = result.get("result", {}).get("written_bytes", 0) - return json.dumps({ + client.post(f"/api/v1/sessions/{session_id}/messages", message) + commit = self._unwrap_result(client.post( + f"/api/v1/sessions/{session_id}/commit", + {"keep_recent_count": 0}, + )) + commit = commit if isinstance(commit, dict) else {} + result: Dict[str, Any] = { "status": "stored", - "message": f"Memory stored ({written}b) and queued for vector indexing.", - }) + "session_id": session_id, + "extraction_status": str(commit.get("status") or "accepted"), + "message": "Memory stored in an OpenViking session and committed for extraction.", + } + if commit.get("task_id"): + result["task_id"] = commit["task_id"] + if commit.get("trace_id"): + result["trace_id"] = commit["trace_id"] + return json.dumps(result) except Exception as e: - logger.error("OpenViking content/write failed: %s", e) - return tool_error(f"Failed to store memory: {e}") + logger.error("OpenViking remember session failed for %s: %s", session_id, e) + return tool_error(f"Failed to store memory in session {session_id}: {e}") def _tool_forget(self, args: dict) -> str: uri, error = _validate_forget_memory_uri(args.get("uri")) diff --git a/tests/openviking_plugin/test_openviking.py b/tests/openviking_plugin/test_openviking.py index d795777768..69dcd36670 100644 --- a/tests/openviking_plugin/test_openviking.py +++ b/tests/openviking_plugin/test_openviking.py @@ -946,7 +946,15 @@ class TestEnsureClientReloadsEnv: def post(self, path, payload=None, **kwargs): self.posts.append((path, payload or {})) - return {"result": {"written_bytes": 11}} + if path.endswith("/commit"): + return { + "result": { + "status": "accepted", + "task_id": "task-remember", + "trace_id": "trace-remember", + } + } + return {"status": "ok"} monkeypatch.setattr("plugins.memory.openviking._VikingClient", _StubClient) monkeypatch.setenv("OPENVIKING_ENDPOINT", "https://openviking.example") @@ -964,14 +972,119 @@ class TestEnsureClientReloadsEnv: )) assert out["status"] == "stored" + assert out["session_id"].startswith("hermes-remember-") + assert out["extraction_status"] == "accepted" + assert out["task_id"] == "task-remember" + assert out["trace_id"] == "trace-remember" assert len(instances) == 2 - assert instances[1].posts[0][0] == "/api/v1/content/write" - assert instances[1].posts[0][1]["content"] == "stable fact" - assert instances[1].posts[0][1]["mode"] == "create" - assert instances[1].posts[0][1]["uri"].startswith( - "viking://user/default/peers/hermes/memories/" + session_id = out["session_id"] + assert instances[1].posts == [ + ( + f"/api/v1/sessions/{session_id}/messages", + { + "role": "user", + "parts": [{"type": "text", "text": "stable fact"}], + }, + ), + ( + f"/api/v1/sessions/{session_id}/commit", + {"keep_recent_count": 0}, + ), + ] + + @pytest.mark.parametrize( + "category", + [ + "preference", + "entity", + "event", + "case", + "pattern", + ], + ) + def test_remember_uses_category_as_user_memory_hint( + self, + monkeypatch, + category, + ): + posts = [] + + class _StubClient: + def post(self, path, payload=None, **kwargs): + posts.append((path, payload or {})) + if path.endswith("/commit"): + return {"result": {"status": "accepted", "task_id": "task-1"}} + return {"status": "ok"} + + provider = OpenVikingMemoryProvider() + provider._client = _StubClient() + provider._agent = "hermes" + monkeypatch.setattr(provider, "_ensure_client", lambda: provider._client) + + out = json.loads(provider._tool_remember({ + "content": "stable fact", + "category": category, + })) + + session_id = out["session_id"] + message_path, message = posts[0] + assert message_path == f"/api/v1/sessions/{session_id}/messages" + assert message["role"] == "user" + assert message["parts"] == [ + {"type": "text", "text": f"[Remember — {category}] stable fact"} + ] + assert "peer_id" not in message + assert posts[1] == ( + f"/api/v1/sessions/{session_id}/commit", + {"keep_recent_count": 0}, ) + def test_remember_uses_a_distinct_one_shot_session_for_each_call(self, monkeypatch): + posts = [] + + class _StubClient: + def post(self, path, payload=None, **kwargs): + posts.append((path, payload or {})) + if path.endswith("/commit"): + return {"result": {"status": "accepted"}} + return {"status": "ok"} + + provider = OpenVikingMemoryProvider() + provider._client = _StubClient() + monkeypatch.setattr(provider, "_ensure_client", lambda: provider._client) + + first = json.loads(provider._tool_remember({"content": "first"})) + second = json.loads(provider._tool_remember({"content": "second"})) + + assert first["session_id"] != second["session_id"] + assert first["session_id"].startswith("hermes-remember-") + assert second["session_id"].startswith("hermes-remember-") + assert all("/api/v1/content/write" not in path for path, _ in posts) + + def test_remember_reports_commit_failure_with_recoverable_session_id(self, monkeypatch): + posts = [] + + class _StubClient: + def post(self, path, payload=None, **kwargs): + posts.append((path, payload or {})) + if path.endswith("/commit"): + raise RuntimeError("commit rejected") + return {"status": "ok"} + + provider = OpenVikingMemoryProvider() + provider._client = _StubClient() + monkeypatch.setattr(provider, "_ensure_client", lambda: provider._client) + + out = json.loads(provider._tool_remember({"content": "stable fact"})) + + assert out["error"].startswith( + "Failed to store memory in session hermes-remember-" + ) + assert out["error"].endswith(": commit rejected") + assert len(posts) == 2 + assert posts[0][0].endswith("/messages") + assert posts[1][0].endswith("/commit") + def test_concurrent_refresh_does_not_return_stale_client(self, monkeypatch): refresh_entered = threading.Event() release_refresh = threading.Event() diff --git a/tests/plugins/memory/test_openviking_optional_peer.py b/tests/plugins/memory/test_openviking_optional_peer.py index 281d73077e..691151dd38 100644 --- a/tests/plugins/memory/test_openviking_optional_peer.py +++ b/tests/plugins/memory/test_openviking_optional_peer.py @@ -321,11 +321,25 @@ def test_wire_requests_keep_writes_and_session_messages_in_the_selected_scope( payload for path, _, payload in records if path == "/api/v1/content/write" ] prefix = f"peers/{peer}/" if peer else "" - assert {write["content"] for write in writes} == {"I like tea", "I like coffee"} + assert {write["content"] for write in writes} == {"I like coffee"} assert all( write["uri"].startswith(f"viking://user/alice/{prefix}memories/") for write in writes ) + remember_messages = [ + (path, payload) + for path, _, payload in records + if path.startswith("/api/v1/sessions/hermes-remember-") + and path.endswith("/messages") + ] + assert len(remember_messages) == 1 + remember_path, remember_message = remember_messages[0] + remember_session = remember_path.removesuffix("/messages") + assert remember_message == { + "role": "user", + "parts": [{"type": "text", "text": "I like tea"}], + } + assert any(path == f"{remember_session}/commit" for path, _, _ in records) batches = [ payload["messages"] for path, _, payload in records