fix(openviking): route remember through session extraction
This commit is contained in:
@@ -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/<user>/memories/...` URIs. When a peer ID is configured, it keeps
|
||||
the existing `viking://user/<user>/peers/<peer>/memories/...` path. In both cases,
|
||||
`<user>` 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-<random>` 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:
|
||||
|
||||
@@ -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"))
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user