From 8d762964b2a39562e6d5121114efb9b74e6ed60a Mon Sep 17 00:00:00 2001 From: Max Date: Wed, 3 Dec 2025 10:05:23 +0800 Subject: [PATCH 1/3] Refactor assistant capabilities retrieval and enhance API endpoints - Replaced the deprecated getConnectorCapabilities method with a unified capability getter in the Assistant model, improving capability retrieval logic. - Added a new API endpoint to retrieve essential assistant information, including fields like id, name, avatar, and connector options, enhancing the assistant's data accessibility. - Updated the LLM management to support filtering by capabilities, allowing for more flexible provider listings based on user-defined models. - Improved overall structure and clarity in the assistant's capabilities and API responses, ensuring better maintainability and usability. --- agent/assistant/agent.go | 51 +------------ agent/llm/capabilities.go | 127 ++++++++++++++++++++++++++++++ openapi/agent/agent.go | 11 +-- openapi/agent/assistant.go | 104 +++++++++++++++++++++++++ openapi/llm/llm.go | 153 ++++++++++++++++++++++++++++--------- 5 files changed, 357 insertions(+), 89 deletions(-) create mode 100644 agent/llm/capabilities.go diff --git a/agent/assistant/agent.go b/agent/assistant/agent.go index c3d9fa42..9cebdf5f 100644 --- a/agent/assistant/agent.go +++ b/agent/assistant/agent.go @@ -11,6 +11,7 @@ import ( "github.com/yaoapp/yao/agent/assistant/handlers" "github.com/yaoapp/yao/agent/context" "github.com/yaoapp/yao/agent/i18n" + "github.com/yaoapp/yao/agent/llm" "github.com/yaoapp/yao/agent/output/message" ) @@ -323,58 +324,12 @@ func (ast *Assistant) GetConnector(ctx *context.Context) (connector.Connector, * } // Get connector capabilities from settings - capabilities := ast.getConnectorCapabilities(conn) + // Uses unified capability getter: 1. User-defined models.yml, 2. connector's Setting()["capabilities"], 3. default + capabilities := llm.GetCapabilitiesFromConn(conn, modelCapabilities) return conn, capabilities, nil } -// getConnectorCapabilities get the capabilities of a connector from settings -// Priority: 1. modelCapabilities mapping, 2. connector's Setting()["capabilities"] -func (ast *Assistant) getConnectorCapabilities(conn connector.Connector) *openai.Capabilities { - if conn == nil { - return &openai.Capabilities{ - Vision: false, - ToolCalls: false, - Audio: false, - Reasoning: false, - Streaming: false, - JSON: false, - Multimodal: false, - TemperatureAdjustable: true, - } - } - - // Get connector ID - connectorID := conn.ID() - - // Priority 1: Check global modelCapabilities mapping - if modelCaps, exists := modelCapabilities[connectorID]; exists { - return &modelCaps - } - - // Priority 2: Get capabilities from connector's Setting() method - // Modern connectors (post-upgrade) provide default capabilities via Setting() - settings := conn.Setting() - if caps, ok := settings["capabilities"]; ok { - if capabilities, ok := caps.(*openai.Capabilities); ok { - return capabilities - } - } - - // Fallback: Return minimal default capabilities - // This should rarely happen with upgraded connectors - return &openai.Capabilities{ - Vision: false, - ToolCalls: false, - Audio: false, - Reasoning: false, - Streaming: false, - JSON: false, - Multimodal: false, - TemperatureAdjustable: true, // Default to true for non-reasoning models - } -} - // Info get the assistant information func (ast *Assistant) Info(locale ...string) *message.AssistantInfo { lc := "en" diff --git a/agent/llm/capabilities.go b/agent/llm/capabilities.go new file mode 100644 index 00000000..63072805 --- /dev/null +++ b/agent/llm/capabilities.go @@ -0,0 +1,127 @@ +package llm + +import ( + "github.com/yaoapp/gou/connector" + "github.com/yaoapp/gou/connector/openai" +) + +// GetCapabilities get the capabilities of a connector by connector ID +// This is a unified function to get connector capabilities with proper priority: +// 1. User-defined model capabilities from agent/models.yml (passed via modelCapabilities map) +// 2. Connector's Setting()["capabilities"] (default capabilities from connector) +// 3. Fallback to minimal default capabilities +// +// Usage in Agent with user-defined models: +// +// capabilities := llm.GetCapabilities(connectorID, modelCapabilities) +// +// Usage in API (without user-defined models): +// +// capabilities := llm.GetCapabilities(connectorID, nil) +func GetCapabilities(connectorID string, modelCapabilities map[string]openai.Capabilities) *openai.Capabilities { + if connectorID == "" { + return getDefaultCapabilities() + } + + // Priority 1: Check user-defined model capabilities from agent/models.yml + if modelCapabilities != nil { + if modelCaps, exists := modelCapabilities[connectorID]; exists { + return &modelCaps + } + } + + // Priority 2: Get connector and extract capabilities from Setting() + conn, err := connector.Select(connectorID) + if err != nil { + // If connector not found, return default + return getDefaultCapabilities() + } + + return GetCapabilitiesFromConn(conn, modelCapabilities) +} + +// GetCapabilitiesFromConn get the capabilities from a connector instance +// This is useful when you already have the connector object +func GetCapabilitiesFromConn(conn connector.Connector, modelCapabilities map[string]openai.Capabilities) *openai.Capabilities { + if conn == nil { + return getDefaultCapabilities() + } + + connectorID := conn.ID() + + // Priority 1: Check user-defined model capabilities from agent/models.yml + if modelCapabilities != nil { + if modelCaps, exists := modelCapabilities[connectorID]; exists { + return &modelCaps + } + } + + // Priority 2: Get capabilities from connector's Setting() method + settings := conn.Setting() + if settings != nil { + if caps, ok := settings["capabilities"]; ok { + // Try to convert to *openai.Capabilities + if capabilities, ok := caps.(*openai.Capabilities); ok { + return capabilities + } + // Try to convert to openai.Capabilities (value type) + if capabilities, ok := caps.(openai.Capabilities); ok { + return &capabilities + } + } + } + + // Priority 3: Fallback to minimal default capabilities + return getDefaultCapabilities() +} + +// getDefaultCapabilities returns minimal default capabilities +// This should rarely be used as modern connectors provide capabilities via Setting() +func getDefaultCapabilities() *openai.Capabilities { + return &openai.Capabilities{ + Vision: false, + ToolCalls: false, + Audio: false, + Reasoning: false, + Streaming: false, + JSON: false, + Multimodal: false, + TemperatureAdjustable: true, // Default to true for non-reasoning models + } +} + +// GetCapabilitiesMap get capabilities as map[string]interface{} for API responses +// This is useful for OpenAPI responses that need JSON-serializable format +func GetCapabilitiesMap(connectorID string, modelCapabilities map[string]openai.Capabilities) map[string]interface{} { + caps := GetCapabilities(connectorID, modelCapabilities) + if caps == nil { + return nil + } + + return ToMap(caps) +} + +// ToMap converts openai.Capabilities to map[string]interface{} +// This is useful for JSON serialization in API responses +func ToMap(caps *openai.Capabilities) map[string]interface{} { + if caps == nil { + return nil + } + + result := make(map[string]interface{}) + + // Handle Vision field specially as it can be bool or string + if caps.Vision != nil { + result["vision"] = caps.Vision + } + + result["audio"] = caps.Audio + result["tool_calls"] = caps.ToolCalls + result["reasoning"] = caps.Reasoning + result["streaming"] = caps.Streaming + result["json"] = caps.JSON + result["multimodal"] = caps.Multimodal + result["temperature_adjustable"] = caps.TemperatureAdjustable + + return result +} diff --git a/openapi/agent/agent.go b/openapi/agent/agent.go index 307f0745..cd6bf4b6 100644 --- a/openapi/agent/agent.go +++ b/openapi/agent/agent.go @@ -16,11 +16,12 @@ func Attach(group *gin.RouterGroup, oauth types.OAuth) { group.Use(oauth.Guard) // Assistant CRUD - Standard REST endpoints - group.GET("/assistants", ListAssistants) // GET /assistants - List assistants - group.POST("/assistants", CreateAssistant) // POST /assistants - Create assistant - group.GET("/assistants/tags", ListAssistantTags) // GET /assistants/tags - Get all assistant tags with permission filtering - group.GET("/assistants/:id", GetAssistant) // GET /assistants/:id - Get assistant details with permission verification - group.PUT("/assistants/:id", UpdateAssistant) // PUT /assistants/:id - Update assistant + group.GET("/assistants", ListAssistants) // GET /assistants - List assistants + group.POST("/assistants", CreateAssistant) // POST /assistants - Create assistant + group.GET("/assistants/tags", ListAssistantTags) // GET /assistants/tags - Get all assistant tags with permission filtering + group.GET("/assistants/:id", GetAssistant) // GET /assistants/:id - Get assistant details with permission verification + group.GET("/assistants/:id/info", GetAssistantInfo) // GET /assistants/:id/messages - Get assistant Information + group.PUT("/assistants/:id", UpdateAssistant) // PUT /assistants/:id - Update assistant // group.DELETE("/assistants/:id", agent.HandleAssistantDelete) // DELETE /assistants/:id - Delete assistant // Assistant Actions diff --git a/openapi/agent/assistant.go b/openapi/agent/assistant.go index ce2d10ea..d0c13487 100644 --- a/openapi/agent/assistant.go +++ b/openapi/agent/assistant.go @@ -512,6 +512,110 @@ func UpdateAssistant(c *gin.Context) { }) } +// GetAssistantInfo retrieves essential assistant information for InputArea component +// Returns only the fields needed for UI display: id, name, avatar, description, connector, connector_options, modes, default_mode +func GetAssistantInfo(c *gin.Context) { + + // Get authorized information + authInfo := authorized.GetInfo(c) + + // Get Agent instance from global variable + agentInstance := agent.GetAgent() + if agentInstance == nil || agentInstance.Store == nil { + errorResp := &response.ErrorResponse{ + Code: response.ErrServerError.Code, + ErrorDescription: "Agent store not initialized", + } + response.RespondWithError(c, response.StatusInternalServerError, errorResp) + return + } + + // Get assistant ID from URL parameter + assistantID := c.Param("id") + if assistantID == "" { + errorResp := &response.ErrorResponse{ + Code: response.ErrInvalidRequest.Code, + ErrorDescription: "assistant_id is required", + } + response.RespondWithError(c, response.StatusBadRequest, errorResp) + return + } + + // Parse locale (optional - defaults to "en-us") + locale := "en-us" + if loc := c.Query("locale"); loc != "" { + locale = strings.ToLower(strings.TrimSpace(loc)) + } + + // Define fields needed for InputArea + infoFields := []string{ + "assistant_id", + "name", + "avatar", + "description", + "connector", + "connector_options", + "modes", + "default_mode", + } + + // Get assistant with specific fields and locale + assistant, err := agentInstance.Store.GetAssistant(assistantID, infoFields, locale) + if err != nil { + log.Error("Failed to get assistant info %s: %v", assistantID, err) + errorResp := &response.ErrorResponse{ + Code: response.ErrInvalidRequest.Code, + ErrorDescription: "Assistant not found: " + err.Error(), + } + response.RespondWithError(c, response.StatusNotFound, errorResp) + return + } + + // Check read permission (same as GetAssistant) + hasPermission, err := checkAssistantPermission(authInfo, assistantID, true) + if err != nil { + log.Error("Failed to check permission for assistant %s: %v", assistantID, err) + errorResp := &response.ErrorResponse{ + Code: response.ErrServerError.Code, + ErrorDescription: "Failed to check permission: " + err.Error(), + } + response.RespondWithError(c, response.StatusForbidden, errorResp) + return + } + + if !hasPermission { + errorResp := &response.ErrorResponse{ + Code: response.ErrAccessDenied.Code, + ErrorDescription: "Forbidden: No permission to access this assistant", + } + response.RespondWithError(c, response.StatusForbidden, errorResp) + return + } + + // Build response with only the required fields + infoResponse := map[string]interface{}{ + "assistant_id": assistant.ID, + "name": assistant.Name, + "avatar": assistant.Avatar, + "description": assistant.Description, + "connector": assistant.Connector, + } + + // Add optional fields if they exist + if assistant.ConnectorOptions != nil { + infoResponse["connector_options"] = assistant.ConnectorOptions + } + if len(assistant.Modes) > 0 { + infoResponse["modes"] = assistant.Modes + } + if assistant.DefaultMode != "" { + infoResponse["default_mode"] = assistant.DefaultMode + } + + // Return the result with standard response format + response.RespondWithSuccess(c, response.StatusOK, infoResponse) +} + // checkAssistantPermission checks if the user has permission to access the assistant // Similar logic to checkCollectionPermission in openapi/kb/collection.go // readable: true for read permission, false for write permission diff --git a/openapi/llm/llm.go b/openapi/llm/llm.go index 14b55eff..30a8374a 100644 --- a/openapi/llm/llm.go +++ b/openapi/llm/llm.go @@ -1,18 +1,34 @@ package llm import ( + "strings" + "github.com/gin-gonic/gin" "github.com/yaoapp/gou/connector" + "github.com/yaoapp/gou/connector/openai" + "github.com/yaoapp/yao/agent" + agentllm "github.com/yaoapp/yao/agent/llm" oauthTypes "github.com/yaoapp/yao/openapi/oauth/types" "github.com/yaoapp/yao/openapi/response" ) // Provider represents an LLM provider option type Provider struct { - Label string `json:"label"` - Value string `json:"value"` - Type string `json:"type"` // "openai" - Builtin bool `json:"builtin"` // true for system built-in, false for user-defined + Label string `json:"label"` + Value string `json:"value"` + Type string `json:"type"` // "openai" + Builtin bool `json:"builtin"` // true for system built-in, false for user-defined + Capabilities map[string]interface{} `json:"capabilities"` // Model capabilities from connector settings +} + +// getModelCapabilities returns user-defined model capabilities from agent DSL +// Returns nil if agent not initialized or no models configured +func getModelCapabilities() map[string]openai.Capabilities { + agentDSL := agent.GetAgent() + if agentDSL != nil && agentDSL.Models != nil { + return agentDSL.Models + } + return nil } // Attach attaches the LLM management handlers to the router with OAuth protection @@ -26,52 +42,50 @@ func Attach(group *gin.RouterGroup, oauth oauthTypes.OAuth) { } // listProviders lists all available LLM providers (built-in + user-defined) +// Supports filtering by capabilities using query parameter: ?filters=vision,tool_calls,audio func listProviders(c *gin.Context) { allProviders := make([]Provider, 0) - // Track which connectors we've already added (to avoid duplicates) - added := make(map[string]bool) + // Parse filter parameters from query string + filtersParam := c.Query("filters") + var filters []string + if filtersParam != "" { + filters = strings.Split(filtersParam, ",") + for i, filter := range filters { + filters[i] = strings.TrimSpace(strings.ToLower(filter)) + } + } - // 1. Get system built-in OpenAI-compatible LLM connectors + // Get user-defined model capabilities once at the start of request + modelCapabilities := getModelCapabilities() + + // Get all OpenAI-compatible LLM connectors from AIConnectors + // Note: All openai type connectors are automatically added to AIConnectors during loading + // See gou/connector/connector.go LoadSource() for details for _, opt := range connector.AIConnectors { connType := getConnectorType(opt.Value) // Only include OpenAI-compatible LLM connectors if connType == "openai" { - allProviders = append(allProviders, Provider{ - Label: opt.Label, - Value: opt.Value, - Type: connType, - Builtin: true, - }) - added[opt.Value] = true - } - } + conn, ok := connector.Connectors[opt.Value] + if !ok { + continue + } - // 2. Get user-defined OpenAI-compatible LLM connectors from the global connector registry - // This includes all loaded connectors, both built-in and user-defined - // Only include OpenAI-compatible connectors (standard openai format) - for id, conn := range connector.Connectors { - // Skip if already added - if added[id] { - continue - } + // Get capabilities from connector settings + capabilities := getCapabilitiesWithModels(conn, modelCapabilities) - // Only include OpenAI-compatible LLM connectors - connType := getConnectorType(id) - if connType == "openai" { - meta := conn.GetMetaInfo() - label := meta.Label - if label == "" { - label = id + // Apply capability filters + if len(filters) > 0 && !matchesFilters(capabilities, filters) { + continue } allProviders = append(allProviders, Provider{ - Label: label, - Value: id, - Type: connType, - Builtin: meta.Builtin, + Label: opt.Label, + Value: opt.Value, + Type: connType, + Builtin: conn.GetMetaInfo().Builtin, + Capabilities: capabilities, }) - added[id] = true } } @@ -92,3 +106,70 @@ func getConnectorType(id string) string { return "unknown" } + +// getCapabilitiesWithModels extracts capabilities from connector settings +// Uses the unified capability getter from agent/llm package +// Takes modelCapabilities as parameter to avoid repeated calls to getModelCapabilities() +func getCapabilitiesWithModels(conn connector.Connector, modelCapabilities map[string]openai.Capabilities) map[string]interface{} { + if conn == nil { + return nil + } + + // Use unified capability getter with user-defined model capabilities + caps := agentllm.GetCapabilitiesFromConn(conn, modelCapabilities) + return agentllm.ToMap(caps) +} + +// matchesFilters checks if capabilities match all requested filters +// Filters are matched case-insensitively and support the following capability keys: +// - vision: true or string value like "openai", "claude" +// - audio: bool +// - tool_calls: bool +// - reasoning: bool +// - streaming: bool +// - json: bool +// - multimodal: bool +// - temperature_adjustable: bool +func matchesFilters(capabilities map[string]interface{}, filters []string) bool { + if capabilities == nil { + return false + } + + // All filters must match (AND logic) + for _, filter := range filters { + matched := false + + // Check each capability field + for key, value := range capabilities { + keyLower := strings.ToLower(key) + + // Match the filter against capability key + if keyLower == filter { + // For vision, check if it's true or a non-empty string + if filter == "vision" { + if boolVal, ok := value.(bool); ok && boolVal { + matched = true + break + } + if strVal, ok := value.(string); ok && strVal != "" { + matched = true + break + } + } else { + // For other capabilities, check if it's true + if boolVal, ok := value.(bool); ok && boolVal { + matched = true + break + } + } + } + } + + // If any filter doesn't match, return false + if !matched { + return false + } + } + + return true +} From bbba6b9fdb4380cff96bb721d8402543b254c5cd Mon Sep 17 00:00:00 2001 From: Max Date: Wed, 3 Dec 2025 10:50:24 +0800 Subject: [PATCH 2/3] Add modes and database support to Assistant model - Enhanced the `loadMap` function to include handling for `modes` and `default_mode` fields, allowing for flexible operational modes and a specified primary mode. - Introduced a new `DB` field in the Assistant model to support database configuration, improving data management capabilities. - Implemented the `ToModes` conversion function to facilitate various input types for modes, ensuring robust handling and validation. - Added comprehensive tests for the `ToModes` function to validate its functionality across different input scenarios, enhancing overall reliability. - Updated relevant methods to ensure consistent integration of the new fields and functionalities within the Assistant model. --- agent/assistant/load.go | 23 ++++++ agent/store/types/convert.go | 36 +++++++++ agent/store/types/convert_test.go | 119 ++++++++++++++++++++++++++++++ 3 files changed, 178 insertions(+) diff --git a/agent/assistant/load.go b/agent/assistant/load.go index c6c8d524..3a059eb0 100644 --- a/agent/assistant/load.go +++ b/agent/assistant/load.go @@ -425,6 +425,20 @@ func loadMap(data map[string]interface{}) (*Assistant, error) { assistant.Automated = v } + // modes + if v, has := data["modes"]; has { + modes, err := store.ToModes(v) + if err != nil { + return nil, err + } + assistant.Modes = modes + } + + // default_mode + if v, ok := data["default_mode"].(string); ok { + assistant.DefaultMode = v + } + // DisableGlobalPrompts if v, ok := data["disable_global_prompts"].(bool); ok { assistant.DisableGlobalPrompts = v @@ -593,6 +607,15 @@ func loadMap(data map[string]interface{}) (*Assistant, error) { assistant.KB = knowledgeBase } + // db + if db, has := data["db"]; has { + database, err := store.ToDatabase(db) + if err != nil { + return nil, err + } + assistant.DB = database + } + // mcp if mcp, has := data["mcp"]; has { mcpServers, err := store.ToMCPServers(mcp) diff --git a/agent/store/types/convert.go b/agent/store/types/convert.go index 03a3f62d..0d3b3974 100644 --- a/agent/store/types/convert.go +++ b/agent/store/types/convert.go @@ -556,6 +556,42 @@ func ToConnectorOptions(v interface{}) (*ConnectorOptions, error) { } } +// ToModes converts various types to []string for modes +func ToModes(v interface{}) ([]string, error) { + if v == nil { + return nil, nil + } + + switch modes := v.(type) { + case []string: + return modes, nil + + case []interface{}: + var result []string + for _, item := range modes { + result = append(result, cast.ToString(item)) + } + return result, nil + + case string: + // Single string becomes a slice with one element + return []string{modes}, nil + + default: + raw, err := jsoniter.Marshal(modes) + if err != nil { + return nil, fmt.Errorf("modes format error: %s", err.Error()) + } + + var result []string + err = jsoniter.Unmarshal(raw, &result) + if err != nil { + return nil, fmt.Errorf("modes format error: %s", err.Error()) + } + return result, nil + } +} + // ToPromptPresets converts various types to map[string][]Prompt func ToPromptPresets(v interface{}) (map[string][]Prompt, error) { if v == nil { diff --git a/agent/store/types/convert_test.go b/agent/store/types/convert_test.go index 800bef93..b0b8b347 100644 --- a/agent/store/types/convert_test.go +++ b/agent/store/types/convert_test.go @@ -1449,6 +1449,125 @@ func TestToConnectorOptions(t *testing.T) { }) } +// TestToModes tests the ToModes conversion function +func TestToModes(t *testing.T) { + t.Run("NilInput", func(t *testing.T) { + result, err := ToModes(nil) + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + if result != nil { + t.Errorf("Expected nil result, got: %v", result) + } + }) + + t.Run("StringSlice", func(t *testing.T) { + modes := []string{"chat", "task", "analyze"} + result, err := ToModes(modes) + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + if len(result) != 3 { + t.Errorf("Expected 3 modes, got %d", len(result)) + } + if result[0] != "chat" { + t.Errorf("Expected 'chat', got '%s'", result[0]) + } + if result[1] != "task" { + t.Errorf("Expected 'task', got '%s'", result[1]) + } + if result[2] != "analyze" { + t.Errorf("Expected 'analyze', got '%s'", result[2]) + } + }) + + t.Run("InterfaceSlice", func(t *testing.T) { + modes := []interface{}{"chat", "task", 123} + result, err := ToModes(modes) + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + if len(result) != 3 { + t.Errorf("Expected 3 modes, got %d", len(result)) + } + if result[0] != "chat" { + t.Errorf("Expected 'chat', got '%s'", result[0]) + } + if result[2] != "123" { + t.Errorf("Expected '123', got '%s'", result[2]) + } + }) + + t.Run("SingleString", func(t *testing.T) { + mode := "chat" + result, err := ToModes(mode) + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + if len(result) != 1 { + t.Errorf("Expected 1 mode, got %d", len(result)) + } + if result[0] != "chat" { + t.Errorf("Expected 'chat', got '%s'", result[0]) + } + }) + + t.Run("EmptySlice", func(t *testing.T) { + modes := []string{} + result, err := ToModes(modes) + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + if len(result) != 0 { + t.Errorf("Expected 0 modes, got %d", len(result)) + } + }) + + t.Run("InvalidInput", func(t *testing.T) { + // Test with data that can't be marshaled + invalidData := make(chan int) + _, err := ToModes(invalidData) + if err == nil { + t.Error("Expected error for invalid input") + } + }) + + t.Run("InvalidJSONUnmarshal", func(t *testing.T) { + // Test with data that marshals but can't unmarshal to []string + data := map[string]interface{}{ + "invalid": "structure", + } + _, err := ToModes(data) + if err == nil { + t.Error("Expected error for invalid unmarshal") + } + }) + + t.Run("MixedTypes", func(t *testing.T) { + modes := []interface{}{"chat", 456, "task", true} + result, err := ToModes(modes) + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + if len(result) != 4 { + t.Errorf("Expected 4 modes, got %d", len(result)) + } + // cast.ToString should convert all to strings + if result[0] != "chat" { + t.Errorf("Expected 'chat', got '%s'", result[0]) + } + if result[1] != "456" { + t.Errorf("Expected '456', got '%s'", result[1]) + } + if result[2] != "task" { + t.Errorf("Expected 'task', got '%s'", result[2]) + } + if result[3] != "true" { + t.Errorf("Expected 'true', got '%s'", result[3]) + } + }) +} + // TestToPromptPresets tests the ToPromptPresets conversion function func TestToPromptPresets(t *testing.T) { t.Run("NilInput", func(t *testing.T) { From ede240302ac94f40498698f54ccc5b1e63c73f71 Mon Sep 17 00:00:00 2001 From: Max Date: Wed, 3 Dec 2025 10:57:47 +0800 Subject: [PATCH 3/3] Update ConnectorOptions handling in tests and types - Changed the `Optional` field in `ConnectorOptions` from a boolean to a pointer to allow for nil values, enhancing flexibility in option handling. - Updated tests in `load_test.go`, `convert_test.go`, and `assistant_test.go` to reflect the new pointer type for `Optional`, ensuring proper assertions and error handling. - Added new test cases to validate behavior when `Optional` is nil or false, improving test coverage and robustness of the ConnectorOptions functionality. --- agent/assistant/load_test.go | 6 ++-- agent/store/types/convert_test.go | 52 ++++++++++++++++++++++++++----- agent/store/types/types.go | 2 +- agent/store/xun/assistant_test.go | 8 +++-- 4 files changed, 55 insertions(+), 13 deletions(-) diff --git a/agent/assistant/load_test.go b/agent/assistant/load_test.go index 48deaee2..08ec735e 100644 --- a/agent/assistant/load_test.go +++ b/agent/assistant/load_test.go @@ -73,7 +73,8 @@ func TestLoadPath(t *testing.T) { // ConnectorOptions assert.NotNil(t, assistant.ConnectorOptions) - assert.True(t, assistant.ConnectorOptions.Optional) + assert.NotNil(t, assistant.ConnectorOptions.Optional) + assert.True(t, *assistant.ConnectorOptions.Optional) assert.NotNil(t, assistant.ConnectorOptions.Connectors) assert.Contains(t, assistant.ConnectorOptions.Connectors, "gpt-4o") assert.Contains(t, assistant.ConnectorOptions.Connectors, "gpt-4o-mini") @@ -370,7 +371,8 @@ func TestUpdate(t *testing.T) { require.NoError(t, err) assert.NotNil(t, assistant.ConnectorOptions) - assert.False(t, assistant.ConnectorOptions.Optional) + assert.NotNil(t, assistant.ConnectorOptions.Optional) + assert.False(t, *assistant.ConnectorOptions.Optional) assert.Contains(t, assistant.ConnectorOptions.Connectors, "new-connector") }) diff --git a/agent/store/types/convert_test.go b/agent/store/types/convert_test.go index b0b8b347..2ba0c752 100644 --- a/agent/store/types/convert_test.go +++ b/agent/store/types/convert_test.go @@ -609,7 +609,7 @@ func TestToAssistantModel(t *testing.T) { if result.ConnectorOptions == nil { t.Error("Expected ConnectorOptions to be set") } else { - if !result.ConnectorOptions.Optional { + if result.ConnectorOptions.Optional == nil || !*result.ConnectorOptions.Optional { t.Error("Expected ConnectorOptions.Optional to be true") } if len(result.ConnectorOptions.Connectors) != 2 { @@ -849,7 +849,7 @@ func TestToAssistantModelNewFields(t *testing.T) { t.Fatal("Expected ConnectorOptions to be set") } - if !result.ConnectorOptions.Optional { + if result.ConnectorOptions.Optional == nil || !*result.ConnectorOptions.Optional { t.Error("Expected Optional to be true") } @@ -1349,8 +1349,9 @@ func TestToConnectorOptions(t *testing.T) { }) t.Run("ConnectorOptionsPointer", func(t *testing.T) { + optionalTrue := true opts := &ConnectorOptions{ - Optional: true, + Optional: &optionalTrue, Connectors: []string{"openai", "anthropic"}, Filters: []ModelCapability{CapVision, CapToolCalls}, } @@ -1364,8 +1365,9 @@ func TestToConnectorOptions(t *testing.T) { }) t.Run("ConnectorOptionsValue", func(t *testing.T) { + optionalTrue := true opts := ConnectorOptions{ - Optional: true, + Optional: &optionalTrue, Connectors: []string{"openai", "anthropic"}, Filters: []ModelCapability{CapVision, CapToolCalls}, } @@ -1373,7 +1375,7 @@ func TestToConnectorOptions(t *testing.T) { if err != nil { t.Errorf("Expected no error, got: %v", err) } - if !result.Optional { + if result.Optional == nil || !*result.Optional { t.Error("Expected Optional to be true") } if len(result.Connectors) != 2 { @@ -1394,7 +1396,7 @@ func TestToConnectorOptions(t *testing.T) { if err != nil { t.Errorf("Expected no error, got: %v", err) } - if !result.Optional { + if result.Optional == nil || !*result.Optional { t.Error("Expected Optional to be true") } if len(result.Connectors) != 3 { @@ -1413,7 +1415,7 @@ func TestToConnectorOptions(t *testing.T) { if err != nil { t.Errorf("Expected no error, got: %v", err) } - if !result.Optional { + if result.Optional == nil || !*result.Optional { t.Error("Expected Optional to be true") } if result.Connectors != nil { @@ -1424,6 +1426,42 @@ func TestToConnectorOptions(t *testing.T) { } }) + t.Run("MapInputOptionalFalse", func(t *testing.T) { + data := map[string]interface{}{ + "optional": false, + "connectors": []string{"openai"}, + "filters": []string{"vision"}, + } + result, err := ToConnectorOptions(data) + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + if result.Optional == nil { + t.Error("Expected Optional to be set") + } else if *result.Optional { + t.Error("Expected Optional to be false") + } + if len(result.Connectors) != 1 { + t.Errorf("Expected 1 connector, got %d", len(result.Connectors)) + } + }) + + t.Run("MapInputOptionalNil", func(t *testing.T) { + data := map[string]interface{}{ + "connectors": []string{"openai"}, + } + result, err := ToConnectorOptions(data) + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + if result.Optional != nil { + t.Errorf("Expected Optional to be nil (not set), got: %v", *result.Optional) + } + if len(result.Connectors) != 1 { + t.Errorf("Expected 1 connector, got %d", len(result.Connectors)) + } + }) + t.Run("InvalidInput", func(t *testing.T) { // Test with data that can't be marshaled invalidData := make(chan int) diff --git a/agent/store/types/types.go b/agent/store/types/types.go index c398219d..85f4dbf8 100644 --- a/agent/store/types/types.go +++ b/agent/store/types/types.go @@ -245,7 +245,7 @@ const ( // ConnectorOptions the connector selection options // Allows defining optional connector selection with filtering capabilities type ConnectorOptions struct { - Optional bool `json:"optional,omitempty"` // Whether connector is optional for user selection + Optional *bool `json:"optional"` // Whether connector is optional for user selection (nil=default, false=hidden, true=shown) Connectors []string `json:"connectors,omitempty"` // List of available connectors, empty means all connectors are available Filters []ModelCapability `json:"filters,omitempty"` // Filter by model capabilities, conditions can be stacked } diff --git a/agent/store/xun/assistant_test.go b/agent/store/xun/assistant_test.go index dc3350c6..042785c5 100644 --- a/agent/store/xun/assistant_test.go +++ b/agent/store/xun/assistant_test.go @@ -454,13 +454,14 @@ func TestSaveAssistant(t *testing.T) { t.Run("ConnectorOptions", func(t *testing.T) { // Test assistant with connector options + optionalTrue := true assistant := &types.AssistantModel{ Name: "Connector Options Test", Type: "assistant", Connector: "openai", Share: "private", ConnectorOptions: &types.ConnectorOptions{ - Optional: true, + Optional: &optionalTrue, Connectors: []string{"openai", "anthropic"}, Filters: []types.ModelCapability{types.CapVision, types.CapToolCalls}, }, @@ -481,7 +482,7 @@ func TestSaveAssistant(t *testing.T) { t.Fatal("Expected connector options to be set") } - if !retrieved.ConnectorOptions.Optional { + if retrieved.ConnectorOptions.Optional == nil || !*retrieved.ConnectorOptions.Optional { t.Error("Expected optional to be true") } @@ -596,13 +597,14 @@ func TestSaveAssistant(t *testing.T) { t.Run("AllNewFieldsTogether", func(t *testing.T) { // Test assistant with all new fields together + optionalFalse := false assistant := &types.AssistantModel{ Name: "All New Fields Test", Type: "assistant", Connector: "openai", Share: "private", ConnectorOptions: &types.ConnectorOptions{ - Optional: false, + Optional: &optionalFalse, Connectors: []string{"openai"}, Filters: []types.ModelCapability{types.CapVision}, },