🐛 Bugfix: Guard memory_user_config against None in store/search memory tools (#3359)

JasonW404 · web-flow · commit 8eae4ed9d08f · 2026-07-02T21:09:18.000+08:00
* 🐛 Bugfix: Guard memory_user_config against None in store/search memory tools

Add None checks before accessing memory_user_config attributes in
StoreMemoryTool and SearchMemoryTool. When config is unavailable,
apply conservative default (agent_share_option='never') to prevent
unintended cross-agent memory sharing.

* fix(memory): restore hidden tabs and hide Tenant Shared in speed mode

- Fix nested array syntax bug that hid Tenant Shared and Agent Shared tabs
- Conditionally hide Tenant Shared tab when isSpeedMode=true (no tenant concept)
- All 5 tabs now properly visible in non-speed mode
- 4 tabs visible in speed mode (Tenant Shared hidden)

* test: add coverage for memory None guard, duplicate cleanup, and system tool filtering

- test_store_memory_tool: add None config, _run_coroutine tests
- test_search_memory_tool: add None config, _run_coroutine tests
- test_memory_config_service: add duplicate record cleanup tests
- test_tool_configuration_service: add SYSTEM_MANAGED_TOOL_NAMES filter test
diff --git a/backend/agents/create_agent_info.py b/backend/agents/create_agent_info.py
@@ -762,6 +762,9 @@ async def create_agent_config(
                 "agent_id": memory_context.agent_id,
             }
 
+            memory_tool_names = {"store_memory", "search_memory"}
+            tool_list = [t for t in tool_list if t.name not in memory_tool_names]
+
             store_tool_config = ToolConfig(
                 class_name="StoreMemoryTool",
                 name="store_memory",
diff --git a/backend/consts/tool_labels.py b/backend/consts/tool_labels.py
@@ -52,3 +52,5 @@
 BUILTIN_LABEL_MAP.update(_category_email)
 BUILTIN_LABEL_MAP.update(_category_memory)
 BUILTIN_LABEL_MAP.update(_category_terminal)
+
+SYSTEM_MANAGED_TOOL_NAMES = frozenset({"store_memory", "search_memory"})
diff --git a/backend/services/memory_config_service.py b/backend/services/memory_config_service.py
@@ -94,6 +94,17 @@ def _update_single_config(user_id: str, config_key: str, config_value: str) -> b
 				f"update_config_by_id failed, user_id={user_id}, key={config_key}, value={config_value}"
 			)
 			return False
+		
+		# Handle duplicate records: delete all except the first one
+		if len(record_list) > 1:
+			logger.warning(
+				f"Found {len(record_list)} duplicate records for user_id={user_id}, key={config_key}. Cleaning up..."
+			)
+			for duplicate in record_list[1:]:
+				dup_id = duplicate["config_id"]
+				delete_result = delete_config_by_config_id(dup_id, updated_by=user_id)
+				if not delete_result:
+					logger.error(f"Failed to delete duplicate config_id={dup_id}")
 	return True
 
 
diff --git a/backend/services/tool_configuration_service.py b/backend/services/tool_configuration_service.py
@@ -15,6 +15,7 @@
 from consts.const import DATA_PROCESS_SERVICE, LOCAL_MCP_SERVER, MCP_MANAGEMENT_API
 from consts.exceptions import MCPConnectionError, NotFoundException, ToolExecutionException
 from consts.model import ToolInstanceInfoRequest, ToolInfo, ToolSourceEnum, ToolValidateRequest
+from consts.tool_labels import SYSTEM_MANAGED_TOOL_NAMES
 from database.outer_api_tool_db import (
     upsert_openapi_service,
     query_openapi_services_by_tenant,
@@ -506,6 +507,9 @@ async def list_all_tools(tenant_id: str, labels: Optional[List[str]] = None):
     for tool in tools_info:
         tool_name = tool.get("name")
 
+        if tool_name in SYSTEM_MANAGED_TOOL_NAMES:
+            continue
+
         # Always use SDK inputs for local tools to stay in sync with current tool code
         is_local = tool.get("source") == "local"
         sdk_info = local_tool_descriptions.get(tool_name) if is_local else None
diff --git a/frontend/app/[locale]/memory/page.tsx b/frontend/app/[locale]/memory/page.tsx
@@ -297,32 +297,30 @@ export default function MemoryContent() {
       ),
       children: renderBaseSettings(),
     },
- [
-          {
-            key: MEMORY_TAB_KEYS.TENANT,
-            label: (
-              <span className="inline-flex items-center gap-2">
-                <UsersRound className="size-4" />
-                {t("memoryManageModal.tenantShareTab")}
-              </span>
-            ),
-            children: renderSingleList(memory.tenantSharedGroup),
-            disabled: !memory.memoryEnabled,
-          },
-          {
-            key: MEMORY_TAB_KEYS.AGENT_SHARED,
-            label: (
-              <span className="inline-flex items-center gap-2">
-                <Share2 className="size-4" />
-                {t("memoryManageModal.agentShareTab")}
-              </span>
-            ),
-            children: renderMemoryWithMenu(memory.agentSharedGroups, true),
-            disabled:
-              !memory.memoryEnabled ||
-              memory.shareOption === MEMORY_SHARE_STRATEGY.NEVER,
-          },
-        ],
+    ...(!isSpeedMode ? [{
+      key: MEMORY_TAB_KEYS.TENANT,
+      label: (
+        <span className="inline-flex items-center gap-2">
+          <UsersRound className="size-4" />
+          {t("memoryManageModal.tenantShareTab")}
+        </span>
+      ),
+      children: renderSingleList(memory.tenantSharedGroup),
+      disabled: !memory.memoryEnabled,
+    }] : []),
+    {
+      key: MEMORY_TAB_KEYS.AGENT_SHARED,
+      label: (
+        <span className="inline-flex items-center gap-2">
+          <Share2 className="size-4" />
+          {t("memoryManageModal.agentShareTab")}
+        </span>
+      ),
+      children: renderMemoryWithMenu(memory.agentSharedGroups, true),
+      disabled:
+        !memory.memoryEnabled ||
+        memory.shareOption === MEMORY_SHARE_STRATEGY.NEVER,
+    },
     {
       key: MEMORY_TAB_KEYS.USER_PERSONAL,
       label: (
diff --git a/sdk/nexent/core/tools/search_memory_tool.py b/sdk/nexent/core/tools/search_memory_tool.py
@@ -1,4 +1,5 @@
 import asyncio
+import concurrent.futures
 import logging
 from typing import Any
 
@@ -11,6 +12,15 @@
 logger = logging.getLogger("search_memory_tool")
 
 
+def _run_coroutine(coro):
+    try:
+        asyncio.get_running_loop()
+    except RuntimeError:
+        return asyncio.run(coro)
+    with concurrent.futures.ThreadPoolExecutor(max_workers=1) as pool:
+        return pool.submit(asyncio.run, coro).result()
+
+
 class SearchMemoryTool(Tool):
     name = "search_memory"
     description = (
@@ -63,25 +73,34 @@ def __init__(
         self.running_prompt_en = "Searching memory..."
         self.running_prompt_zh = "搜索记忆中..."
 
+    def _resolve_memory_levels(self) -> list[str]:
+        """Determine which memory levels to search based on user preferences.
+
+        Conservative default: when config is unavailable, assume "never"
+        (no agent-level sharing) to protect user privacy.
+        """
+        levels = ["tenant", "user", "agent", "user_agent"]
+        if not self.memory_user_config or self.memory_user_config.agent_share_option == "never":
+            levels.remove("agent")
+        if self.memory_user_config and self.agent_id in getattr(self.memory_user_config, "disable_agent_ids", []):
+            if "agent" in levels:
+                levels.remove("agent")
+        if self.memory_user_config and self.agent_id in getattr(self.memory_user_config, "disable_user_agent_ids", []):
+            if "user_agent" in levels:
+                levels.remove("user_agent")
+        return levels
+
     def forward(self, query: str, top_k: int = 5) -> str:
         logger.info(f"[ACTIVE MEMORY] SearchMemoryTool invoked: query={query[:200]}, top_k={top_k}, user_id={self.user_id}, agent_id={self.agent_id}")
         if self.observer:
             running_prompt = self.running_prompt_zh if self.observer.lang == "zh" else self.running_prompt_en
             self.observer.add_message("", ProcessType.TOOL, running_prompt)
 
-        memory_levels = ["tenant", "user", "agent", "user_agent"]
-        if self.memory_user_config.agent_share_option == "never":
-            memory_levels.remove("agent")
-        if self.agent_id in getattr(self.memory_user_config, "disable_agent_ids", []):
-            if "agent" in memory_levels:
-                memory_levels.remove("agent")
-        if self.agent_id in getattr(self.memory_user_config, "disable_user_agent_ids", []):
-            if "user_agent" in memory_levels:
-                memory_levels.remove("user_agent")
+        memory_levels = self._resolve_memory_levels()
 
         try:
             from ...memory.memory_service import search_memory_in_levels
-            result = asyncio.run(search_memory_in_levels(
+            result = _run_coroutine(search_memory_in_levels(
                 query_text=query,
                 memory_config=self.memory_config,
                 tenant_id=self.tenant_id,
diff --git a/sdk/nexent/core/tools/store_memory_tool.py b/sdk/nexent/core/tools/store_memory_tool.py
@@ -1,4 +1,5 @@
 import asyncio
+import concurrent.futures
 import logging
 from typing import Any
 
@@ -11,6 +12,15 @@
 logger = logging.getLogger("store_memory_tool")
 
 
+def _run_coroutine(coro):
+    try:
+        asyncio.get_running_loop()
+    except RuntimeError:
+        return asyncio.run(coro)
+    with concurrent.futures.ThreadPoolExecutor(max_workers=1) as pool:
+        return pool.submit(asyncio.run, coro).result()
+
+
 class StoreMemoryTool(Tool):
     name = "store_memory"
     description = (
@@ -58,6 +68,21 @@ def __init__(
         self.running_prompt_en = "Saving to memory..."
         self.running_prompt_zh = "保存到记忆中..."
 
+    def _resolve_memory_levels(self) -> list[str]:
+        """Determine which memory levels to write to based on user preferences.
+
+        Conservative default: when config is unavailable, assume "never"
+        (no agent-level sharing) to protect user privacy.
+        """
+        levels = ["user_agent", "agent"]
+        if not self.memory_user_config or self.memory_user_config.agent_share_option == "never":
+            levels.remove("agent")
+        if self.memory_user_config and self.agent_id in getattr(self.memory_user_config, "disable_user_agent_ids", []):
+            levels = [l for l in levels if l != "user_agent"]
+        if self.memory_user_config and self.agent_id in getattr(self.memory_user_config, "disable_agent_ids", []):
+            levels = [l for l in levels if l != "agent"]
+        return levels
+
     def forward(self, content: str) -> str:
         logger.info(f"[ACTIVE MEMORY] StoreMemoryTool invoked: content={content[:200]}, user_id={self.user_id}, agent_id={self.agent_id}, store_count={self.store_count}/{self.max_stores_per_run}")
         if self.observer:
@@ -67,19 +92,13 @@ def forward(self, content: str) -> str:
         if self.store_count >= self.max_stores_per_run:
             return "Memory storage limit reached for this conversation. Information will be saved automatically at the end."
 
-        levels = ["user_agent", "agent"]
-        if self.memory_user_config.agent_share_option == "never":
-            levels.remove("agent")
-        if self.agent_id in getattr(self.memory_user_config, "disable_user_agent_ids", []):
-            levels = [l for l in levels if l != "user_agent"]
-        if self.agent_id in getattr(self.memory_user_config, "disable_agent_ids", []):
-            levels = [l for l in levels if l != "agent"]
+        levels = self._resolve_memory_levels()
         if not levels:
             return "No memory levels available (all disabled by user preferences)."
 
         try:
             from ...memory.memory_service import add_memory_in_levels
-            result = asyncio.run(add_memory_in_levels(
+            result = _run_coroutine(add_memory_in_levels(
                 messages=[{"role": "user", "content": content}],
                 memory_config=self.memory_config,
                 tenant_id=self.tenant_id,
diff --git a/test/backend/services/test_memory_config_service.py b/test/backend/services/test_memory_config_service.py
@@ -188,6 +188,38 @@ def test_update_single_config_insert_branch_fail(self, m_get_info, m_insert):
         ok = _update_single_config(self.user_id, "MEMORY_SWITCH", "Y")
         self.assertFalse(ok)
 
+    @patch("backend.services.memory_config_service.delete_config_by_config_id", return_value=True)
+    @patch("backend.services.memory_config_service.update_config_by_id", return_value=True)
+    @patch("backend.services.memory_config_service.get_memory_config_info")
+    def test_update_single_config_cleans_up_duplicates(self, m_get_info, m_update, m_del):
+        m_get_info.return_value = [
+            {"config_id": 10, "config_value": "N"},
+            {"config_id": 20, "config_value": "Y"},
+            {"config_id": 30, "config_value": "N"},
+        ]
+        from backend.services.memory_config_service import _update_single_config
+
+        ok = _update_single_config(self.user_id, "MEMORY_SWITCH", "Y")
+        self.assertTrue(ok)
+        m_update.assert_called_once()
+        self.assertEqual(m_del.call_count, 2)
+        m_del.assert_any_call(20, updated_by=self.user_id)
+        m_del.assert_any_call(30, updated_by=self.user_id)
+
+    @patch("backend.services.memory_config_service.delete_config_by_config_id", return_value=False)
+    @patch("backend.services.memory_config_service.update_config_by_id", return_value=True)
+    @patch("backend.services.memory_config_service.get_memory_config_info")
+    def test_update_single_config_duplicate_cleanup_failure_logged(self, m_get_info, m_update, m_del):
+        m_get_info.return_value = [
+            {"config_id": 10, "config_value": "N"},
+            {"config_id": 20, "config_value": "Y"},
+        ]
+        from backend.services.memory_config_service import _update_single_config
+
+        ok = _update_single_config(self.user_id, "MEMORY_SWITCH", "Y")
+        self.assertTrue(ok)
+        m_del.assert_called_once_with(20, updated_by=self.user_id)
+
     # ------------------------------ _add_multi_value ------------------------------
     @patch("backend.services.memory_config_service.insert_config", return_value=True)
     @patch("backend.services.memory_config_service.get_memory_config_info", return_value=[])
diff --git a/test/backend/services/test_tool_configuration_service.py b/test/backend/services/test_tool_configuration_service.py
@@ -1105,6 +1105,32 @@ async def test_list_all_tools_with_labels(self, mock_query_by_labels, mock_descr
         assert result[0]["tool_id"] == 2
         mock_query_by_labels.assert_called_once_with("tenant1", ["database", "file"])
 
+    @patch('backend.services.tool_configuration_service.get_local_tools_description_zh')
+    @patch('backend.services.tool_configuration_service.query_all_tools')
+    async def test_list_all_tools_filters_system_managed_tools(self, mock_query, mock_descriptions):
+        """list_all_tools filters out tools in SYSTEM_MANAGED_TOOL_NAMES."""
+        mock_query.return_value = [
+            {"tool_id": 1, "name": "tavily_search", "description": "d1", "source": "local",
+             "params": [], "inputs": "{}", "is_available": True, "create_time": "", "usage": ""},
+            {"tool_id": 2, "name": "store_memory", "description": "d2", "source": "local",
+             "params": [], "inputs": "{}", "is_available": True, "create_time": "", "usage": ""},
+            {"tool_id": 3, "name": "search_memory", "description": "d3", "source": "local",
+             "params": [], "inputs": "{}", "is_available": True, "create_time": "", "usage": ""},
+            {"tool_id": 4, "name": "postgres_database", "description": "d4", "source": "local",
+             "params": [], "inputs": "{}", "is_available": True, "create_time": "", "usage": ""},
+        ]
+        mock_descriptions.return_value = {}
+
+        from backend.services.tool_configuration_service import list_all_tools
+        result = await list_all_tools("tenant1")
+
+        result_names = [t["name"] for t in result]
+        assert "store_memory" not in result_names
+        assert "search_memory" not in result_names
+        assert "tavily_search" in result_names
+        assert "postgres_database" in result_names
+        assert len(result) == 2
+
 
 # test the fixture and helper function
 @pytest.fixture
diff --git a/test/sdk/core/tools/test_search_memory_tool.py b/test/sdk/core/tools/test_search_memory_tool.py
@@ -2,7 +2,7 @@
 from unittest.mock import MagicMock, patch, AsyncMock
 
 from sdk.nexent.core.utils.observer import MessageObserver, ProcessType
-from sdk.nexent.core.tools.search_memory_tool import SearchMemoryTool
+from sdk.nexent.core.tools.search_memory_tool import SearchMemoryTool, _run_coroutine
 
 
 @pytest.fixture
@@ -207,3 +207,47 @@ def test_forward_exception_returns_friendly_error(search_memory_tool):
     assert "Memory search failed" in result
     assert "Elasticsearch timeout" in result
     assert "Continuing without memory results" in result
+
+
+def test_levels_none_config_conservative_default(mock_observer):
+    tool = SearchMemoryTool(
+        memory_config={"test": "config"},
+        tenant_id="tenant_1",
+        user_id="user_1",
+        agent_id="agent_1",
+        memory_user_config=None,
+        observer=mock_observer,
+    )
+
+    with patch(
+        "sdk.nexent.memory.memory_service.search_memory_in_levels",
+        new_callable=AsyncMock,
+        return_value={"results": []},
+    ) as mock_search:
+        tool.forward("query")
+
+    call_kwargs = mock_search.call_args[1]
+    assert "agent" not in call_kwargs["memory_levels"]
+    assert "tenant" in call_kwargs["memory_levels"]
+    assert "user" in call_kwargs["memory_levels"]
+    assert "user_agent" in call_kwargs["memory_levels"]
+
+
+def test_run_coroutine_no_running_loop():
+    async def sample_coro():
+        return "result"
+
+    result = _run_coroutine(sample_coro())
+    assert result == "result"
+
+
+def test_run_coroutine_with_running_loop():
+    async def sample_coro():
+        return "result"
+
+    async def run_with_loop():
+        return _run_coroutine(sample_coro())
+
+    import asyncio
+    result = asyncio.run(run_with_loop())
+    assert result == "result"
diff --git a/test/sdk/core/tools/test_store_memory_tool.py b/test/sdk/core/tools/test_store_memory_tool.py

Original file line number	Diff line number	Diff line change
`@@ -762,6 +762,9 @@ async def create_agent_config(`
`762`	`762`	`"agent_id": memory_context.agent_id,`
`763`	`763`	`}`
`764`	`764`
	`765`	`+ memory_tool_names = {"store_memory", "search_memory"}`
	`766`	`+ tool_list = [t for t in tool_list if t.name not in memory_tool_names]`
	`767`	`+`
`765`	`768`	`store_tool_config = ToolConfig(`
`766`	`769`	`class_name="StoreMemoryTool",`
`767`	`770`	`name="store_memory",`