diff --git a/plugins/memory/openviking/__init__.py b/plugins/memory/openviking/__init__.py index 82f1f26a0a0..a57a60e67bd 100644 --- a/plugins/memory/openviking/__init__.py +++ b/plugins/memory/openviking/__init__.py @@ -49,7 +49,7 @@ from agent.message_content import flatten_message_text from agent.memory_provider import MemoryProvider from agent.skill_commands import extract_user_instruction_from_skill_message from tools.registry import tool_error -from utils import atomic_json_write +from utils import atomic_json_write, env_var_enabled logger = logging.getLogger(__name__) @@ -160,7 +160,7 @@ def _derive_openviking_user_text(content: Any) -> str: def _sync_trace_enabled() -> bool: - return os.environ.get(_SYNC_TRACE_ENV, "").strip().lower() in {"1", "true", "yes", "on"} + return env_var_enabled(_SYNC_TRACE_ENV) def _preview(value: Any, limit: int = 160) -> str: @@ -2461,8 +2461,8 @@ class OpenVikingMemoryProvider(MemoryProvider): tool_id = str(message.get("tool_call_id") or message.get("id") or "") if tool_id: completed_tool_ids.add(tool_id) - if cls._is_openviking_recall_tool_name(message.get("name")): - skipped_tool_ids.add(tool_id) + if cls._is_openviking_recall_tool_name(message.get("name")): + skipped_tool_ids.add(tool_id) continue if message.get("role") != "assistant": continue diff --git a/tests/openviking_plugin/test_openviking.py b/tests/openviking_plugin/test_openviking.py index 3a743287672..171e6abc8ac 100644 --- a/tests/openviking_plugin/test_openviking.py +++ b/tests/openviking_plugin/test_openviking.py @@ -539,6 +539,51 @@ class TestOpenVikingTurnConversion: assert recall_tool_name not in batch_text assert "Old OpenViking memory content" not in batch_text + def test_messages_to_openviking_batch_empty_tool_id_does_not_drop_other_results(self): + # A recall tool result that arrives with an empty tool_call_id must not + # poison the skip set with "" and silently drop unrelated tool results + # that also lack an id. Empty tool_call_id is reachable in the canonical + # transcript (agent_runtime_helpers defaults it to ""). + turn = [ + {"role": "user", "content": "What did we decide?"}, + { + "role": "assistant", + "content": "", + "tool_calls": [ + { + "id": "", + "type": "function", + "function": { + "name": "viking_search", + "arguments": json.dumps({"query": "decision"}), + }, + } + ], + }, + { + "role": "tool", + "tool_call_id": "", + "name": "viking_search", + "content": json.dumps({"results": ["recall stuff"]}), + }, + { + "role": "tool", + "tool_call_id": "", + "name": "shell_command", + "content": "important shell output", + }, + {"role": "assistant", "content": "done"}, + ] + + batch = OpenVikingMemoryProvider._messages_to_openviking_batch(turn) + + batch_text = json.dumps(batch) + # The unrelated (empty-id) shell result must survive. + assert "important shell output" in batch_text + # The recall tool result must still be excluded. + assert "recall stuff" not in batch_text + assert "viking_search" not in batch_text + def test_messages_to_openviking_batch_preserves_responses_text_parts(self): turn = [ {"role": "user", "content": [{"type": "input_text", "text": "hello"}]},