diff --git a/AGENTS.md b/AGENTS.md index 8f227968e3ae..2213f4d24fac 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -438,6 +438,9 @@ Leaks as literal `?[K` text under `prompt_toolkit`'s `patch_stdout`. Use space-p ### DO NOT hardcode cross-tool references in schema descriptions Tool schema descriptions must not mention tools from other toolsets by name (e.g., `browser_navigate` saying "prefer web_search"). Those tools may be unavailable (missing API keys, disabled toolset), causing the model to hallucinate calls to non-existent tools. If a cross-reference is needed, add it dynamically in `get_tool_definitions()` in `model_tools.py` — see the `browser_navigate` / `execute_code` post-processing blocks for the pattern. +### DO NOT bypass abstraction layers to mutate production data +When a system provides a semantic/structured API (e.g., OpenViking's memory tools), **never** use low-level filesystem or raw HTTP endpoints to directly delete, move, or overwrite its internal files. Doing so corrupts indices, vectors, derived metadata, and consistency guarantees. If you need to remove or fix data, use the system's exposed semantic operations; if none exist, stop and ask rather than poking the storage layer directly. This rule applies to databases, vector stores, memory providers, and any layered architecture. + ### Tests must not write to `~/.hermes/` The `_isolate_hermes_home` autouse fixture in `tests/conftest.py` redirects `HERMES_HOME` to a temp dir. Never hardcode `~/.hermes/` paths in tests. diff --git a/run_agent.py b/run_agent.py index 333dda3927fd..931ff2c97918 100644 --- a/run_agent.py +++ b/run_agent.py @@ -6465,14 +6465,14 @@ def flush_memories(self, messages: list = None, min_turns: int = None): if self._cached_system_prompt: api_messages = [{"role": "system", "content": self._cached_system_prompt}] + api_messages - # Make one API call with only the memory tool available - memory_tool_def = None + # Make one API call with memory-related tools available + memory_tool_defs = [] for t in (self.tools or []): - if t.get("function", {}).get("name") == "memory": - memory_tool_def = t - break + name = t.get("function", {}).get("name") + if name in ("memory", "viking_remember"): + memory_tool_defs.append(t) - if not memory_tool_def: + if not memory_tool_defs: messages.pop() # remove flush msg return @@ -6484,7 +6484,7 @@ def flush_memories(self, messages: list = None, min_turns: int = None): response = _call_llm( task="flush_memories", messages=api_messages, - tools=[memory_tool_def], + tools=memory_tool_defs, temperature=0.3, max_tokens=5120, # timeout resolved from auxiliary.flush_memories.timeout config @@ -6496,7 +6496,7 @@ def flush_memories(self, messages: list = None, min_turns: int = None): if not _aux_available and self.api_mode == "codex_responses": # No auxiliary client -- use the Codex Responses path directly codex_kwargs = self._build_api_kwargs(api_messages) - codex_kwargs["tools"] = self._responses_tools([memory_tool_def]) + codex_kwargs["tools"] = self._responses_tools(memory_tool_defs) codex_kwargs["temperature"] = 0.3 if "max_output_tokens" in codex_kwargs: codex_kwargs["max_output_tokens"] = 5120 @@ -6506,7 +6506,7 @@ def flush_memories(self, messages: list = None, min_turns: int = None): from agent.anthropic_adapter import build_anthropic_kwargs as _build_ant_kwargs ant_kwargs = _build_ant_kwargs( model=self.model, messages=api_messages, - tools=[memory_tool_def], max_tokens=5120, + tools=memory_tool_defs, max_tokens=5120, reasoning_config=None, preserve_dots=self._anthropic_preserve_dots(), ) @@ -6515,7 +6515,7 @@ def flush_memories(self, messages: list = None, min_turns: int = None): api_kwargs = { "model": self.model, "messages": api_messages, - "tools": [memory_tool_def], + "tools": memory_tool_defs, "temperature": 0.3, **self._max_tokens_param(5120), } @@ -6557,6 +6557,15 @@ def flush_memories(self, messages: list = None, min_turns: int = None): print(f" 🧠 Memory flush: saved to {args.get('target', 'memory')}") except Exception as e: logger.debug("Memory flush tool call failed: %s", e) + elif tc.function.name == "viking_remember": + try: + args = json.loads(tc.function.arguments) + if self._memory_manager: + self._memory_manager.handle_tool_call("viking_remember", args=args) + if not self.quiet_mode: + print(" 🧠 Memory flush: saved to OpenViking") + except Exception as e: + logger.debug("Memory flush tool call failed: %s", e) except Exception as e: logger.debug("Memory flush API call failed: %s", e) finally: @@ -6609,6 +6618,14 @@ def _compress_context(self, messages: list, system_message: str, *, approx_token if self._session_db: try: + # End-of-session extraction for external memory providers + # (e.g. OpenViking commit) before we rotate the session ID + if self._memory_manager: + try: + self._memory_manager.on_session_end(messages) + except Exception: + pass + # Propagate title to the new session with auto-numbering old_title = self._session_db.get_session_title(self.session_id) self._session_db.end_session(self.session_id, "compression") @@ -6632,6 +6649,17 @@ def _compress_context(self, messages: list, system_message: str, *, approx_token self._session_db.update_system_prompt(self.session_id, new_system_prompt) # Reset flush cursor — new session starts with no messages written self._last_flushed_db_idx = 0 + + # Re-initialize memory providers with the new session_id so + # subsequent sync_turn calls target the new session + if self._memory_manager: + try: + self._memory_manager.initialize_all( + session_id=self.session_id, + platform=self.platform or os.environ.get("HERMES_SESSION_SOURCE", "cli"), + ) + except Exception: + pass except Exception as e: logger.warning("Session DB compression split failed — new session will NOT be indexed: %s", e) diff --git a/tests/run_agent/test_compression_memory_provider.py b/tests/run_agent/test_compression_memory_provider.py new file mode 100644 index 000000000000..beb4f060e832 --- /dev/null +++ b/tests/run_agent/test_compression_memory_provider.py @@ -0,0 +1,100 @@ +"""Tests for memory provider lifecycle during context compression. + +Verifies that external memory providers (e.g. OpenViking) are properly +notified when a session is split due to compression. +""" + +import os +import tempfile +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + +from run_agent import AIAgent + + +@pytest.fixture +def agent_with_mem_mgr(): + with ( + patch("run_agent.get_tool_definitions", return_value=[]), + patch("run_agent.check_toolset_requirements", return_value={}), + patch("run_agent.OpenAI"), + patch.dict(os.environ, {"OPENROUTER_API_KEY": "test-key"}), + ): + a = AIAgent( + model="test/model", + quiet_mode=True, + skip_context_files=True, + skip_memory=False, + session_id="original-session", + ) + # Replace the real memory manager with a mock so we can assert calls + a._memory_manager = MagicMock() + a._memory_manager.build_system_prompt.return_value = "" + a._cached_system_prompt = "You are helpful." + yield a + + +class TestCompressContextNotifiesMemoryProvider: + """_compress_context must call on_session_end and re-initialize providers.""" + + def test_compress_calls_on_session_end_before_split(self, agent_with_mem_mgr): + """When compression splits the session, the memory provider must receive + on_session_end with the full messages so it can commit/archive them.""" + agent = agent_with_mem_mgr + from hermes_state import SessionDB + + with tempfile.TemporaryDirectory() as tmpdir: + db = SessionDB(db_path=Path(tmpdir) / "test.db") + agent._session_db = db + db.create_session(session_id=agent.session_id, source="test") + + messages = [ + {"role": "user", "content": "hello"}, + {"role": "assistant", "content": "hi"}, + {"role": "user", "content": "what is my name"}, + ] + + with patch.object( + agent.context_compressor, "compress", return_value=[ + {"role": "user", "content": "[SUMMARY] earlier chat"}, + {"role": "user", "content": "what is my name"}, + ] + ): + compressed, _ = agent._compress_context( + messages, "system", approx_tokens=1000 + ) + + agent._memory_manager.on_session_end.assert_called_once() + call_args = agent._memory_manager.on_session_end.call_args + assert call_args.args[0] == messages + + def test_compress_reinitializes_memory_provider_with_new_session(self, agent_with_mem_mgr): + """After splitting, the memory provider must be re-initialized with the + new session_id so future sync_turn calls target the new session.""" + agent = agent_with_mem_mgr + original_session_id = agent.session_id + from hermes_state import SessionDB + + with tempfile.TemporaryDirectory() as tmpdir: + db = SessionDB(db_path=Path(tmpdir) / "test.db") + agent._session_db = db + db.create_session(session_id=agent.session_id, source="test") + + messages = [ + {"role": "user", "content": "hello"}, + {"role": "assistant", "content": "hi"}, + ] + + with patch.object( + agent.context_compressor, "compress", return_value=[ + {"role": "user", "content": "[SUMMARY] earlier chat"}, + ] + ): + agent._compress_context(messages, "system", approx_tokens=1000) + + agent._memory_manager.initialize_all.assert_called_once() + call_kwargs = agent._memory_manager.initialize_all.call_args.kwargs + assert call_kwargs["session_id"] != original_session_id + assert agent.session_id == call_kwargs["session_id"] diff --git a/tests/run_agent/test_flush_memories_codex.py b/tests/run_agent/test_flush_memories_codex.py index b4b3c648e654..fef49c853f4a 100644 --- a/tests/run_agent/test_flush_memories_codex.py +++ b/tests/run_agent/test_flush_memories_codex.py @@ -275,3 +275,127 @@ def test_codex_mode_no_aux_uses_responses_api(self, monkeypatch): mock_stream.assert_called_once() mock_memory.assert_called_once() assert mock_memory.call_args.kwargs["content"] == "Codex flush test" + + +def _make_agent_with_tools(monkeypatch, tools, api_mode="chat_completions", provider="openrouter"): + """Build an AIAgent with a specific tool list for flush_memories testing.""" + monkeypatch.setattr(run_agent, "get_tool_definitions", lambda **kw: tools) + monkeypatch.setattr(run_agent, "check_toolset_requirements", lambda: {}) + monkeypatch.setattr(run_agent, "OpenAI", _FakeOpenAI) + + agent = run_agent.AIAgent( + api_key="test-key", + base_url="https://test.example.com/v1", + provider=provider, + api_mode=api_mode, + max_iterations=4, + quiet_mode=True, + skip_context_files=True, + skip_memory=True, + ) + agent._memory_store = MagicMock() + agent._memory_flush_min_turns = 1 + agent._user_turn_count = 5 + return agent + + +def _chat_response_with_viking_remember_call(): + """Simulated response calling viking_remember instead of memory.""" + return SimpleNamespace( + choices=[SimpleNamespace( + message=SimpleNamespace( + content=None, + tool_calls=[SimpleNamespace( + function=SimpleNamespace( + name="viking_remember", + arguments=json.dumps({ + "content": "User likes strawberry cake.", + "category": "preference", + }), + ), + )], + ), + )], + usage=SimpleNamespace(prompt_tokens=100, completion_tokens=20, total_tokens=120), + ) + + +class TestFlushMemoriesExposesVikingRemember: + """flush_memories() must expose viking_remember when OpenViking tools are present.""" + + def test_flush_includes_viking_remember_tool(self, monkeypatch): + """The API call must include viking_remember in the tools list.""" + tools = [ + { + "type": "function", + "function": { + "name": "memory", + "description": "Manage memories.", + "parameters": {"type": "object", "properties": {}}, + }, + }, + { + "type": "function", + "function": { + "name": "viking_remember", + "description": "Store fact in OpenViking.", + "parameters": {"type": "object", "properties": {}}, + }, + }, + ] + agent = _make_agent_with_tools(monkeypatch, tools) + mock_response = _chat_response_with_viking_remember_call() + + with patch("agent.auxiliary_client.call_llm", return_value=mock_response) as mock_call: + messages = [ + {"role": "user", "content": "Hello"}, + {"role": "assistant", "content": "Hi"}, + {"role": "user", "content": "Remember this"}, + ] + agent._memory_manager = MagicMock() + agent.flush_memories(messages) + + mock_call.assert_called_once() + call_kwargs = mock_call.call_args + tool_names = [t["function"]["name"] for t in call_kwargs.kwargs["tools"]] + assert "viking_remember" in tool_names, ( + f"Expected viking_remember in flush tools, got {tool_names}" + ) + + def test_flush_executes_viking_remember_tool_call(self, monkeypatch): + """When the model returns a viking_remember tool call, flush must execute it.""" + tools = [ + { + "type": "function", + "function": { + "name": "memory", + "description": "Manage memories.", + "parameters": {"type": "object", "properties": {}}, + }, + }, + { + "type": "function", + "function": { + "name": "viking_remember", + "description": "Store fact in OpenViking.", + "parameters": {"type": "object", "properties": {}}, + }, + }, + ] + agent = _make_agent_with_tools(monkeypatch, tools) + mock_response = _chat_response_with_viking_remember_call() + + with patch("agent.auxiliary_client.call_llm", return_value=mock_response): + messages = [ + {"role": "user", "content": "Hello"}, + {"role": "assistant", "content": "Hi"}, + {"role": "user", "content": "Remember this"}, + ] + agent._memory_manager = MagicMock() + agent.flush_memories(messages) + + agent._memory_manager.handle_tool_call.assert_called_once() + call_args = agent._memory_manager.handle_tool_call.call_args + assert call_args.args[0] == "viking_remember" + assert call_args.kwargs["args"]["content"] == "User likes strawberry cake." + assert call_args.kwargs["args"]["category"] == "preference"