fix(openviking): clarify remember submission status
This commit is contained in:
@@ -129,15 +129,30 @@ changes future writes, not the location of existing memories.
|
||||
`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
|
||||
classifies the source and can add, merge, or skip a memory through its normal
|
||||
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.
|
||||
The tool returns `status: submitted` because extraction can add a memory, merge
|
||||
the fact into an existing memory, or produce no memory operation. It does not
|
||||
promise that OpenViking created a distinct memory file. The fact is submitted
|
||||
as an unchanged `user` message so OpenViking owns the final classification.
|
||||
The legacy `category` argument is still accepted from existing callers but is
|
||||
not advertised or used. The one-shot session is separate from the live Hermes
|
||||
conversation, so an explicit remember does not commit or rotate the active
|
||||
conversation session.
|
||||
|
||||
If the message request or commit fails, the error includes the canonical
|
||||
session URI, the failed stage, the observed message status, and an `ov session
|
||||
commit <session-id>` recovery command. Inspect the session first. An archive
|
||||
means the commit completed. A non-empty live `messages.jsonl` with no archive
|
||||
means the message was accepted but still needs a commit. An empty live file
|
||||
without an archive is ambiguous and must not trigger an automatic resubmission.
|
||||
Use the same OpenViking profile and credentials as Hermes for manual recovery.
|
||||
OpenViking server auto-commit is disabled by default, so an accepted message
|
||||
whose explicit commit fails normally remains live and unextracted until it is
|
||||
manually committed.
|
||||
|
||||
Hermes built-in `memory` tool additions are mirrored to OpenViking after the
|
||||
local memory operation succeeds:
|
||||
|
||||
@@ -614,21 +614,17 @@ BROWSE_SCHEMA = {
|
||||
REMEMBER_SCHEMA = {
|
||||
"name": "viking_remember",
|
||||
"description": (
|
||||
"Explicitly submit a fact or memory to the OpenViking memory pipeline. "
|
||||
"Use for important information the agent should remember long-term. "
|
||||
"OpenViking automatically categorizes, merges, and indexes the memory."
|
||||
"Submit important long-term information to OpenViking through session "
|
||||
"memory extraction. Success means the source was submitted, not that a "
|
||||
"distinct memory file was created. OpenViking can add, merge, or skip the "
|
||||
"final memory. If the message is accepted but commit fails, it normally "
|
||||
"remains live and unextracted because server auto-commit is disabled by "
|
||||
"default; follow the returned recovery instructions."
|
||||
),
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"content": {"type": "string", "description": "The information to remember."},
|
||||
"category": {
|
||||
"type": "string",
|
||||
"enum": ["preference", "entity", "event", "case", "pattern"],
|
||||
"description": (
|
||||
"Optional extraction hint. OpenViking makes the final classification."
|
||||
),
|
||||
},
|
||||
},
|
||||
"required": ["content"],
|
||||
},
|
||||
@@ -5264,28 +5260,56 @@ class OpenVikingMemoryProvider(MemoryProvider):
|
||||
if not client:
|
||||
return tool_error("OpenViking server not connected")
|
||||
|
||||
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]}"
|
||||
session_uri = _user_scoped_uri(
|
||||
self._user_space(client),
|
||||
f"sessions/{session_id}",
|
||||
)
|
||||
recovery_command = f"ov session commit {session_id}"
|
||||
recovery_note = (
|
||||
"Inspect session_uri before recovery. If history/archive_* exists, do not "
|
||||
"retry. If messages.jsonl contains the fact and no archive exists, run "
|
||||
"recovery_command with the same OpenViking profile and credentials as "
|
||||
"Hermes. Otherwise, do not resubmit automatically; report the uncertain "
|
||||
"state to the user."
|
||||
)
|
||||
message: Dict[str, Any] = {
|
||||
"role": "user",
|
||||
"parts": [self._text_part(message_content)],
|
||||
"parts": [self._text_part(content)],
|
||||
}
|
||||
|
||||
# Use a dedicated session so explicit remember does not commit or
|
||||
# otherwise alter the live Hermes conversation session.
|
||||
try:
|
||||
client.post(f"/api/v1/sessions/{session_id}/messages", message)
|
||||
except Exception as e:
|
||||
logger.error("OpenViking remember message failed for %s: %s", session_id, e)
|
||||
return tool_error(
|
||||
f"Memory message submission failed for session {session_id}: {e}",
|
||||
session_id=session_id,
|
||||
session_uri=session_uri,
|
||||
failure_stage="message",
|
||||
message_status="unknown",
|
||||
recovery_command=recovery_command,
|
||||
recovery_note=recovery_note,
|
||||
)
|
||||
|
||||
try:
|
||||
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",
|
||||
"status": "submitted",
|
||||
"session_id": session_id,
|
||||
"session_uri": session_uri,
|
||||
"message_status": "accepted",
|
||||
"extraction_status": str(commit.get("status") or "accepted"),
|
||||
"message": "Memory stored in an OpenViking session and committed for extraction.",
|
||||
"message": (
|
||||
"Memory source submitted to OpenViking session extraction. "
|
||||
"OpenViking may add, merge, or skip the final memory."
|
||||
),
|
||||
}
|
||||
if commit.get("task_id"):
|
||||
result["task_id"] = commit["task_id"]
|
||||
@@ -5293,8 +5317,16 @@ class OpenVikingMemoryProvider(MemoryProvider):
|
||||
result["trace_id"] = commit["trace_id"]
|
||||
return json.dumps(result)
|
||||
except Exception as 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}")
|
||||
logger.error("OpenViking remember commit failed for %s: %s", session_id, e)
|
||||
return tool_error(
|
||||
f"Memory message was accepted, but commit failed for session {session_id}: {e}",
|
||||
session_id=session_id,
|
||||
session_uri=session_uri,
|
||||
failure_stage="commit",
|
||||
message_status="accepted",
|
||||
recovery_command=recovery_command,
|
||||
recovery_note=recovery_note,
|
||||
)
|
||||
|
||||
def _tool_forget(self, args: dict) -> str:
|
||||
uri, error = _validate_forget_memory_uri(args.get("uri"))
|
||||
|
||||
@@ -971,8 +971,10 @@ class TestEnsureClientReloadsEnv:
|
||||
{"content": "stable fact"},
|
||||
))
|
||||
|
||||
assert out["status"] == "stored"
|
||||
assert out["status"] == "submitted"
|
||||
assert out["session_id"].startswith("hermes-remember-")
|
||||
assert out["session_uri"] == f"viking://user/default/sessions/{out['session_id']}"
|
||||
assert out["message_status"] == "accepted"
|
||||
assert out["extraction_status"] == "accepted"
|
||||
assert out["task_id"] == "task-remember"
|
||||
assert out["trace_id"] == "trace-remember"
|
||||
@@ -992,21 +994,7 @@ class TestEnsureClientReloadsEnv:
|
||||
),
|
||||
]
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"category",
|
||||
[
|
||||
"preference",
|
||||
"entity",
|
||||
"event",
|
||||
"case",
|
||||
"pattern",
|
||||
],
|
||||
)
|
||||
def test_remember_uses_category_as_user_memory_hint(
|
||||
self,
|
||||
monkeypatch,
|
||||
category,
|
||||
):
|
||||
def test_remember_accepts_legacy_category_but_submits_raw_user_text(self, monkeypatch):
|
||||
posts = []
|
||||
|
||||
class _StubClient:
|
||||
@@ -1023,17 +1011,16 @@ class TestEnsureClientReloadsEnv:
|
||||
|
||||
out = json.loads(provider._tool_remember({
|
||||
"content": "stable fact",
|
||||
"category": category,
|
||||
"category": "preference",
|
||||
}))
|
||||
|
||||
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 message["parts"] == [{"type": "text", "text": "stable fact"}]
|
||||
assert "peer_id" not in message
|
||||
assert "category" not in openviking_plugin.REMEMBER_SCHEMA["parameters"]["properties"]
|
||||
assert posts[1] == (
|
||||
f"/api/v1/sessions/{session_id}/commit",
|
||||
{"keep_recent_count": 0},
|
||||
@@ -1061,7 +1048,31 @@ class TestEnsureClientReloadsEnv:
|
||||
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):
|
||||
def test_remember_reports_unknown_message_submission_failure(self, monkeypatch):
|
||||
posts = []
|
||||
|
||||
class _StubClient:
|
||||
def post(self, path, payload=None, **kwargs):
|
||||
posts.append((path, payload or {}))
|
||||
raise TimeoutError("message timeout")
|
||||
|
||||
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("Memory message submission failed for session ")
|
||||
assert out["error"].endswith(": message timeout")
|
||||
assert out["failure_stage"] == "message"
|
||||
assert out["message_status"] == "unknown"
|
||||
assert out["session_uri"].endswith(f"/sessions/{out['session_id']}")
|
||||
assert out["recovery_command"] == f"ov session commit {out['session_id']}"
|
||||
assert "do not resubmit automatically" in out["recovery_note"]
|
||||
assert len(posts) == 1
|
||||
assert posts[0][0].endswith("/messages")
|
||||
|
||||
def test_remember_reports_commit_failure_with_recovery_command(self, monkeypatch):
|
||||
posts = []
|
||||
|
||||
class _StubClient:
|
||||
@@ -1078,9 +1089,14 @@ class TestEnsureClientReloadsEnv:
|
||||
out = json.loads(provider._tool_remember({"content": "stable fact"}))
|
||||
|
||||
assert out["error"].startswith(
|
||||
"Failed to store memory in session hermes-remember-"
|
||||
"Memory message was accepted, but commit failed for session hermes-remember-"
|
||||
)
|
||||
assert out["error"].endswith(": commit rejected")
|
||||
assert out["failure_stage"] == "commit"
|
||||
assert out["message_status"] == "accepted"
|
||||
assert out["session_uri"].endswith(f"/sessions/{out['session_id']}")
|
||||
assert out["recovery_command"] == f"ov session commit {out['session_id']}"
|
||||
assert "same OpenViking profile and credentials as Hermes" in out["recovery_note"]
|
||||
assert len(posts) == 2
|
||||
assert posts[0][0].endswith("/messages")
|
||||
assert posts[1][0].endswith("/commit")
|
||||
|
||||
@@ -290,7 +290,7 @@ def test_wire_requests_keep_writes_and_session_messages_in_the_selected_scope(
|
||||
result = json.loads(
|
||||
provider.handle_tool_call("viking_remember", {"content": "I like tea"})
|
||||
)
|
||||
assert result["status"] == "stored"
|
||||
assert result["status"] == "submitted"
|
||||
provider.on_memory_write("add", "user", "I like coffee")
|
||||
provider.sync_turn("hello", "hi", session_id="peer-test")
|
||||
assert provider._drain_writers("peer-test", timeout=5.0)
|
||||
|
||||
Reference in New Issue
Block a user