🐛 Bugfix:fix aidp search tool params' save error#3296 (#3297)

* 🐛 Bugfix: Update HTTP client settings to increase timeout and disable SSL verification in aidp_service and aidp_search_tool (#3280)

* 🐛 Bugfix: Fix page show

* 🐛 Bugfix: Prevent saving null values in tool parameters across backend and frontend components. Ensure only defined values are used when merging and updating tool configurations.

* 🐛 Bugfix: Ensure `useSaveGuard` returns true upon successful save and update unit tests to reflect changes in return type for tool instance creation and update.

Author: DongJiBao2001

backend/database/tool_db.py

@@ -47,6 +47,13 @@ def create_or_update_tool_by_tool_info(tool_info, tenant_id: str, user_id: str,
     tool_info_dict = tool_info.__dict__ | {
         "tenant_id": tenant_id, "user_id": user_id, "version_no": version_no}
 
+    # Filter out null values from params to avoid saving nulls to database
+    if 'params' in tool_info_dict and tool_info_dict['params'] is not None:
+        tool_info_dict['params'] = {
+            k: v for k, v in tool_info_dict['params'].items()
+            if v is not None
+        }
+
     with get_db_session() as session:
         # Query if there is an existing ToolInstance
         # Note: Do not filter by user_id to avoid creating duplicate instances
@@ -71,7 +78,7 @@ def create_or_update_tool_by_tool_info(tool_info, tenant_id: str, user_id: str,
             session.add(new_tool_instance)
             session.flush()  # Flush to get the ID
             tool_instance = new_tool_instance
-        return tool_instance
+        return as_dict(tool_instance)
 
 
 def query_all_tools(tenant_id: str):
@@ -258,7 +265,11 @@ def add_tool_field(tool_info):
         tool_params = tool.params
         for ele in tool_params:
             param_name = ele["name"]
-            ele["default"] = tool_info["params"].get(param_name)
+            instance_value = tool_info["params"].get(param_name)
+            # Only set default if instance value is not None
+            # This prevents null values from being saved to database and returned as defaults
+            if instance_value is not None:
+                ele["default"] = instance_value
         tool_dict = as_dict(tool)
         tool_dict["params"] = tool_params
 

backend/services/agent_service.py

@@ -1241,7 +1241,9 @@ async def update_agent_info_impl(request: AgentInfoRequest, authorization: str =
                      if inst.get("tool_id") == tool_id),
                     None
                 )
-                params = (existing_instance or {}).get("params", {})
+                # Safely get params, default to empty dict if None or not present
+                raw_params = (existing_instance or {}).get("params")
+                params = raw_params if raw_params is not None else {}
                 create_or_update_tool_by_tool_info(
                     tool_info=ToolInstanceInfoRequest(
                         tool_id=tool_id,

frontend/app/[locale]/agents/components/agentConfig/ToolManagement.tsx

@@ -143,13 +143,15 @@ export default function ToolManagement({
 
         if (tooInstance.success && tooInstance.data) {
           // Merge instance params with default params
+          // Only use instance value if it exists and is not null/undefined
           const mergedParams =
             defaultTool.initParams?.map((param: ToolParam) => {
               const instanceValue = tooInstance.data?.params?.[param.name];
+              // Use instance value only if it's not null or undefined
+              const hasValidInstanceValue = instanceValue !== null && instanceValue !== undefined;
               return {
                 ...param,
-                value:
-                  instanceValue !== undefined ? instanceValue : param.value,
+                value: hasValidInstanceValue ? instanceValue : param.value,
               };
             }) ||
             defaultTool.initParams ||

frontend/app/[locale]/agents/components/agentConfig/tool/ToolConfigModal.tsx

@@ -1313,8 +1313,18 @@ export default function ToolConfigModal({
         return;
       }
 
-      // Convert params to backend format (use the synced params)
-      const paramsObj = currentParams.reduce(
+      // Convert params to backend format - use latestFormValues directly to avoid async state issues
+      // This ensures we capture the most recent form values without relying on async setState
+      const syncedParams = [...currentParams];
+      if (latestFormValues) {
+        Object.entries(latestFormValues).forEach(([fieldName, value]) => {
+          const index = parseInt(fieldName.replace("param_", ""));
+          if (!isNaN(index) && syncedParams[index]) {
+            syncedParams[index] = { ...syncedParams[index], value };
+          }
+        });
+      }
+      const paramsObj = syncedParams.reduce(
         (acc, param) => {
           acc[param.name] = param.value;
           return acc;
@@ -1326,7 +1336,7 @@ export default function ToolConfigModal({
       // Include display_names for knowledge base tools to pass to prompt generation
       const updatedTool: typeof toolToSave = {
         ...toolToSave,
-        initParams: currentParams,
+        initParams: syncedParams,
         // Store knowledge base display names for prompt generation
         ...(toolRequiresKbSelection && selectedKbDisplayNames.length > 0
           ? { display_names: selectedKbDisplayNames }

frontend/hooks/agent/useSaveGuard.ts

@@ -40,8 +40,12 @@ async function batchUpdateToolConfigs(
   for (const tool of currentTools) {
     const toolId = parseInt(tool.id);
     const isEnabled = true; // Selected tools are always enabled
+    // Only include params that have a defined value (not undefined or null)
+    // This ensures we don't save null values from form defaults or stale data
     const params = tool.initParams?.reduce((acc: Record<string, any>, param: any) => {
-      acc[param.name] = param.value;
+      if (param.value !== undefined && param.value !== null) {
+        acc[param.name] = param.value;
+      }
       return acc;
     }, {} as Record<string, any>) || {};
 
@@ -192,14 +196,24 @@ export const useSaveGuard = () => {
         const baselineTools = useAgentConfigStore.getState().baselineAgent?.tools || [];
         await batchUpdateToolConfigs(finalAgentId, currentEditedAgent.tools || [], baselineTools);
 
-        // Common logic for both creation and update: refresh cache and update store
+        // Refresh cache
         await queryClient.invalidateQueries({
           queryKey: ["agentInfo", finalAgentId]
         });
         await queryClient.refetchQueries({
           queryKey: ["agentInfo", finalAgentId]
         });
 
+        // CRITICAL: Update store with the latest data from cache after saving tool configs
+        // This ensures that on subsequent saves, the tool initParams reflect the latest
+        // values that were saved (including any defaults merged by the backend)
+        const latestAgentData = queryClient.getQueryData(["agentInfo", finalAgentId]);
+        if (latestAgentData && typeof latestAgentData === 'object' && 'tools' in latestAgentData) {
+          const latestTools = (latestAgentData as any).tools || [];
+          // Update editedAgent with the latest tools from cache
+          useAgentConfigStore.getState().updateTools(latestTools);
+        }
+
         // Refresh skill instances after save
         await queryClient.invalidateQueries({
           queryKey: ["agentSkillInstances", finalAgentId]
@@ -208,6 +222,8 @@ export const useSaveGuard = () => {
         // Also invalidate the agents list cache to ensure the list reflects any changes
         queryClient.invalidateQueries({ queryKey: ["agents"] });
 
+        // Mark as saved (this will sync editedAgent to baselineAgent)
+        useAgentConfigStore.getState().markAsSaved();
         return true;
       } else {
         message.error(result.message || t("businessLogic.config.error.saveFailed") );

test/backend/database/test_tool_db.py

@@ -215,13 +215,16 @@ def test_create_or_update_tool_by_tool_info_update_existing(monkeypatch, mock_se
     mock_ctx.__exit__.return_value = None
     monkeypatch.setattr(
         "backend.database.tool_db.get_db_session", lambda: mock_ctx)
+    monkeypatch.setattr("backend.database.tool_db.as_dict",
+                        lambda obj: obj.__dict__ if hasattr(obj, '__dict__') else obj)
 
     tool_info = MagicMock()
     tool_info.__dict__ = {"agent_id": 1, "tool_id": 1}
 
     result = create_or_update_tool_by_tool_info(tool_info, "tenant1", "user1")
 
-    assert result == mock_tool_instance
+    # Result is now as_dict() of the tool_instance
+    assert isinstance(result, dict)
 
 
 def test_create_or_update_tool_by_tool_info_create_new(monkeypatch, mock_session):
@@ -263,6 +266,8 @@ def __init__(self, **kwargs):
 
     monkeypatch.setattr(
         "backend.database.tool_db.ToolInstance", MockToolInstanceClass)
+    monkeypatch.setattr("backend.database.tool_db.as_dict",
+                        lambda obj: obj.__dict__ if hasattr(obj, '__dict__') else obj)
 
     session.add = MagicMock()
     session.flush = MagicMock()
@@ -272,7 +277,8 @@ def __init__(self, **kwargs):
 
     result = create_or_update_tool_by_tool_info(tool_info, "tenant1", "user1")
 
-    assert isinstance(result, MockToolInstanceClass)
+    # Result is now as_dict() of the tool_instance (a dict)
+    assert isinstance(result, dict)
     session.add.assert_called_once()
     session.flush.assert_called_once()