Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions plugins/memory/openviking/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,9 @@ def __init__(self):
self._client: Optional[_VikingClient] = None
self._endpoint = ""
self._api_key = ""
self._account = "default"
self._user = "default"
self._agent = "hermes"
self._session_id = ""
self._turn_count = 0
self._sync_thread: Optional[threading.Thread] = None
Expand Down Expand Up @@ -318,21 +321,28 @@ def initialize(self, session_id: str, **kwargs) -> None:
self._session_id = session_id
self._turn_count = 0

self._connect()

# Register as the last active provider for atexit safety net
global _last_active_provider
_last_active_provider = self

def _connect(self) -> bool:
try:
self._client = _VikingClient(
client = _VikingClient(
self._endpoint, self._api_key,
account=self._account, user=self._user, agent=self._agent,
)
if not self._client.health():
if not client.health():
logger.warning("OpenViking server at %s is not reachable", self._endpoint)
self._client = None
return False
self._client = client
return True
except ImportError:
logger.warning("httpx not installed — OpenViking plugin disabled")
self._client = None

# Register as the last active provider for atexit safety net
global _last_active_provider
_last_active_provider = self
return False

def system_prompt_block(self) -> str:
if not self._client:
Expand Down Expand Up @@ -498,7 +508,7 @@ def get_tool_schemas(self) -> List[Dict[str, Any]]:
return [SEARCH_SCHEMA, READ_SCHEMA, BROWSE_SCHEMA, REMEMBER_SCHEMA, ADD_RESOURCE_SCHEMA]

def handle_tool_call(self, tool_name: str, args: dict, **kwargs) -> str:
if not self._client:
if not self._client and not self._connect():
return tool_error("OpenViking server not connected")

try:
Expand Down
15 changes: 14 additions & 1 deletion run_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -863,6 +863,15 @@ def _qwen_portal_headers() -> dict:
}


def _memory_provider_tools_allowed(enabled_toolsets=None, disabled_toolsets=None) -> bool:
"""Return whether external memory provider tools may be exposed."""
if disabled_toolsets and "memory" in disabled_toolsets:
return False
if enabled_toolsets is None:
return True
return "memory" in enabled_toolsets

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should resolve toolsets before checking for memory. As written, enabled_toolsets=["hermes-acp"], ["hermes-cli"], or ["all"] returns false even though those toolsets resolve to include "memory", so external provider tools can disappear when memory is actually enabled.



class AIAgent:
"""
AI Agent with tool calling capabilities.
Expand Down Expand Up @@ -1757,7 +1766,11 @@ def __init__(
# through get_tool_definitions()). Duplicate function names cause
# 400 errors on providers that enforce unique names (e.g. Xiaomi
# MiMo via Nous Portal).
if self._memory_manager and self.tools is not None:
if (
self._memory_manager
and self.tools is not None
and _memory_provider_tools_allowed(self.enabled_toolsets, self.disabled_toolsets)
):
_existing_tool_names = {
t.get("function", {}).get("name")
for t in self.tools
Expand Down
13 changes: 13 additions & 0 deletions tests/agent/test_memory_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from agent.memory_provider import MemoryProvider
from agent.memory_manager import MemoryManager
from run_agent import _memory_provider_tools_allowed

# ---------------------------------------------------------------------------
# Concrete test provider
Expand Down Expand Up @@ -979,6 +980,18 @@ def test_memory_manager_tool_injection_deduplicates(self):
assert tool_names.count("web_search") == 1
assert len(existing_tools) == 3 # web_search + ext_recall + ext_remember

def test_memory_provider_tools_respect_enabled_toolsets(self):
"""Provider tools should not bypass platform toolset filtering."""
assert _memory_provider_tools_allowed(None, None) is True
assert _memory_provider_tools_allowed(["memory"], None) is True
assert _memory_provider_tools_allowed([], None) is False
assert _memory_provider_tools_allowed(["terminal", "file"], None) is False

def test_memory_provider_tools_respect_disabled_toolsets(self):
"""An explicit disabled memory toolset must win over default injection."""
assert _memory_provider_tools_allowed(None, ["memory"]) is False
assert _memory_provider_tools_allowed(["memory"], ["memory"]) is False

def test_on_memory_write_tolerates_provider_failure(self):
"""If a provider's on_memory_write raises, others still get notified."""
mgr = MemoryManager()
Expand Down
37 changes: 37 additions & 0 deletions tests/plugins/memory/test_openviking_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,40 @@ def test_tool_search_sorts_missing_raw_score_after_negative_scores():
]
assert [entry["score"] for entry in result["results"]] == [0.1, 0.0, -0.25]
assert result["total"] == 3


def test_handle_tool_call_reconnects_after_startup_health_failure(monkeypatch):
instances = []

class FakeVikingClient:
def __init__(self, endpoint, api_key="", account="", user="", agent=""):
self.endpoint = endpoint
self.posts = []
self.index = len(instances)
instances.append(self)

def health(self):
return self.index > 0

def post(self, path, payload=None, **kwargs):
self.posts.append((path, payload or {}))
return {}

monkeypatch.setenv("OPENVIKING_ENDPOINT", "http://openviking.local")
monkeypatch.setattr("plugins.memory.openviking._VikingClient", FakeVikingClient)

provider = OpenVikingMemoryProvider()
provider.initialize("session-1")

assert provider._client is None

result = json.loads(provider.handle_tool_call("viking_remember", {"content": "stable fact"}))

assert result["status"] == "stored"
assert len(instances) == 2
assert instances[1].posts == [
(
"/api/v1/sessions/session-1/messages",
{"role": "user", "parts": [{"type": "text", "text": "[Remember] stable fact"}]},
)
]