From cbb87389f33583518975fbf72671de3fd224bb28 Mon Sep 17 00:00:00 2001 From: qin-ctx Date: Mon, 15 Jun 2026 15:57:23 +0800 Subject: [PATCH] fix(openviking): adapt memory provider for current api --- plugins/memory/openviking/README.md | 9 +- plugins/memory/openviking/__init__.py | 120 +++++++++++------- tests/openviking_plugin/test_openviking.py | 18 +-- .../memory/test_openviking_provider.py | 107 ++++++++++++++-- .../user-guide/features/memory-providers.md | 5 + 5 files changed, 191 insertions(+), 68 deletions(-) diff --git a/plugins/memory/openviking/README.md b/plugins/memory/openviking/README.md index 07e9484d4dd..fa39bdf3c79 100644 --- a/plugins/memory/openviking/README.md +++ b/plugins/memory/openviking/README.md @@ -27,7 +27,14 @@ All config via environment variables in `.env`: | Env Var | Default | Description | |---------|---------|-------------| | `OPENVIKING_ENDPOINT` | `http://127.0.0.1:1933` | Server URL | -| `OPENVIKING_API_KEY` | (none) | API key (optional) | +| `OPENVIKING_API_KEY` | (none) | User/admin API key for authenticated servers | +| `OPENVIKING_ACCOUNT` | `default` | Tenant account for local/trusted mode | +| `OPENVIKING_USER` | `default` | Tenant user for local/trusted mode | +| `OPENVIKING_AGENT` | `hermes` | Actor peer ID used for peer-scoped memories | + +When `OPENVIKING_API_KEY` is set, Hermes lets OpenViking derive account/user +identity from the key. In local or trusted deployments without an API key, +Hermes sends `OPENVIKING_ACCOUNT` and `OPENVIKING_USER` as identity headers. ## Tools diff --git a/plugins/memory/openviking/__init__.py b/plugins/memory/openviking/__init__.py index 810f2db43e0..53a51c3f089 100644 --- a/plugins/memory/openviking/__init__.py +++ b/plugins/memory/openviking/__init__.py @@ -10,9 +10,9 @@ lifecycle instead of read-only search endpoints. Config via environment variables (profile-scoped via each profile's .env): OPENVIKING_ENDPOINT — Server URL (default: http://127.0.0.1:1933) OPENVIKING_API_KEY — API key (required for authenticated servers) - OPENVIKING_ACCOUNT — Tenant account (default: default) - OPENVIKING_USER — Tenant user (default: default) - OPENVIKING_AGENT — Tenant agent (default: hermes) + OPENVIKING_ACCOUNT — Tenant account for local/trusted mode (default: default) + OPENVIKING_USER — Tenant user for local/trusted mode (default: default) + OPENVIKING_AGENT — Actor peer ID for Hermes (default: hermes) Capabilities: - Automatic memory extraction on session commit (6 categories) @@ -118,21 +118,18 @@ class _VikingClient: if self._httpx is None: raise ImportError("httpx is required for OpenViking: pip install httpx") - def _headers(self) -> dict: - # Always send tenant headers when account/user are configured. - # OpenViking 0.3.x requires X-OpenViking-Account and X-OpenViking-User - # for ROOT API key requests to tenant-scoped APIs — omitting them - # causes INVALID_ARGUMENT errors even when account="default". - # User-level keys can omit them (server derives tenancy from the key), - # but ROOT keys must always include them explicitly. - h = { - "Content-Type": "application/json", - "X-OpenViking-Agent": self._agent, - } - if self._account: - h["X-OpenViking-Account"] = self._account - if self._user: - h["X-OpenViking-User"] = self._user + def _headers(self, *, include_tenant: bool | None = None) -> dict: + if include_tenant is None: + include_tenant = not bool(self._api_key) + + h = {"Content-Type": "application/json"} + if self._agent: + h["X-OpenViking-Actor-Peer"] = self._agent + if include_tenant: + if self._account: + h["X-OpenViking-Account"] = self._account + if self._user: + h["X-OpenViking-User"] = self._user if self._api_key: h["X-API-Key"] = self._api_key h["Authorization"] = "Bearer " + self._api_key @@ -141,11 +138,33 @@ class _VikingClient: def _url(self, path: str) -> str: return f"{self._endpoint}{path}" - def _multipart_headers(self) -> dict: - headers = self._headers() + def _multipart_headers(self, *, include_tenant: bool | None = None) -> dict: + headers = self._headers(include_tenant=include_tenant) headers.pop("Content-Type", None) return headers + @staticmethod + def _needs_trusted_identity_retry(exc: Exception) -> bool: + message = str(exc) + return ( + "Trusted mode requests must include X-OpenViking-Account" in message + or "Trusted mode requests must include X-OpenViking-User" in message + or "Trusted mode requests must include X-OpenViking-Account or explicit account_id" in message + ) + + def _send_with_trusted_identity_retry(self, send, *, multipart: bool = False) -> dict: + try: + headers = self._multipart_headers() if multipart else self._headers() + return self._parse_response(send(headers)) + except RuntimeError as exc: + if not self._api_key or not self._needs_trusted_identity_retry(exc): + raise + headers = ( + self._multipart_headers(include_tenant=True) + if multipart else self._headers(include_tenant=True) + ) + return self._parse_response(send(headers)) + def _parse_response(self, resp) -> dict: try: data = resp.json() @@ -176,28 +195,33 @@ class _VikingClient: return data def get(self, path: str, **kwargs) -> dict: - resp = self._httpx.get( - self._url(path), headers=self._headers(), timeout=_TIMEOUT, **kwargs + return self._send_with_trusted_identity_retry( + lambda headers: self._httpx.get( + self._url(path), headers=headers, timeout=_TIMEOUT, **kwargs + ) ) - return self._parse_response(resp) def post(self, path: str, payload: dict = None, **kwargs) -> dict: - resp = self._httpx.post( - self._url(path), json=payload or {}, headers=self._headers(), - timeout=_TIMEOUT, **kwargs + return self._send_with_trusted_identity_retry( + lambda headers: self._httpx.post( + self._url(path), json=payload or {}, headers=headers, + timeout=_TIMEOUT, **kwargs + ) ) - return self._parse_response(resp) def upload_temp_file(self, file_path: Path) -> str: mime_type = mimetypes.guess_type(file_path.name)[0] or "application/octet-stream" - with file_path.open("rb") as f: - resp = self._httpx.post( - self._url("/api/v1/resources/temp_upload"), - files={"file": (file_path.name, f, mime_type)}, - headers=self._multipart_headers(), - timeout=_TIMEOUT, - ) - data = self._parse_response(resp) + + def _send(headers): + with file_path.open("rb") as f: + return self._httpx.post( + self._url("/api/v1/resources/temp_upload"), + files={"file": (file_path.name, f, mime_type)}, + headers=headers, + timeout=_TIMEOUT, + ) + + data = self._send_with_trusted_identity_retry(_send, multipart=True) result = data.get("result", {}) temp_file_id = result.get("temp_file_id", "") if not temp_file_id: @@ -442,7 +466,7 @@ class OpenVikingMemoryProvider(MemoryProvider): }, { "key": "api_key", - "description": "OpenViking API key (leave blank for local dev mode)", + "description": "OpenViking user/admin API key (leave blank for local dev mode)", "secret": True, "env_var": "OPENVIKING_API_KEY", }, @@ -460,7 +484,7 @@ class OpenVikingMemoryProvider(MemoryProvider): }, { "key": "agent", - "description": "OpenViking agent ID within the account ([hermes], useful in multi-agent mode)", + "description": "OpenViking actor peer ID for Hermes-scoped memories ([hermes])", "default": "hermes", "env_var": "OPENVIKING_AGENT", }, @@ -542,7 +566,7 @@ class OpenVikingMemoryProvider(MemoryProvider): ) resp = client.post("/api/v1/search/find", { "query": query, - "top_k": 5, + "limit": 5, }) result = resp.get("result", {}) parts = [] @@ -586,10 +610,13 @@ class OpenVikingMemoryProvider(MemoryProvider): "content": user_content[:4000], # trim very long messages }) # Add assistant message - client.post(f"/api/v1/sessions/{sid}/messages", { + assistant_payload = { "role": "assistant", "content": assistant_content[:4000], - }) + } + if self._agent: + assistant_payload["peer_id"] = self._agent + client.post(f"/api/v1/sessions/{sid}/messages", assistant_payload) except Exception as e: logger.debug("OpenViking sync_turn failed: %s", e) @@ -627,9 +654,9 @@ class OpenVikingMemoryProvider(MemoryProvider): logger.warning("OpenViking session commit failed: %s", e) def _build_memory_uri(self, subdir: str) -> str: - """Build a viking:// memory URI under the configured user/agent/subdir.""" + """Build a viking:// memory URI under the configured peer namespace.""" slug = uuid.uuid4().hex[:12] - return f"viking://user/{self._user}/agent/{self._agent}/memories/{subdir}/mem_{slug}.md" + return f"viking://user/peers/{self._agent}/memories/{subdir}/mem_{slug}.md" def on_memory_write( self, @@ -743,14 +770,15 @@ class OpenVikingMemoryProvider(MemoryProvider): payload: Dict[str, Any] = {"query": query} mode = args.get("mode", "auto") - if mode != "auto": - payload["mode"] = mode if args.get("scope"): payload["target_uri"] = args["scope"] if args.get("limit"): - payload["top_k"] = args["limit"] + payload["limit"] = args["limit"] + endpoint = "/api/v1/search/search" if mode == "deep" else "/api/v1/search/find" + if endpoint == "/api/v1/search/search" and self._session_id: + payload["session_id"] = self._session_id - resp = self._client.post("/api/v1/search/find", payload) + resp = self._client.post(endpoint, payload) result = resp.get("result", {}) # Format results for the model — keep it concise diff --git a/tests/openviking_plugin/test_openviking.py b/tests/openviking_plugin/test_openviking.py index 505ac54eb3c..c2307c1e1b5 100644 --- a/tests/openviking_plugin/test_openviking.py +++ b/tests/openviking_plugin/test_openviking.py @@ -236,8 +236,8 @@ class TestOpenVikingBrowse: class TestOpenVikingMemoryUriBuilder: """Regression tests for _build_memory_uri — fixes #36969. - Before the fix the URI omitted /agent/{agent}/, causing all agents - under the same user to share the same memory namespace. + OpenViking's current memory layout stores peer-scoped memories under + viking://user/peers/{peer_id}/... """ def _make_provider(self, user="alice", agent="coder"): @@ -246,19 +246,19 @@ class TestOpenVikingMemoryUriBuilder: p._agent = agent return p - def test_uri_layout_includes_agent_segment(self): - """URI must contain /agent/{agent}/ between user and memories.""" + def test_uri_layout_includes_peer_segment(self): + """URI must contain /peers/{peer_id}/ between user and memories.""" p = self._make_provider(user="alice", agent="coder") uri = p._build_memory_uri("preferences") - assert uri.startswith("viking://user/alice/agent/coder/memories/preferences/mem_") + assert uri.startswith("viking://user/peers/coder/memories/preferences/mem_") assert uri.endswith(".md") - def test_uri_uses_configured_agent_not_default(self): - """_agent value must be interpolated — not hardcoded to 'hermes'.""" + def test_uri_uses_configured_peer_not_default(self): + """_agent value is the OpenViking actor peer ID, not hardcoded to 'hermes'.""" p = self._make_provider(user="alice", agent="research-bot") uri = p._build_memory_uri("entities") - assert "/agent/research-bot/" in uri - assert "/agent/hermes/" not in uri + assert "/peers/research-bot/" in uri + assert "/peers/hermes/" not in uri def test_uri_slug_is_twelve_hex_chars_and_unique(self): """Slug must be 12 hex chars and differ between calls.""" diff --git a/tests/plugins/memory/test_openviking_provider.py b/tests/plugins/memory/test_openviking_provider.py index 3f609cd1d67..2bdfae01a32 100644 --- a/tests/plugins/memory/test_openviking_provider.py +++ b/tests/plugins/memory/test_openviking_provider.py @@ -37,6 +37,41 @@ def test_tool_search_sorts_by_raw_score_across_buckets(): assert result["total"] == 3 +def test_tool_search_uses_current_openviking_find_payload(): + provider = OpenVikingMemoryProvider() + provider._client = MagicMock() + provider._client.post.return_value = {"result": {"memories": [], "resources": [], "skills": []}} + + result = json.loads(provider._tool_search({ + "query": "ranking", + "mode": "fast", + "scope": "viking://resources/docs", + "limit": 7, + })) + + provider._client.post.assert_called_once_with("/api/v1/search/find", { + "query": "ranking", + "target_uri": "viking://resources/docs", + "limit": 7, + }) + assert result["results"] == [] + + +def test_tool_search_deep_uses_session_search_endpoint(): + provider = OpenVikingMemoryProvider() + provider._session_id = "session-123" + provider._client = MagicMock() + provider._client.post.return_value = {"result": {"memories": [], "resources": [], "skills": []}} + + result = json.loads(provider._tool_search({"query": "connect facts", "mode": "deep"})) + + provider._client.post.assert_called_once_with("/api/v1/search/search", { + "query": "connect facts", + "session_id": "session-123", + }) + assert result["results"] == [] + + def test_tool_search_sorts_missing_raw_score_after_negative_scores(): provider = OpenVikingMemoryProvider() provider._client = MagicMock() @@ -313,9 +348,9 @@ def test_viking_client_upload_temp_file_uses_multipart_identity_headers(tmp_path assert "files" in captured_kwargs assert "json" not in captured_kwargs headers = captured_kwargs["headers"] - assert headers["X-OpenViking-Account"] == "test-account" - assert headers["X-OpenViking-User"] == "test-user" - assert headers["X-OpenViking-Agent"] == "test-agent" + assert "X-OpenViking-Account" not in headers + assert "X-OpenViking-User" not in headers + assert headers["X-OpenViking-Actor-Peer"] == "test-agent" assert headers["X-API-Key"] == "test-key" assert "Content-Type" not in headers @@ -350,16 +385,16 @@ def test_viking_client_headers_include_bearer_when_api_key_set(): headers = client._headers() assert headers["X-API-Key"] == "test-key" assert headers["Authorization"] == "Bearer test-key" + assert headers["X-OpenViking-Actor-Peer"] == "hermes" + assert "X-OpenViking-Account" not in headers + assert "X-OpenViking-User" not in headers def test_viking_client_headers_send_tenant_when_default(): - # account/user set to the literal string "default". OpenViking 0.3.x - # requires X-OpenViking-Account and X-OpenViking-User for ROOT API key - # requests to tenant-scoped APIs — omitting them causes - # INVALID_ARGUMENT errors even when account="default". + # Local/trusted mode needs explicit tenant identity headers. client = _VikingClient( "https://example.com", - api_key="test-key", + api_key="", account="default", user="default", agent="hermes", @@ -367,8 +402,8 @@ def test_viking_client_headers_send_tenant_when_default(): headers = client._headers() assert headers["X-OpenViking-Account"] == "default" assert headers["X-OpenViking-User"] == "default" - assert headers["X-OpenViking-Agent"] == "hermes" - assert headers["Authorization"] == "Bearer test-key" + assert headers["X-OpenViking-Actor-Peer"] == "hermes" + assert "Authorization" not in headers def test_viking_client_headers_send_tenant_when_empty_falls_back_to_default(): @@ -384,11 +419,12 @@ def test_viking_client_headers_send_tenant_when_empty_falls_back_to_default(): headers = client._headers() assert headers["X-OpenViking-Account"] == "default" assert headers["X-OpenViking-User"] == "default" + assert headers["X-OpenViking-Actor-Peer"] == "hermes" assert "Authorization" not in headers assert "X-API-Key" not in headers -def test_viking_client_headers_sent_with_real_tenant_values(): +def test_viking_client_headers_can_include_tenant_for_trusted_retry(): client = _VikingClient( "https://example.com", api_key="test-key", @@ -396,9 +432,54 @@ def test_viking_client_headers_sent_with_real_tenant_values(): user="real-user", agent="hermes", ) - headers = client._headers() + headers = client._headers(include_tenant=True) assert headers["X-OpenViking-Account"] == "real-account" assert headers["X-OpenViking-User"] == "real-user" + assert headers["Authorization"] == "Bearer test-key" + + +def test_viking_client_retries_with_tenant_headers_for_trusted_mode(monkeypatch): + client = _VikingClient( + "https://example.com", + api_key="test-key", + account="acct", + user="usr", + agent="hermes", + ) + captured_headers = [] + + def capture_httpx_post(url, **kwargs): + captured_headers.append(dict(kwargs["headers"])) + if len(captured_headers) == 1: + return SimpleNamespace( + status_code=400, + text="", + json=lambda: { + "status": "error", + "error": { + "code": "INVALID_ARGUMENT", + "message": "Trusted mode requests must include X-OpenViking-Account.", + }, + }, + raise_for_status=lambda: None, + ) + return SimpleNamespace( + status_code=200, + text="", + json=lambda: {"status": "ok", "result": {"ok": True}}, + raise_for_status=lambda: None, + ) + + monkeypatch.setattr(client._httpx, "post", capture_httpx_post) + + assert client.post("/api/v1/fs/ls", {"query": "x"}) == { + "status": "ok", + "result": {"ok": True}, + } + assert "X-OpenViking-Account" not in captured_headers[0] + assert "X-OpenViking-User" not in captured_headers[0] + assert captured_headers[1]["X-OpenViking-Account"] == "acct" + assert captured_headers[1]["X-OpenViking-User"] == "usr" def test_viking_client_health_sends_auth_headers(monkeypatch): @@ -420,3 +501,5 @@ def test_viking_client_health_sends_auth_headers(monkeypatch): assert client.health() is True assert captured["url"] == "https://example.com/health" assert captured["headers"]["Authorization"] == "Bearer test-key" + assert captured["headers"]["X-OpenViking-Actor-Peer"] == "hermes" + assert "X-OpenViking-Account" not in captured["headers"] diff --git a/website/docs/user-guide/features/memory-providers.md b/website/docs/user-guide/features/memory-providers.md index 43b70334da6..fb8f8f1e9b9 100644 --- a/website/docs/user-guide/features/memory-providers.md +++ b/website/docs/user-guide/features/memory-providers.md @@ -284,6 +284,8 @@ hermes memory setup # select "openviking" # Or manually: hermes config set memory.provider openviking echo "OPENVIKING_ENDPOINT=http://localhost:1933" >> ~/.hermes/.env +# Authenticated servers should use a user/admin API key: +echo "OPENVIKING_API_KEY=..." >> ~/.hermes/.env ``` **Key features:** @@ -291,6 +293,9 @@ echo "OPENVIKING_ENDPOINT=http://localhost:1933" >> ~/.hermes/.env - Automatic memory extraction on session commit (profile, preferences, entities, events, cases, patterns) - `viking://` URI scheme for hierarchical knowledge browsing +`OPENVIKING_ACCOUNT` and `OPENVIKING_USER` are used for local/trusted mode. +`OPENVIKING_AGENT` names the OpenViking actor peer for Hermes-scoped memories. + --- ### Mem0