Merge pull request #35 from dreamfactorysoftware/fix/custom-tool-save-on-create

fix: persist custom tools on service create and on re-save without ids

Author: nicdavidson

src/Models/McpCustomTool.php

@@ -85,6 +85,7 @@ public static function getAllForService(int $serviceId): array
     public static function syncToolsForService(int $serviceId, array $tools): void
     {
         $existing = static::where('service_id', $serviceId)->get()->keyBy('id');
+        $existingByName = $existing->keyBy('name');
         $receivedIds = [];
 
         foreach ($tools as $toolData) {
@@ -104,8 +105,19 @@ public static function syncToolsForService(int $serviceId, array $tools): void
                 }
             }
 
+            // Resolve the matching existing row — by id first, then by name.
+            // The name fallback matters when the UI saves a tool right after
+            // creating it: the server assigned an id, but the UI never
+            // refreshed its form state so it resends with id missing and we'd
+            // otherwise hit the (service_id, name) unique constraint.
+            $tool = null;
             if (!empty($toolData['id']) && $existing->has($toolData['id'])) {
                 $tool = $existing->get($toolData['id']);
+            } elseif (!empty($toolData['name']) && $existingByName->has($toolData['name'])) {
+                $tool = $existingByName->get($toolData['name']);
+            }
+
+            if ($tool) {
                 $tool->update($toolData);
                 if ($tool->wasChanged(['storage_service_id', 'storage_path', 'scm_repository', 'scm_reference'])) {
                     Cache::forget(self::buildScmCacheKey(

src/Models/McpServerConfig.php

@@ -55,6 +55,10 @@ public static function getConfig($id, $local_config = null, $protect = true)
 
     /**
      * Override to handle custom_tools sync on save.
+     *
+     * On initial service create, $id is null here (the service has not been
+     * inserted yet). The sync runs from storeConfig() below, which fires
+     * after the service row is created with a real id.
      */
     public static function setConfig($id, $config, $local_config = null)
     {
@@ -63,12 +67,36 @@ public static function setConfig($id, $config, $local_config = null)
 
         parent::setConfig($id, $config, $local_config);
 
-        if (is_array($customTools)) {
-            try {
-                McpCustomTool::syncToolsForService($id, $customTools);
-            } catch (\Exception $e) {
-                // Table may not exist yet if migration hasn't run
-            }
+        if ($id && is_array($customTools)) {
+            self::syncCustomTools((int) $id, $customTools);
+        }
+    }
+
+    /**
+     * Override to sync custom_tools when a new service's config is stored
+     * for the first time (post-insert, when the service id is known).
+     */
+    public static function storeConfig($id, $config)
+    {
+        $customTools = $config['custom_tools'] ?? null;
+        unset($config['custom_tools']);
+
+        parent::storeConfig($id, $config);
+
+        if ($id && is_array($customTools)) {
+            self::syncCustomTools((int) $id, $customTools);
+        }
+    }
+
+    private static function syncCustomTools(int $serviceId, array $customTools): void
+    {
+        try {
+            McpCustomTool::syncToolsForService($serviceId, $customTools);
+        } catch (\Throwable $e) {
+            \Log::error('Failed to sync MCP custom tools', [
+                'service_id' => $serviceId,
+                'error'      => $e->getMessage(),
+            ]);
         }
     }