diff --git a/apps/frontend/src/renderer/components/AgentTools.tsx b/apps/frontend/src/renderer/components/AgentTools.tsx index 69ad85d3..9dcf22d2 100644 --- a/apps/frontend/src/renderer/components/AgentTools.tsx +++ b/apps/frontend/src/renderer/components/AgentTools.tsx @@ -52,13 +52,15 @@ import type { ProjectEnvConfig, AgentMcpOverrides, AgentMcpOverride, CustomMcpSe import { CustomMcpDialog } from './CustomMcpDialog'; import { useTranslation } from 'react-i18next'; import { - DEFAULT_PHASE_MODELS, - DEFAULT_PHASE_THINKING, - DEFAULT_FEATURE_MODELS, - DEFAULT_FEATURE_THINKING, AVAILABLE_MODELS, - THINKING_LEVELS + THINKING_LEVELS, } from '../../shared/constants/models'; +import { + useResolvedAgentSettings, + resolveAgentSettings as resolveAgentModelConfig, + type AgentSettingsSource, + type ResolvedAgentSettings, +} from '../hooks'; import type { ModelTypeShort, ThinkingLevel } from '../../shared/types/settings'; // Agent configuration data - mirrors AGENT_CONFIGS from backend @@ -71,17 +73,7 @@ interface AgentConfig { mcp_servers: string[]; mcp_optional?: string[]; // Maps to settings source - either a phase or a feature - settingsSource: { - type: 'phase'; - phase: 'spec' | 'planning' | 'coding' | 'qa'; - } | { - type: 'feature'; - feature: 'insights' | 'ideation' | 'roadmap' | 'githubIssues' | 'githubPrs' | 'utility'; - } | { - type: 'fixed'; // For agents not yet configurable - model: ModelTypeShort; - thinking: ThinkingLevel; - }; + settingsSource: AgentSettingsSource; } // Helper to get model label from short name @@ -971,11 +963,9 @@ export function AgentTools() { } }, []); - // Get phase and feature settings with defaults - const phaseModels = settings.customPhaseModels || DEFAULT_PHASE_MODELS; - const phaseThinking = settings.customPhaseThinking || DEFAULT_PHASE_THINKING; - const featureModels = settings.featureModels || DEFAULT_FEATURE_MODELS; - const featureThinking = settings.featureThinking || DEFAULT_FEATURE_THINKING; + // Resolve agent settings using the centralized utility + // Resolution order: custom overrides -> selected profile's config -> global defaults + const { phaseModels, phaseThinking, featureModels, featureThinking } = useResolvedAgentSettings(settings); // Get MCP server states for display const mcpServers = envConfig?.mcpServers || {}; @@ -991,27 +981,9 @@ export function AgentTools() { ].filter(Boolean).length; // Resolve model and thinking for an agent based on its settings source - const resolveAgentSettings = useMemo(() => { + const getAgentModelConfig = useMemo(() => { return (config: AgentConfig): { model: ModelTypeShort; thinking: ThinkingLevel } => { - const source = config.settingsSource; - - if (source.type === 'phase') { - return { - model: phaseModels[source.phase], - thinking: phaseThinking[source.phase], - }; - } else if (source.type === 'feature') { - return { - model: featureModels[source.feature], - thinking: featureThinking[source.feature], - }; - } else { - // Fixed settings - return { - model: source.model, - thinking: source.thinking, - }; - } + return resolveAgentModelConfig(config.settingsSource, { phaseModels, phaseThinking, featureModels, featureThinking }); }; }, [phaseModels, phaseThinking, featureModels, featureThinking]); @@ -1371,7 +1343,7 @@ export function AgentTools() { {isExpanded && (
{agents.map(({ id, config }) => { - const { model, thinking } = resolveAgentSettings(config); + const { model, thinking } = getAgentModelConfig(config); return ( { + describe('Profile Selection', () => { + it('should find auto profile by ID', () => { + const profile = DEFAULT_AGENT_PROFILES.find(p => p.id === 'auto'); + expect(profile).toBeDefined(); + expect(profile?.id).toBe('auto'); + expect(profile?.name).toBe('Auto (Optimized)'); + expect(profile?.model).toBe('opus'); + }); + + it('should find complex profile by ID', () => { + const profile = DEFAULT_AGENT_PROFILES.find(p => p.id === 'complex'); + expect(profile).toBeDefined(); + expect(profile?.id).toBe('complex'); + expect(profile?.name).toBe('Complex Tasks'); + expect(profile?.model).toBe('opus'); + }); + + it('should find balanced profile by ID', () => { + const profile = DEFAULT_AGENT_PROFILES.find(p => p.id === 'balanced'); + expect(profile).toBeDefined(); + expect(profile?.id).toBe('balanced'); + expect(profile?.name).toBe('Balanced'); + expect(profile?.model).toBe('sonnet'); + }); + + it('should find quick profile by ID', () => { + const profile = DEFAULT_AGENT_PROFILES.find(p => p.id === 'quick'); + expect(profile).toBeDefined(); + expect(profile?.id).toBe('quick'); + expect(profile?.name).toBe('Quick Edits'); + expect(profile?.model).toBe('haiku'); + }); + }); + + describe('Auto Profile Phase Configuration', () => { + it('should have Opus for all phases in auto profile', () => { + const profile = DEFAULT_AGENT_PROFILES.find(p => p.id === 'auto'); + const phaseModels = profile?.phaseModels; + + expect(phaseModels).toBeDefined(); + expect(phaseModels?.spec).toBe('opus'); + expect(phaseModels?.planning).toBe('opus'); + expect(phaseModels?.coding).toBe('opus'); + expect(phaseModels?.qa).toBe('opus'); + }); + + it('should have optimized thinking levels in auto profile', () => { + const profile = DEFAULT_AGENT_PROFILES.find(p => p.id === 'auto'); + const phaseThinking = profile?.phaseThinking; + + expect(phaseThinking).toBeDefined(); + expect(phaseThinking?.spec).toBe('ultrathink'); + expect(phaseThinking?.planning).toBe('high'); + expect(phaseThinking?.coding).toBe('low'); + expect(phaseThinking?.qa).toBe('low'); + }); + }); + + describe('Balanced Profile Phase Configuration', () => { + it('should have Sonnet for all phases in balanced profile', () => { + const profile = DEFAULT_AGENT_PROFILES.find(p => p.id === 'balanced'); + const phaseModels = profile?.phaseModels; + + expect(phaseModels).toBeDefined(); + expect(phaseModels?.spec).toBe('sonnet'); + expect(phaseModels?.planning).toBe('sonnet'); + expect(phaseModels?.coding).toBe('sonnet'); + expect(phaseModels?.qa).toBe('sonnet'); + }); + + it('should have medium thinking for all phases in balanced profile', () => { + const profile = DEFAULT_AGENT_PROFILES.find(p => p.id === 'balanced'); + const phaseThinking = profile?.phaseThinking; + + expect(phaseThinking).toBeDefined(); + expect(phaseThinking?.spec).toBe('medium'); + expect(phaseThinking?.planning).toBe('medium'); + expect(phaseThinking?.coding).toBe('medium'); + expect(phaseThinking?.qa).toBe('medium'); + }); + }); + + describe('Profile Resolution Logic', () => { + it('should use profile phase models when no custom overrides exist', () => { + // Simulate settings with selected profile but no custom overrides + const selectedProfileId = 'auto'; + const customPhaseModels = undefined; + + const profile = DEFAULT_AGENT_PROFILES.find(p => p.id === selectedProfileId) || DEFAULT_AGENT_PROFILES[0]; + const profilePhaseModels = profile.phaseModels || DEFAULT_PHASE_MODELS; + const phaseModels = customPhaseModels || profilePhaseModels; + + // Should resolve to auto profile's opus models + expect(phaseModels.spec).toBe('opus'); + expect(phaseModels.planning).toBe('opus'); + expect(phaseModels.coding).toBe('opus'); + expect(phaseModels.qa).toBe('opus'); + }); + + it('should use custom overrides when they exist', () => { + // Simulate settings with custom overrides + const selectedProfileId = 'auto'; + const customPhaseModels = { + spec: 'sonnet' as const, + planning: 'sonnet' as const, + coding: 'sonnet' as const, + qa: 'sonnet' as const, + }; + + const profile = DEFAULT_AGENT_PROFILES.find(p => p.id === selectedProfileId) || DEFAULT_AGENT_PROFILES[0]; + const profilePhaseModels = profile.phaseModels || DEFAULT_PHASE_MODELS; + const phaseModels = customPhaseModels || profilePhaseModels; + + // Should resolve to custom overrides (sonnet) + expect(phaseModels.spec).toBe('sonnet'); + expect(phaseModels.planning).toBe('sonnet'); + expect(phaseModels.coding).toBe('sonnet'); + expect(phaseModels.qa).toBe('sonnet'); + }); + + it('should default to auto profile when selectedProfileId is undefined', () => { + const selectedProfileId = undefined; + const effectiveProfileId = selectedProfileId || 'auto'; + + const profile = DEFAULT_AGENT_PROFILES.find(p => p.id === effectiveProfileId) || DEFAULT_AGENT_PROFILES[0]; + + expect(profile.id).toBe('auto'); + expect(profile.model).toBe('opus'); + }); + + it('should fall back to first profile when selected profile is not found', () => { + const selectedProfileId = 'non-existent-profile'; + + const profile = DEFAULT_AGENT_PROFILES.find(p => p.id === selectedProfileId) || DEFAULT_AGENT_PROFILES[0]; + + expect(profile.id).toBe('auto'); + expect(profile.model).toBe('opus'); + }); + }); + + describe('Agent Settings Resolution (Utility)', () => { + it('should resolve phase-based agent settings correctly', () => { + const profile = DEFAULT_AGENT_PROFILES.find(p => p.id === 'auto')!; + const phaseModels = profile.phaseModels!; + const phaseThinking = profile.phaseThinking!; + const featureModels = DEFAULT_FEATURE_MODELS; + const featureThinking = DEFAULT_FEATURE_THINKING; + + const resolvedSettings = { phaseModels, phaseThinking, featureModels, featureThinking }; + + // Spec phase agent + const specAgent = resolveAgentSettings( + { type: 'phase', phase: 'spec' }, + resolvedSettings + ); + expect(specAgent.model).toBe('opus'); + expect(specAgent.thinking).toBe('ultrathink'); + + // Planning phase agent + const planningAgent = resolveAgentSettings( + { type: 'phase', phase: 'planning' }, + resolvedSettings + ); + expect(planningAgent.model).toBe('opus'); + expect(planningAgent.thinking).toBe('high'); + + // Coding phase agent + const codingAgent = resolveAgentSettings( + { type: 'phase', phase: 'coding' }, + resolvedSettings + ); + expect(codingAgent.model).toBe('opus'); + expect(codingAgent.thinking).toBe('low'); + + // QA phase agent + const qaAgent = resolveAgentSettings( + { type: 'phase', phase: 'qa' }, + resolvedSettings + ); + expect(qaAgent.model).toBe('opus'); + expect(qaAgent.thinking).toBe('low'); + }); + + it('should resolve feature-based agent settings correctly', () => { + const phaseModels = DEFAULT_PHASE_MODELS; + const phaseThinking = { spec: 'medium' as const, planning: 'medium' as const, coding: 'medium' as const, qa: 'medium' as const }; + const featureModels = DEFAULT_FEATURE_MODELS; + const featureThinking = DEFAULT_FEATURE_THINKING; + + const resolvedSettings = { phaseModels, phaseThinking, featureModels, featureThinking }; + + // Insights feature agent (defaults to sonnet) + const insightsAgent = resolveAgentSettings( + { type: 'feature', feature: 'insights' }, + resolvedSettings + ); + expect(insightsAgent.model).toBe('sonnet'); + expect(insightsAgent.thinking).toBe('medium'); + + // Ideation feature agent (defaults to opus) + const ideationAgent = resolveAgentSettings( + { type: 'feature', feature: 'ideation' }, + resolvedSettings + ); + expect(ideationAgent.model).toBe('opus'); + expect(ideationAgent.thinking).toBe('high'); + + // Roadmap feature agent (defaults to opus) + const roadmapAgent = resolveAgentSettings( + { type: 'feature', feature: 'roadmap' }, + resolvedSettings + ); + expect(roadmapAgent.model).toBe('opus'); + expect(roadmapAgent.thinking).toBe('high'); + + // GitHub Issues feature agent (defaults to opus) + const githubIssuesAgent = resolveAgentSettings( + { type: 'feature', feature: 'githubIssues' }, + resolvedSettings + ); + expect(githubIssuesAgent.model).toBe('opus'); + expect(githubIssuesAgent.thinking).toBe('medium'); + + // GitHub PRs feature agent (defaults to opus) + const githubPrsAgent = resolveAgentSettings( + { type: 'feature', feature: 'githubPrs' }, + resolvedSettings + ); + expect(githubPrsAgent.model).toBe('opus'); + expect(githubPrsAgent.thinking).toBe('medium'); + + // Utility feature agent (defaults to haiku) + const utilityAgent = resolveAgentSettings( + { type: 'feature', feature: 'utility' }, + resolvedSettings + ); + expect(utilityAgent.model).toBe('haiku'); + expect(utilityAgent.thinking).toBe('low'); + }); + + it('should resolve fixed settings correctly', () => { + const phaseModels = DEFAULT_PHASE_MODELS; + const phaseThinking = { spec: 'medium' as const, planning: 'medium' as const, coding: 'medium' as const, qa: 'medium' as const }; + const featureModels = DEFAULT_FEATURE_MODELS; + const featureThinking = DEFAULT_FEATURE_THINKING; + + const resolvedSettings = { phaseModels, phaseThinking, featureModels, featureThinking }; + + // Fixed settings agent + const fixedAgent = resolveAgentSettings( + { type: 'fixed', model: 'opus', thinking: 'high' }, + resolvedSettings + ); + expect(fixedAgent.model).toBe('opus'); + expect(fixedAgent.thinking).toBe('high'); + }); + }); + + describe('Bug Fix Regression Test (ACS-255)', () => { + it('should resolve to opus when auto profile is selected (not sonnet from defaults)', () => { + // This test verifies the fix for ACS-255: + // MCP Server Overview was showing Sonnet instead of Opus when Auto profile was selected + + const selectedProfileId = 'auto'; + const customPhaseModels = undefined; // No custom overrides + + // The bug was using DEFAULT_PHASE_MODELS directly (which is BALANCED_PHASE_MODELS = sonnet) + // The fix is to resolve the selected profile first + const profile = DEFAULT_AGENT_PROFILES.find(p => p.id === selectedProfileId) || DEFAULT_AGENT_PROFILES[0]; + const profilePhaseModels = profile.phaseModels || DEFAULT_PHASE_MODELS; + const phaseModels = customPhaseModels || profilePhaseModels; + + // Should be opus (from auto profile), NOT sonnet (from DEFAULT_PHASE_MODELS) + expect(phaseModels.spec).toBe('opus'); + expect(phaseModels.planning).toBe('opus'); + expect(phaseModels.coding).toBe('opus'); + expect(phaseModels.qa).toBe('opus'); + }); + + it('should ensure DEFAULT_PHASE_MODELS is balanced (sonnet)', () => { + // This documents that DEFAULT_PHASE_MODELS is the balanced profile (sonnet) + // The bug was that this was being used instead of resolving the selected profile + + expect(DEFAULT_PHASE_MODELS.spec).toBe('sonnet'); + expect(DEFAULT_PHASE_MODELS.planning).toBe('sonnet'); + expect(DEFAULT_PHASE_MODELS.coding).toBe('sonnet'); + expect(DEFAULT_PHASE_MODELS.qa).toBe('sonnet'); + }); + }); +}); diff --git a/apps/frontend/src/renderer/hooks/index.ts b/apps/frontend/src/renderer/hooks/index.ts index 21f70a6a..2afff22a 100644 --- a/apps/frontend/src/renderer/hooks/index.ts +++ b/apps/frontend/src/renderer/hooks/index.ts @@ -1,4 +1,10 @@ // Export all custom hooks -export { useIpcListeners } from './useIpc'; -export { useVirtualizedTree } from './useVirtualizedTree'; export { useClaudeLoginTerminal } from './useClaudeLoginTerminal'; +export { useIpcListeners } from './useIpc'; +export { + useResolvedAgentSettings, + resolveAgentSettings, + type ResolvedAgentSettings, + type AgentSettingsSource, +} from './useResolvedAgentSettings'; +export { useVirtualizedTree } from './useVirtualizedTree'; diff --git a/apps/frontend/src/renderer/hooks/useResolvedAgentSettings.ts b/apps/frontend/src/renderer/hooks/useResolvedAgentSettings.ts new file mode 100644 index 00000000..de8b5a4c --- /dev/null +++ b/apps/frontend/src/renderer/hooks/useResolvedAgentSettings.ts @@ -0,0 +1,147 @@ +/** + * Agent Settings Resolution Hook + * + * Provides centralized logic for resolving agent model and thinking settings + * based on the selected agent profile, custom overrides, and defaults. + * + * Resolution order for phase settings: + * 1. Custom phase overrides (if user has customized) + * 2. Selected profile's phaseModels/phaseThinking + * 3. DEFAULT_PHASE_MODELS/DEFAULT_PHASE_THINKING (fallback) + * + * Feature settings are not tied to profiles and use: + * 1. Custom feature overrides (if user has customized) + * 2. DEFAULT_FEATURE_MODELS/DEFAULT_FEATURE_THINKING (fallback) + */ + +import { useMemo } from 'react'; +import { + DEFAULT_AGENT_PROFILES, + DEFAULT_PHASE_MODELS, + DEFAULT_PHASE_THINKING, + DEFAULT_FEATURE_MODELS, + DEFAULT_FEATURE_THINKING, +} from '../../shared/constants/models'; +import type { + AppSettings, + PhaseModelConfig, + PhaseThinkingConfig, + FeatureModelConfig, + FeatureThinkingConfig, + ModelTypeShort, + ThinkingLevel, +} from '../../shared/types/settings'; + +/** + * Resolved agent settings configuration + * Contains all the resolved model and thinking settings for agents + */ +export interface ResolvedAgentSettings { + /** Phase model settings (spec, planning, coding, qa) */ + phaseModels: PhaseModelConfig; + /** Phase thinking level settings */ + phaseThinking: PhaseThinkingConfig; + /** Feature model settings (insights, ideation, roadmap, githubIssues, githubPrs, utility) */ + featureModels: FeatureModelConfig; + /** Feature thinking level settings */ + featureThinking: FeatureThinkingConfig; +} + +/** + * Agent settings source configuration + * Determines where an agent's model and thinking settings come from + */ +export type AgentSettingsSource = + | { type: 'phase'; phase: 'spec' | 'planning' | 'coding' | 'qa' } + | { type: 'feature'; feature: 'insights' | 'ideation' | 'roadmap' | 'githubIssues' | 'githubPrs' | 'utility' } + | { type: 'fixed'; model: ModelTypeShort; thinking: ThinkingLevel }; + +/** + * Resolved model and thinking for an agent + */ +export interface AgentModelConfig { + model: ModelTypeShort; + thinking: ThinkingLevel; +} + +/** + * Hook to resolve agent settings based on the selected profile and custom overrides + * + * @param settings - The application settings containing selected profile and custom overrides + * @returns Resolved agent settings with proper profile resolution + * + * @example + * ```tsx + * const { phaseModels, phaseThinking, featureModels, featureThinking } = useResolvedAgentSettings(settings); + * ``` + */ +export function useResolvedAgentSettings(settings: AppSettings): ResolvedAgentSettings { + return useMemo(() => { + // Get selected profile ID, default to 'auto' + const selectedProfileId = settings.selectedAgentProfile || 'auto'; + + // Find the selected profile + const selectedProfile = DEFAULT_AGENT_PROFILES.find((p) => p.id === selectedProfileId) || DEFAULT_AGENT_PROFILES[0]; + + // Profile defaults (used when no custom overrides exist) + const profilePhaseModels = selectedProfile.phaseModels || DEFAULT_PHASE_MODELS; + const profilePhaseThinking = selectedProfile.phaseThinking || DEFAULT_PHASE_THINKING; + + // Effective phase config: custom overrides take priority over profile defaults + const phaseModels = settings.customPhaseModels || profilePhaseModels; + const phaseThinking = settings.customPhaseThinking || profilePhaseThinking; + + // Feature settings (not tied to profiles, use custom or defaults) + const featureModels = settings.featureModels || DEFAULT_FEATURE_MODELS; + const featureThinking = settings.featureThinking || DEFAULT_FEATURE_THINKING; + + return { + phaseModels, + phaseThinking, + featureModels, + featureThinking, + }; + }, [ + settings.selectedAgentProfile, + settings.customPhaseModels, + settings.customPhaseThinking, + settings.featureModels, + settings.featureThinking, + ]); +} + +/** + * Resolves model and thinking settings for a specific agent based on its settings source + * + * @param settingsSource - The agent's settings source (phase, feature, or fixed) + * @param resolvedSettings - The resolved agent settings from useResolvedAgentSettings + * @returns Model and thinking configuration for the agent + * + * @example + * ```tsx + * const resolvedSettings = useResolvedAgentSettings(settings); + * const { model, thinking } = resolveAgentSettings(agentConfig.settingsSource, resolvedSettings); + * ``` + */ +export function resolveAgentSettings( + settingsSource: AgentSettingsSource, + resolvedSettings: ResolvedAgentSettings +): AgentModelConfig { + if (settingsSource.type === 'phase') { + return { + model: resolvedSettings.phaseModels[settingsSource.phase], + thinking: resolvedSettings.phaseThinking[settingsSource.phase], + }; + } else if (settingsSource.type === 'feature') { + return { + model: resolvedSettings.featureModels[settingsSource.feature], + thinking: resolvedSettings.featureThinking[settingsSource.feature], + }; + } else { + // Fixed settings + return { + model: settingsSource.model, + thinking: settingsSource.thinking, + }; + } +}