diff --git a/src/tui/components/RunApp.tsx b/src/tui/components/RunApp.tsx index 3ce9d620..20735396 100644 --- a/src/tui/components/RunApp.tsx +++ b/src/tui/components/RunApp.tsx @@ -23,9 +23,10 @@ import type { HistoricExecutionContext } from './IterationDetailView.js'; import { ProgressDashboard } from './ProgressDashboard.js'; import { ConfirmationDialog } from './ConfirmationDialog.js'; import { HelpOverlay } from './HelpOverlay.js'; -import { SettingsView } from './SettingsView.js'; +import { SettingsView, getConfiguredAgentName } from './SettingsView.js'; import { AgentModelPicker, + normalizeModelValue, resolveAgentConfigForSelection, type AgentModelSelection, } from './AgentModelPicker.js'; @@ -1150,6 +1151,25 @@ export function RunApp({ const displayTrackerName = isViewingRemote ? (remoteTrackerName ?? trackerName) : trackerName; const displayModel = isViewingRemote ? (remoteModel ?? currentModel) : localModel; + const handleSettingsSave = useCallback( + async (newConfig: StoredConfig): Promise => { + if (!onSaveSettings) { + throw new Error('Settings save is not available'); + } + + const previousAgent = getConfiguredAgentName(storedConfig); + const nextAgent = getConfiguredAgentName(newConfig); + const previousModel = normalizeModelValue(storedConfig?.model); + const nextModel = normalizeModelValue(newConfig.model); + + await onSaveSettings(newConfig); + if (previousAgent !== nextAgent || previousModel !== nextModel) { + setDetectedModel(nextModel ?? ''); + } + }, + [onSaveSettings, storedConfig] + ); + const handleAgentModelConfirm = useCallback( async (selection: AgentModelSelection): Promise => { if (!engine) { @@ -3976,8 +3996,9 @@ export function RunApp({ visible={showSettings} config={storedConfig} agents={availableAgents} + agentConfigs={storedConfig.agents} trackers={availableTrackers} - onSave={onSaveSettings} + onSave={handleSettingsSave} onClose={() => setShowSettings(false)} /> )} diff --git a/src/tui/components/SettingsView.test.ts b/src/tui/components/SettingsView.test.ts new file mode 100644 index 00000000..fb5f31b5 --- /dev/null +++ b/src/tui/components/SettingsView.test.ts @@ -0,0 +1,61 @@ +/** + * ABOUTME: Tests for SettingsView helper behavior. + * Verifies model setting choices are derived from selected agent metadata. + */ + +import { beforeAll, describe, expect, test } from 'bun:test'; +import { registerBuiltinAgents } from '../../plugins/agents/builtin/index.js'; +import { + buildModelOptionsForAgent, + buildSettingDefinitions, +} from './SettingsView.js'; + +beforeAll(() => { + registerBuiltinAgents(); +}); + +describe('SettingsView helpers', () => { + test('shows known Claude models as select choices', () => { + const settings = buildSettingDefinitions(['claude'], [], { agent: 'claude' }); + const modelSetting = settings.find((setting) => setting.key === 'model'); + + expect(modelSetting?.type).toBe('select'); + expect(modelSetting?.options).toEqual(['', 'sonnet', 'opus', 'haiku']); + }); + + test('resolves configured agent aliases for model choices', () => { + const options = buildModelOptionsForAgent( + 'work-claude', + [ + { + name: 'work-claude', + plugin: 'claude', + options: {}, + }, + ], + undefined + ); + + expect(options).toEqual(['', 'sonnet', 'opus', 'haiku']); + }); + + test('keeps open-ended agents as free text', () => { + const settings = buildSettingDefinitions(['codex'], [], { agent: 'codex' }); + const modelSetting = settings.find((setting) => setting.key === 'model'); + + expect(modelSetting?.type).toBe('text'); + expect(modelSetting?.options).toBeUndefined(); + }); + + test('clears model override when the default choice is selected', () => { + const settings = buildSettingDefinitions(['claude'], [], { + agent: 'claude', + model: 'opus', + }); + const modelSetting = settings.find((setting) => setting.key === 'model'); + + expect(modelSetting?.setValue({ agent: 'claude', model: 'opus' }, '')).toEqual({ + agent: 'claude', + }); + }); +}); diff --git a/src/tui/components/SettingsView.tsx b/src/tui/components/SettingsView.tsx index e03feee5..2387ce4c 100644 --- a/src/tui/components/SettingsView.tsx +++ b/src/tui/components/SettingsView.tsx @@ -9,17 +9,22 @@ import { useState, useCallback, useEffect } from 'react'; import { useKeyboard } from '@opentui/react'; import { colors } from '../theme.js'; import type { StoredConfig, SubagentDetailLevel, NotificationSoundMode } from '../../config/types.js'; +import type { AgentPluginConfig } from '../../plugins/agents/types.js'; import type { TrackerPluginMeta } from '../../plugins/trackers/types.js'; +import { + listModelsForAgent, + normalizeModelValue, +} from './AgentModelPicker.js'; /** * Setting item types for different field kinds */ -type SettingType = 'select' | 'number' | 'boolean' | 'text'; +export type SettingType = 'select' | 'number' | 'boolean' | 'text'; /** * Individual setting definition */ -interface SettingDefinition { +export interface SettingDefinition { key: string; label: string; type: SettingType; @@ -42,6 +47,8 @@ export interface SettingsViewProps { config: StoredConfig; /** Available agent names for selection */ agents: string[]; + /** Configured agent instances used to resolve agent aliases */ + agentConfigs?: AgentPluginConfig[]; /** Available tracker plugins */ trackers: TrackerPluginMeta[]; /** Callback when settings should be saved */ @@ -50,13 +57,78 @@ export interface SettingsViewProps { onClose: () => void; } +/** + * Resolve the active agent name from stored config using runtime precedence. + */ +export function getConfiguredAgentName(config: StoredConfig | undefined): string | undefined { + return ( + config?.agent ?? + config?.defaultAgent ?? + config?.agents?.find((agent) => agent.default)?.name ?? + config?.agents?.[0]?.name + ); +} + +/** + * Build model choices for agents that expose known model names. + */ +export function buildModelOptionsForAgent( + agentName: string | undefined, + agentConfigs: AgentPluginConfig[], + currentModel: string | undefined +): string[] | undefined { + if (!agentName) { + return undefined; + } + + const knownModels = listModelsForAgent(agentName, agentConfigs); + if (knownModels.length === 0) { + return undefined; + } + + const normalizedCurrent = normalizeModelValue(currentModel); + return [ + '', + ...(normalizedCurrent && !knownModels.includes(normalizedCurrent) + ? [normalizedCurrent] + : []), + ...knownModels, + ]; +} + +/** + * Apply a model value to stored config, removing the field for the default model. + */ +function setModelValue( + config: StoredConfig, + value: string | number | boolean +): StoredConfig { + const nextConfig = { ...config }; + const normalized = normalizeModelValue(String(value)); + if (normalized) { + nextConfig.model = normalized; + } else { + delete nextConfig.model; + } + return nextConfig; +} + /** * Build setting definitions based on available plugins */ -function buildSettingDefinitions( +export function buildSettingDefinitions( agents: string[], - trackers: TrackerPluginMeta[] + trackers: TrackerPluginMeta[], + config: StoredConfig, + agentConfigs: AgentPluginConfig[] = [] ): SettingDefinition[] { + const selectedAgent = getConfiguredAgentName(config); + const modelOptions = buildModelOptionsForAgent( + selectedAgent, + agentConfigs, + config.model + ); + return [ { key: 'tracker', @@ -78,7 +150,7 @@ function buildSettingDefinitions( type: 'select', description: 'AI agent plugin to use', options: agents, - getValue: (config) => config.agent ?? config.defaultAgent ?? config.agents?.[0]?.name, + getValue: (config) => getConfiguredAgentName(config), setValue: (config, value) => ({ ...config, agent: value as string, @@ -89,13 +161,11 @@ function buildSettingDefinitions( { key: 'model', label: 'Model', - type: 'text', + type: modelOptions ? 'select' : 'text', description: 'Model override for the selected agent', - getValue: (config) => config.model, - setValue: (config, value) => ({ - ...config, - model: value as string, - }), + options: modelOptions, + getValue: (config) => normalizeModelValue(config.model) ?? '', + setValue: setModelValue, requiresRestart: false, }, { @@ -189,7 +259,7 @@ function buildSettingDefinitions( * Format a setting value for display */ function formatValue(value: string | number | boolean | undefined): string { - if (value === undefined) return '(not set)'; + if (value === undefined || value === '') return '(not set)'; if (typeof value === 'boolean') return value ? 'Yes' : 'No'; return String(value); } @@ -201,6 +271,7 @@ export function SettingsView({ visible, config, agents, + agentConfigs = [], trackers, onSave, onClose, @@ -213,7 +284,12 @@ export function SettingsView({ const [error, setError] = useState(null); const [hasChanges, setHasChanges] = useState(false); - const settings = buildSettingDefinitions(agents, trackers); + const settings = buildSettingDefinitions( + agents, + trackers, + editingConfig, + agentConfigs + ); // Reset state when config changes externally useEffect(() => {