feat: agents.json refactor (#221)

apps/backend/internal/agent/settings/controller/controller.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/kandev/kandev/internal/agent/agents"
 	"github.com/kandev/kandev/internal/agent/discovery"
 	"github.com/kandev/kandev/internal/agent/mcpconfig"
 	"github.com/kandev/kandev/internal/agent/registry"
@@ -19,6 +20,20 @@ import (
 	"go.uber.org/zap"
 )
 
+// buildCommandString builds a display-friendly command string with proper quoting.
+func buildCommandString(cmd []string) string {
+	var parts []string
+	for _, arg := range cmd {
+		if strings.ContainsAny(arg, " \t\n\"'`$\\") {
+			escaped := strings.ReplaceAll(arg, "\"", "\\\"")
+			parts = append(parts, "\""+escaped+"\"")
+		} else {
+			parts = append(parts, arg)
+		}
+	}
+	return strings.Join(parts, " ")
+}
+
 var (
 	ErrAgentNotFound        = errors.New("agent not found")
 	ErrAgentProfileNotFound = errors.New("agent profile not found")
@@ -35,7 +50,7 @@ type Controller struct {
 	agentRegistry  *registry.Registry
 	sessionChecker SessionChecker
 	mcpService     *mcpconfig.Service
-	modelFetcher   *modelfetcher.Fetcher
+	modelCache     *modelfetcher.Cache
 	logger         *logger.Logger
 }
 
@@ -51,7 +66,7 @@ func NewController(repo store.Repository, discoveryRegistry *discovery.Registry,
 		agentRegistry:  agentRegistry,
 		sessionChecker: sessionChecker,
 		mcpService:     mcpconfig.NewService(repo),
-		modelFetcher:   modelfetcher.NewFetcher(agentRegistry, log),
+		modelCache:     modelfetcher.NewCache(),
 		logger:         log.WithFields(zap.String("component", "agent-settings-controller")),
 	}
 }
@@ -145,81 +160,95 @@ func (c *Controller) ListAvailableAgents(ctx context.Context) (*dto.ListAvailabl
 	for _, result := range results {
 		availabilityByName[result.Name] = result
 	}
-	definitions := c.discovery.Definitions()
+
+	enabled := c.agentRegistry.ListEnabled()
 	now := time.Now().UTC()
-	payload := make([]dto.AvailableAgentDTO, 0, len(definitions))
-	for _, def := range definitions {
-		availability, ok := availabilityByName[def.Name]
+	payload := make([]dto.AvailableAgentDTO, 0, len(enabled))
+	for _, ag := range enabled {
+		availability, ok := availabilityByName[ag.ID()]
 		if !ok {
 			availability = discovery.Availability{
-				Name:          def.Name,
-				SupportsMCP:   def.SupportsMCP,
-				MCPConfigPath: "",
-				Available:     false,
+				Name:      ag.ID(),
+				Available: false,
 			}
 		}
 
-		displayName := def.DisplayName
+		displayName := ag.DisplayName()
 		if displayName == "" {
-			displayName = def.Name
+			displayName = ag.Name()
 		}
-		// Convert model entries
-		modelEntries := make([]dto.ModelEntryDTO, 0, len(def.ModelConfig.AvailableModels))
-		for _, model := range def.ModelConfig.AvailableModels {
-			modelEntries = append(modelEntries, dto.ModelEntryDTO{
-				ID:            model.ID,
-				Name:          model.Name,
-				Provider:      model.Provider,
-				ContextWindow: model.ContextWindow,
-				IsDefault:     model.IsDefault,
-			})
+
+		// Get models from the agent
+		var modelEntries []dto.ModelEntryDTO
+		var supportsDynamic bool
+		modelList, err := ag.ListModels(ctx)
+		if err == nil && modelList != nil {
+			supportsDynamic = modelList.SupportsDynamic
+			modelEntries = make([]dto.ModelEntryDTO, 0, len(modelList.Models))
+			for _, model := range modelList.Models {
+				modelEntries = append(modelEntries, dto.ModelEntryDTO{
+					ID:            model.ID,
+					Name:          model.Name,
+					Provider:      model.Provider,
+					ContextWindow: model.ContextWindow,
+					IsDefault:     model.IsDefault,
+				})
+			}
+		}
+
+		// Get discovery result for capabilities
+		disc, discErr := ag.IsInstalled(ctx)
+		var capabilities dto.AgentCapabilitiesDTO
+		if discErr == nil && disc != nil {
+			capabilities = dto.AgentCapabilitiesDTO{
+				SupportsSessionResume: disc.Capabilities.SupportsSessionResume,
+				SupportsShell:         disc.Capabilities.SupportsShell,
+				SupportsWorkspaceOnly: disc.Capabilities.SupportsWorkspaceOnly,
+			}
 		}
 
 		// Convert permission settings
 		var permissionSettings map[string]dto.PermissionSettingDTO
-		var passthroughConfig *dto.PassthroughConfigDTO
-		if agentConfig, ok := c.agentRegistry.Get(def.Name); ok {
-			if agentConfig.PermissionSettings != nil {
-				permissionSettings = make(map[string]dto.PermissionSettingDTO, len(agentConfig.PermissionSettings))
-				for key, setting := range agentConfig.PermissionSettings {
-					permissionSettings[key] = dto.PermissionSettingDTO{
-						Supported:    setting.Supported,
-						Default:      setting.Default,
-						Label:        setting.Label,
-						Description:  setting.Description,
-						ApplyMethod:  setting.ApplyMethod,
-						CLIFlag:      setting.CLIFlag,
-						CLIFlagValue: setting.CLIFlagValue,
-					}
-				}
-			}
-			// Convert passthrough config
-			if agentConfig.PassthroughConfig.Supported {
-				passthroughConfig = &dto.PassthroughConfigDTO{
-					Supported:   agentConfig.PassthroughConfig.Supported,
-					Label:       agentConfig.PassthroughConfig.Label,
-					Description: agentConfig.PassthroughConfig.Description,
+		permSettings := ag.PermissionSettings()
+		if permSettings != nil {
+			permissionSettings = make(map[string]dto.PermissionSettingDTO, len(permSettings))
+			for key, setting := range permSettings {
+				permissionSettings[key] = dto.PermissionSettingDTO{
+					Supported:    setting.Supported,
+					Default:      setting.Default,
+					Label:        setting.Label,
+					Description:  setting.Description,
+					ApplyMethod:  setting.ApplyMethod,
+					CLIFlag:      setting.CLIFlag,
+					CLIFlagValue: setting.CLIFlagValue,
 				}
 			}
 		}
 
+		// Convert passthrough config
+		var passthroughConfig *dto.PassthroughConfigDTO
+		if ptAgent, ok := ag.(agents.PassthroughAgent); ok {
+			pt := ptAgent.PassthroughConfig()
+			passthroughConfig = &dto.PassthroughConfigDTO{
+				Supported:   pt.Supported,
+				Label:       pt.Label,
+				Description: pt.Description,
+			}
+		}
+
 		payload = append(payload, dto.AvailableAgentDTO{
-			Name:              availability.Name,
+			Name:              ag.ID(),
 			DisplayName:       displayName,
 			SupportsMCP:       availability.SupportsMCP,
 			MCPConfigPath:     availability.MCPConfigPath,
 			InstallationPaths: availability.InstallationPaths,
 			Available:         availability.Available,
 			MatchedPath:       availability.MatchedPath,
-			Capabilities: dto.AgentCapabilitiesDTO{
-				SupportsSessionResume: def.Capabilities.SupportsSessionResume,
-				SupportsShell:         def.Capabilities.SupportsShell,
-				SupportsWorkspaceOnly: def.Capabilities.SupportsWorkspaceOnly,
-			},
+			Capabilities:      capabilities,
 			ModelConfig: dto.ModelConfigDTO{
-				DefaultModel:          def.ModelConfig.DefaultModel,
+				DefaultModel:          ag.DefaultModel(),
 				AvailableModels:       modelEntries,
-				SupportsDynamicModels: len(def.ModelConfig.DynamicModelsCmd) > 0,
+				SupportsDynamicModels: supportsDynamic,
 			},
 			PermissionSettings: permissionSettings,
 			PassthroughConfig:  passthroughConfig,
@@ -250,18 +279,23 @@ func (c *Controller) EnsureInitialAgentProfiles(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
-	displayNameByAgent := c.displayNameByAgent()
-	defaultModelByAgent := c.defaultModelByAgent()
 	for _, result := range results {
 		if !result.Available {
 			continue
 		}
-		displayName, ok := displayNameByAgent[result.Name]
-		if !ok || displayName == "" {
+		agentConfig, ok := c.agentRegistry.Get(result.Name)
+		if !ok {
+			return fmt.Errorf("unknown agent: %s", result.Name)
+		}
+		displayName := agentConfig.DisplayName()
+		if displayName == "" {
+			displayName = agentConfig.Name()
+		}
+		if displayName == "" {
 			return fmt.Errorf("unknown agent display name: %s", result.Name)
 		}
-		defaultModel, ok := defaultModelByAgent[result.Name]
-		if !ok || defaultModel == "" {
+		defaultModel := agentConfig.DefaultModel()
+		if defaultModel == "" {
 			return fmt.Errorf("unknown agent default model: %s", result.Name)
 		}
 		agent, err := c.repo.GetAgentByName(ctx, result.Name)
@@ -300,16 +334,17 @@ func (c *Controller) EnsureInitialAgentProfiles(ctx context.Context) error {
 		if len(profiles) > 0 {
 			continue
 		}
-		// Get agent config to read permission settings defaults
+		// Read permission settings defaults from agent config
 		var autoApprove, allowIndexing, skipPermissions bool
-		if agentConfig, ok := c.agentRegistry.Get(agent.Name); ok && agentConfig.PermissionSettings != nil {
-			if setting, exists := agentConfig.PermissionSettings["auto_approve"]; exists {
+		permSettings := agentConfig.PermissionSettings()
+		if permSettings != nil {
+			if setting, exists := permSettings["auto_approve"]; exists {
 				autoApprove = setting.Default
 			}
-			if setting, exists := agentConfig.PermissionSettings["allow_indexing"]; exists {
+			if setting, exists := permSettings["allow_indexing"]; exists {
 				allowIndexing = setting.Default
 			}
-			if setting, exists := agentConfig.PermissionSettings["dangerously_skip_permissions"]; exists {
+			if setting, exists := permSettings["dangerously_skip_permissions"]; exists {
 				skipPermissions = setting.Default
 			}
 		}
@@ -435,7 +470,14 @@ func (c *Controller) CreateAgent(ctx context.Context, req CreateAgentRequest) (*
 	if !matched.Available {
 		return nil, fmt.Errorf("agent not installed: %s", req.Name)
 	}
-	displayName := c.mustDisplayName(req.Name)
+	agentConfig, agOk := c.agentRegistry.Get(req.Name)
+	if !agOk {
+		return nil, fmt.Errorf("unknown agent: %s", req.Name)
+	}
+	displayName := agentConfig.DisplayName()
+	if displayName == "" {
+		displayName = agentConfig.Name()
+	}
 	if displayName == "" {
 		return nil, fmt.Errorf("unknown agent display name: %s", req.Name)
 	}
@@ -528,7 +570,14 @@ func (c *Controller) CreateProfile(ctx context.Context, req CreateProfileRequest
 	if err != nil {
 		return nil, err
 	}
-	displayName := c.mustDisplayName(agent.Name)
+	agentConfig, agOk := c.agentRegistry.Get(agent.Name)
+	if !agOk {
+		return nil, fmt.Errorf("unknown agent: %s", agent.Name)
+	}
+	displayName := agentConfig.DisplayName()
+	if displayName == "" {
+		displayName = agentConfig.Name()
+	}
 	if displayName == "" {
 		return nil, fmt.Errorf("unknown agent display name: %s", agent.Name)
 	}
@@ -653,43 +702,6 @@ func toProfileDTO(profile *models.AgentProfile) dto.AgentProfileDTO {
 	}
 }
 
-func (c *Controller) displayNameByAgent() map[string]string {
-	definitions := c.discovery.Definitions()
-	mapped := make(map[string]string, len(definitions))
-	for _, def := range definitions {
-		if def.Name == "" || def.DisplayName == "" {
-			continue
-		}
-		mapped[def.Name] = def.DisplayName
-	}
-	return mapped
-}
-
-func (c *Controller) mustDisplayName(agentName string) string {
-	if agentName == "" {
-		return ""
-	}
-	definitions := c.discovery.Definitions()
-	for _, def := range definitions {
-		if def.Name == agentName {
-			return def.DisplayName
-		}
-	}
-	return ""
-}
-
-func (c *Controller) defaultModelByAgent() map[string]string {
-	definitions := c.discovery.Definitions()
-	mapped := make(map[string]string, len(definitions))
-	for _, def := range definitions {
-		if def.Name == "" || def.ModelConfig.DefaultModel == "" {
-			continue
-		}
-		mapped[def.Name] = def.ModelConfig.DefaultModel
-	}
-	return mapped
-}
-
 // detectAgents runs discovery and forces mock-agent available when enabled.
 func (c *Controller) detectAgents(ctx context.Context) ([]discovery.Availability, error) {
 	results, err := c.discovery.Detect(ctx)
@@ -698,7 +710,7 @@ func (c *Controller) detectAgents(ctx context.Context) ([]discovery.Availability
 	}
 	// Force mock-agent as available when enabled (skip file-presence discovery)
 	agentConfig, ok := c.agentRegistry.Get("mock-agent")
-	if ok && agentConfig.Enabled {
+	if ok && agentConfig.Enabled() {
 		for i := range results {
 			if results[i].Name == "mock-agent" {
 				results[i].Available = true
@@ -718,148 +730,101 @@ type CommandPreviewRequest struct {
 
 // PreviewAgentCommand generates a preview of the CLI command that will be executed
 func (c *Controller) PreviewAgentCommand(ctx context.Context, agentName string, req CommandPreviewRequest) (*dto.CommandPreviewResponse, error) {
-	// Get agent config from registry
 	agentConfig, ok := c.agentRegistry.Get(agentName)
 	if !ok {
 		return nil, fmt.Errorf("agent type %q not found in registry", agentName)
 	}
 
-	// Build the command based on whether passthrough is supported AND enabled
-	var cmd []string
-	if agentConfig.PassthroughConfig.Supported && req.CLIPassthrough {
-		cmd = c.buildPassthroughCommandPreview(agentConfig, req)
+	var cmd agents.Command
+	if ptAgent, ok := agentConfig.(agents.PassthroughAgent); ok && req.CLIPassthrough {
+		cmd = ptAgent.BuildPassthroughCommand(agents.PassthroughOptions{
+			Model:            req.Model,
+			Prompt:           "{prompt}",
+			PermissionValues: req.PermissionSettings,
+		})
 	} else {
-		cmd = c.buildStandardCommandPreview(agentConfig, req)
+		cmd = agentConfig.BuildCommand(agents.CommandOptions{
+			Model:            req.Model,
+			PermissionValues: req.PermissionSettings,
+		})
 	}
 
-	// Build the command string with proper quoting for display
-	cmdString := c.buildCommandString(cmd)
-
 	return &dto.CommandPreviewResponse{
 		Supported:     true,
-		Command:       cmd,
-		CommandString: cmdString,
+		Command:       cmd.Args(),
+		CommandString: buildCommandString(cmd.Args()),
 	}, nil
 }
 
-// buildStandardCommandPreview builds the standard command (non-passthrough) for preview purposes
-func (c *Controller) buildStandardCommandPreview(agentConfig *registry.AgentTypeConfig, req CommandPreviewRequest) []string {
-	// Start with base command from config
-	cmd := make([]string, len(agentConfig.Cmd))
-	copy(cmd, agentConfig.Cmd)
-
-	// Apply model flag if configured and model is set
-	if req.Model != "" && agentConfig.ModelFlag != "" {
-		expanded := strings.ReplaceAll(agentConfig.ModelFlag, "{model}", req.Model)
-		parts := strings.SplitN(expanded, " ", 2)
-		cmd = append(cmd, parts...)
-	}
-
-	// Apply permission settings that use CLI flags
-	cmd = c.applyPermissionFlags(cmd, agentConfig, req.PermissionSettings)
-
-	return cmd
-}
-
-// buildPassthroughCommandPreview builds the passthrough command for preview purposes
-func (c *Controller) buildPassthroughCommandPreview(agentConfig *registry.AgentTypeConfig, req CommandPreviewRequest) []string {
-	// Start with passthrough_cmd
-	cmd := make([]string, len(agentConfig.PassthroughConfig.PassthroughCmd))
-	copy(cmd, agentConfig.PassthroughConfig.PassthroughCmd)
-
-	// Apply model flag if configured and model is set
-	if req.Model != "" && agentConfig.PassthroughConfig.ModelFlag != "" {
-		expanded := strings.ReplaceAll(agentConfig.PassthroughConfig.ModelFlag, "{model}", req.Model)
-		// Split on first space to separate flag from value (if combined)
-		parts := strings.SplitN(expanded, " ", 2)
-		cmd = append(cmd, parts...)
-	}
-
-	// Apply permission settings that use CLI flags
-	cmd = c.applyPermissionFlags(cmd, agentConfig, req.PermissionSettings)
-
-	// Add prompt - use PromptFlag if configured, otherwise append directly
-	cmd = c.appendPromptPlaceholder(cmd, agentConfig.PassthroughConfig.PromptFlag)
-
-	return cmd
-}
-
-// appendPromptPlaceholder adds the {prompt} placeholder to the command
-// If promptFlag is set (e.g., "--prompt {prompt}"), it uses that format
-// Otherwise, it appends {prompt} directly at the end
-func (c *Controller) appendPromptPlaceholder(cmd []string, promptFlag string) []string {
-	if promptFlag != "" {
-		// Use the configured prompt flag format, e.g., "--prompt {prompt}"
-		parts := strings.SplitN(promptFlag, " ", 2)
-		return append(cmd, parts...)
-	}
-	// Default: append prompt directly at the end
-	return append(cmd, "{prompt}")
-}
-
-// applyPermissionFlags applies CLI flags for permission settings that are enabled
-func (c *Controller) applyPermissionFlags(cmd []string, agentConfig *registry.AgentTypeConfig, permissionValues map[string]bool) []string {
-	if agentConfig.PermissionSettings == nil || permissionValues == nil {
-		return cmd
-	}
-
-	for settingName, setting := range agentConfig.PermissionSettings {
-		// Skip if not supported or not a CLI flag setting
-		if !setting.Supported || setting.ApplyMethod != "cli_flag" || setting.CLIFlag == "" {
-			continue
-		}
-
-		// Get the value for this setting from the request
-		value, exists := permissionValues[settingName]
-		if !exists || !value {
-			continue
-		}
-
-		// Apply the CLI flag
-		if setting.CLIFlagValue != "" {
-			// Flag with value: "--flag value"
-			cmd = append(cmd, setting.CLIFlag, setting.CLIFlagValue)
-		} else {
-			// Boolean flag or multiple flags: "--flag" or "--flag1 --flag2 arg"
-			// Split on spaces to handle multiple flags in one setting
-			parts := strings.Fields(setting.CLIFlag)
-			cmd = append(cmd, parts...)
-		}
-	}
-
-	return cmd
-}
-
-// buildCommandString builds a display-friendly command string with proper quoting.
-// Note: We intentionally don't use strconv.Quote here because it produces Go string
-// syntax (e.g., "\t" becomes "\\t") which is not ideal for shell command display.
-// The manual quoting approach produces more readable shell-style output.
-func (c *Controller) buildCommandString(cmd []string) string {
-	var parts []string
-	for _, arg := range cmd {
-		// Quote arguments that contain spaces or special characters
-		if strings.ContainsAny(arg, " \t\n\"'`$\\") {
-			// Use double quotes and escape internal double quotes
-			escaped := strings.ReplaceAll(arg, "\"", "\\\"")
-			parts = append(parts, "\""+escaped+"\"")
-		} else {
-			parts = append(parts, arg)
-		}
-	}
-	return strings.Join(parts, " ")
-}
-
 // FetchDynamicModels fetches models for an agent, optionally refreshing the cache
 func (c *Controller) FetchDynamicModels(ctx context.Context, agentName string, refresh bool) (*dto.DynamicModelsResponse, error) {
-	result, err := c.modelFetcher.Fetch(ctx, agentName, refresh)
-	if err != nil {
-		return nil, err
+	ag, ok := c.agentRegistry.Get(agentName)
+	if !ok {
+		return nil, fmt.Errorf("agent %q not found", agentName)
 	}
 
-	// Convert to DTOs
-	modelDTOs := make([]dto.ModelEntryDTO, 0, len(result.Models))
-	for _, m := range result.Models {
-		modelDTOs = append(modelDTOs, dto.ModelEntryDTO{
+	// Check cache unless refresh is requested
+	if !refresh {
+		if entry, exists := c.modelCache.Get(agentName); exists && entry.IsValid() {
+			cachedAt := entry.CachedAt
+			modelDTOs := modelsToDTO(entry.Models)
+			var errStr *string
+			if entry.Error != nil {
+				s := entry.Error.Error()
+				errStr = &s
+			}
+			return &dto.DynamicModelsResponse{
+				AgentName: agentName,
+				Models:    modelDTOs,
+				Cached:    true,
+				CachedAt:  &cachedAt,
+				Error:     errStr,
+			}, nil
+		}
+	}
+
+	// Fetch models from the agent
+	modelList, err := ag.ListModels(ctx)
+	if err != nil {
+		c.logger.Warn("model fetch failed",
+			zap.String("agent", agentName),
+			zap.Error(err))
+		c.modelCache.Set(agentName, nil, err)
+		s := err.Error()
+		return &dto.DynamicModelsResponse{
+			AgentName: agentName,
+			Models:    nil,
+			Cached:    false,
+			Error:     &s,
+		}, nil
+	}
+
+	models := modelList.Models
+
+	// Cache the result if dynamic models are supported
+	if modelList.SupportsDynamic {
+		c.modelCache.Set(agentName, models, nil)
+		cachedAt := time.Now()
+		return &dto.DynamicModelsResponse{
+			AgentName: agentName,
+			Models:    modelsToDTO(models),
+			Cached:    true,
+			CachedAt:  &cachedAt,
+		}, nil
+	}
+
+	return &dto.DynamicModelsResponse{
+		AgentName: agentName,
+		Models:    modelsToDTO(models),
+		Cached:    false,
+	}, nil
+}
+
+// modelsToDTO converts agent models to DTOs.
+func modelsToDTO(models []agents.Model) []dto.ModelEntryDTO {
+	dtos := make([]dto.ModelEntryDTO, 0, len(models))
+	for _, m := range models {
+		dtos = append(dtos, dto.ModelEntryDTO{
 			ID:            m.ID,
 			Name:          m.Name,
 			Provider:      m.Provider,
@@ -868,19 +833,5 @@ func (c *Controller) FetchDynamicModels(ctx context.Context, agentName string, r
 			Source:        m.Source,
 		})
 	}
-
-	// Convert error to string pointer
-	var errStr *string
-	if result.Error != nil {
-		s := result.Error.Error()
-		errStr = &s
-	}
-
-	return &dto.DynamicModelsResponse{
-		AgentName: result.AgentName,
-		Models:    modelDTOs,
-		Cached:    result.Cached,
-		CachedAt:  result.CachedAt,
-		Error:     errStr,
-	}, nil
+	return dtos
 }