From b5d393b4bcfcc804b772d61f93b995d2077cc762 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Fri, 30 May 2025 17:12:24 +1000
Subject: [PATCH] FIX: custom tools incorrectly setting all fields to blank
 enum (#1385)

Previous to this change, enum was set to [] which broke all non
enum tools
---
 app/models/ai_tool.rb                         | 26 ++++++++++++++
 .../components/ai-tool-editor-form.gjs        | 15 +++++---
 .../admin/ai_tools_controller_spec.rb         | 34 +++++++++++++++++++
 3 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/app/models/ai_tool.rb b/app/models/ai_tool.rb
index ec0d3ec6..27fbd9c6 100644
--- a/app/models/ai_tool.rb
+++ b/app/models/ai_tool.rb
@@ -22,6 +22,8 @@ class AiTool < ActiveRecord::Base
               message: I18n.t("discourse_ai.tools.name.characters"),
             }
 
+  validate :validate_parameters_enum
+
   def signature
     {
       name: function_call_name,
@@ -57,6 +59,30 @@ class AiTool < ActiveRecord::Base
     end
   end
 
+  def validate_parameters_enum
+    return unless parameters.is_a?(Array)
+
+    parameters.each_with_index do |param, index|
+      next if !param.is_a?(Hash) || !param.key?("enum")
+      enum_values = param["enum"]
+
+      if enum_values.empty?
+        errors.add(
+          :parameters,
+          "Parameter '#{param["name"]}' at index #{index}: enum cannot be empty",
+        )
+        next
+      end
+
+      if enum_values.uniq.length != enum_values.length
+        errors.add(
+          :parameters,
+          "Parameter '#{param["name"]}' at index #{index}: enum values must be unique",
+        )
+      end
+    end
+  end
+
   def self.preamble
     <<~JS
      /**
diff --git a/assets/javascripts/discourse/components/ai-tool-editor-form.gjs b/assets/javascripts/discourse/components/ai-tool-editor-form.gjs
index a2c0137b..e7b632fe 100644
--- a/assets/javascripts/discourse/components/ai-tool-editor-form.gjs
+++ b/assets/javascripts/discourse/components/ai-tool-editor-form.gjs
@@ -30,11 +30,16 @@ export default class AiToolEditorForm extends Component {
 
   get formData() {
     const parameters = (this.args.editingModel.parameters ?? []).map(
-      (parameter) => ({
-        ...parameter,
-        isEnum: !!parameter.enum?.length,
-        enum: (parameter.enum ??= []),
-      })
+      (parameter) => {
+        const mappedParameter = {
+          ...parameter,
+        };
+        parameter.isEnum = parameter.enum && parameter.enum.length > 0;
+        if (!parameter.isEnum) {
+          delete mappedParameter.enum;
+        }
+        return mappedParameter;
+      }
     );
 
     return {
diff --git a/spec/requests/admin/ai_tools_controller_spec.rb b/spec/requests/admin/ai_tools_controller_spec.rb
index fe5a9d4c..b96e5361 100644
--- a/spec/requests/admin/ai_tools_controller_spec.rb
+++ b/spec/requests/admin/ai_tools_controller_spec.rb
@@ -92,6 +92,40 @@ RSpec.describe DiscourseAi::Admin::AiToolsController do
         )
       end
     end
+
+    context "when enum validation fails" do
+      it "fails to create tool with empty enum" do
+        attrs = valid_attributes
+        attrs[:parameters] = [attrs[:parameters].first.merge(enum: [])]
+
+        expect {
+          post "/admin/plugins/discourse-ai/ai-tools.json",
+               params: { ai_tool: attrs }.to_json,
+               headers: {
+                 "CONTENT_TYPE" => "application/json",
+               }
+        }.not_to change(AiTool, :count)
+
+        expect(response).to have_http_status(:unprocessable_entity)
+        expect(response.parsed_body["errors"]).to include(match(/enum cannot be empty/))
+      end
+
+      it "fails to create tool with duplicate enum values" do
+        attrs = valid_attributes
+        attrs[:parameters] = [attrs[:parameters].first.merge(enum: %w[c f c])]
+
+        expect {
+          post "/admin/plugins/discourse-ai/ai-tools.json",
+               params: { ai_tool: attrs }.to_json,
+               headers: {
+                 "CONTENT_TYPE" => "application/json",
+               }
+        }.not_to change(AiTool, :count)
+
+        expect(response).to have_http_status(:unprocessable_entity)
+        expect(response.parsed_body["errors"]).to include(match(/enum values must be unique/))
+      end
+    end
   end
 
   describe "PUT #update" do