From 92c8f4e3dc25f8a159a412141105b67142abbfe1 Mon Sep 17 00:00:00 2001 From: wxw <1406771356@qq.com> Date: Wed, 24 Jun 2026 18:16:37 +0400 Subject: [PATCH 01/11] feat: improve config provider TUI interaction - Support wrap-around navigation in provider, custom provider, and model lists (up at first item jumps to last, down at last item jumps to first) - Add d key to delete custom models in model selection step with confirmation prompt - Add protocol selection (anthropic/openai) to manual configuration flow and respect UseAnthropic setting in saved config - Preserve manual protocol selection when re-entering the form - Fix manual form input fields losing focus after returning from next step - Use safe slice removal in removeFromSlice to avoid backing array corruption --- cmd/opencodereview/provider_cmd.go | 8 ++ cmd/opencodereview/provider_tui.go | 139 ++++++++++++++++++++++++++--- 2 files changed, 135 insertions(+), 12 deletions(-) diff --git a/cmd/opencodereview/provider_cmd.go b/cmd/opencodereview/provider_cmd.go index 9e29394..af2ab85 100644 --- a/cmd/opencodereview/provider_cmd.go +++ b/cmd/opencodereview/provider_cmd.go @@ -95,6 +95,13 @@ func applyManualConfig(configPath string, cfg *Config, result providerTUIResult) cfg.Llm.URL = result.url cfg.Llm.Model = result.model cfg.Llm.AuthToken = result.apiKey + if result.protocol == "anthropic" { + useAnthropic := true + cfg.Llm.UseAnthropic = &useAnthropic + } else { + useAnthropic := false + cfg.Llm.UseAnthropic = &useAnthropic + } if err := saveConfig(configPath, cfg); err != nil { return err @@ -102,6 +109,7 @@ func applyManualConfig(configPath string, cfg *Config, result providerTUIResult) fmt.Println("\nManual configuration saved.") fmt.Printf("URL: %s\n", result.url) + fmt.Printf("Protocol: %s\n", result.protocol) fmt.Printf("Model: %s\n", result.model) fmt.Println("\nTesting connection...") diff --git a/cmd/opencodereview/provider_tui.go b/cmd/opencodereview/provider_tui.go index 52e9493..707348b 100644 --- a/cmd/opencodereview/provider_tui.go +++ b/cmd/opencodereview/provider_tui.go @@ -46,6 +46,7 @@ type manualStep int const ( manualStepURL manualStep = iota + manualStepProtocol manualStepModel manualStepAuthToken ) @@ -93,11 +94,12 @@ type providerTUIModel struct { cpAuthInput textinput.Model // --- tab: manual --- - inManualForm bool - manualStep manualStep - manualURLInput textinput.Model - manualModelInput textinput.Model - manualTokenInput textinput.Model + inManualForm bool + manualStep manualStep + manualProtocolIdx int + manualURLInput textinput.Model + manualModelInput textinput.Model + manualTokenInput textinput.Model // --- shared model/api-key steps (official + existing custom) --- modelIdx int @@ -113,10 +115,12 @@ type providerTUIModel struct { cancelled bool // --- delete confirmation --- - confirmingDelete bool - deleteTargetIdx int - deleteTargetName string - deletedProviders []string + confirmingDelete bool + deleteTargetIdx int + deleteTargetName string + deletedProviders []string + confirmingDeleteModel bool + deleteModelName string } func collectCustomProviders(cfg *Config) []customProviderListItem { @@ -260,6 +264,11 @@ func newProviderTUI(cfg *Config) providerTUIModel { if cfg.Llm.AuthToken != "" { m.manualTokenInput.SetValue(cfg.Llm.AuthToken) } + if cfg.Llm.UseAnthropic == nil || *cfg.Llm.UseAnthropic { + m.manualProtocolIdx = 0 // anthropic + } else { + m.manualProtocolIdx = 1 // openai + } } return m @@ -380,6 +389,10 @@ func (m providerTUIModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m.updateDeleteConfirm(key) } + if m.step == stepModel && m.confirmingDeleteModel { + return m.updateDeleteModelConfirm(key) + } + switch key { case "ctrl+c": m.cancelled = true @@ -429,6 +442,14 @@ func (m providerTUIModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.confirmingDelete = true m.deleteTargetIdx = m.customIdx m.deleteTargetName = m.customProviders[m.customIdx].name + return m, nil + } + if m.step == stepModel && m.activeTab == tabCustom && m.customIdx < len(m.customProviders) { + models := m.models() + if m.modelIdx < len(models) { + m.confirmingDeleteModel = true + m.deleteModelName = models[m.modelIdx] + } } return m, nil } @@ -661,6 +682,16 @@ func (m providerTUIModel) updateManualForm(key string, msg tea.KeyPressMsg) (tea return m, m.focusManualStep() case "enter": return m.handleManualFormEnter() + case "up", "k": + if m.manualStep == manualStepProtocol && m.manualProtocolIdx > 0 { + m.manualProtocolIdx-- + } + return m, nil + case "down", "j": + if m.manualStep == manualStepProtocol && m.manualProtocolIdx < len(cpProtocols)-1 { + m.manualProtocolIdx++ + } + return m, nil default: return m.passThroughManualInput(msg) } @@ -693,20 +724,49 @@ func (m providerTUIModel) updateDeleteConfirm(key string) (tea.Model, tea.Cmd) { return m, nil } +func (m providerTUIModel) updateDeleteModelConfirm(key string) (tea.Model, tea.Cmd) { + switch key { + case "y", "Y": + if m.customIdx >= len(m.customProviders) { + m.confirmingDeleteModel = false + return m, nil + } + models := m.models() + if m.modelIdx < len(models) { + cp := m.customProviders[m.customIdx] + cp.entry.Models = removeFromSlice(cp.entry.Models, m.modelIdx) + m.customProviders[m.customIdx] = cp + if m.modelIdx >= len(cp.entry.Models) && m.modelIdx > 0 { + m.modelIdx = len(cp.entry.Models) - 1 + } + } + m.confirmingDeleteModel = false + return m, nil + case "n", "N", "esc": + m.confirmingDeleteModel = false + return m, nil + case "ctrl+c": + m.cancelled = true + return m, tea.Quit + } + return m, nil +} + func (m providerTUIModel) handleManualFormEnter() (tea.Model, tea.Cmd) { switch m.manualStep { case manualStepURL: if m.manualURLInput.Value() == "" { return m, nil } - m.manualURLInput.Blur() + m.manualStep = manualStepProtocol + return m, nil + case manualStepProtocol: m.manualStep = manualStepModel return m, m.manualModelInput.Focus() case manualStepModel: if m.manualModelInput.Value() == "" { return m, nil } - m.manualModelInput.Blur() m.manualStep = manualStepAuthToken return m, m.manualTokenInput.Focus() case manualStepAuthToken: @@ -721,6 +781,8 @@ func (m *providerTUIModel) blurManualStep() { switch m.manualStep { case manualStepURL: m.manualURLInput.Blur() + case manualStepProtocol: + // no input to blur case manualStepModel: m.manualModelInput.Blur() case manualStepAuthToken: @@ -732,6 +794,8 @@ func (m providerTUIModel) focusManualStep() tea.Cmd { switch m.manualStep { case manualStepURL: return m.manualURLInput.Focus() + case manualStepProtocol: + return nil case manualStepModel: return m.manualModelInput.Focus() case manualStepAuthToken: @@ -745,6 +809,8 @@ func (m providerTUIModel) passThroughManualInput(msg tea.Msg) (tea.Model, tea.Cm switch m.manualStep { case manualStepURL: m.manualURLInput, cmd = m.manualURLInput.Update(msg) + case manualStepProtocol: + return m, nil case manualStepModel: m.manualModelInput, cmd = m.manualModelInput.Update(msg) case manualStepAuthToken: @@ -813,15 +879,21 @@ func (m providerTUIModel) handleUp() (tea.Model, tea.Cmd) { case tabOfficial: if m.officialIdx > 0 { m.officialIdx-- + } else { + m.officialIdx = len(m.providers) - 1 } case tabCustom: if m.customIdx > 0 { m.customIdx-- + } else { + m.customIdx = m.customListCount() - 1 } } case stepModel: if m.modelIdx > 0 { m.modelIdx-- + } else { + m.modelIdx = m.modelCount() - 1 } } return m, nil @@ -834,20 +906,36 @@ func (m providerTUIModel) handleDown() (tea.Model, tea.Cmd) { case tabOfficial: if m.officialIdx < len(m.providers)-1 { m.officialIdx++ + } else { + m.officialIdx = 0 } case tabCustom: if m.customIdx < m.customListCount()-1 { m.customIdx++ + } else { + m.customIdx = 0 } } case stepModel: if m.modelIdx < m.modelCount()-1 { m.modelIdx++ + } else { + m.modelIdx = 0 } } return m, nil } +func removeFromSlice(s []string, idx int) []string { + if idx < 0 || idx >= len(s) { + return s + } + result := make([]string, 0, len(s)-1) + result = append(result, s[:idx]...) + result = append(result, s[idx+1:]...) + return result +} + func (m *providerTUIModel) loadExistingAPIKey() { m.apiKeyMasked = false m.apiKeyOriginal = "" @@ -950,6 +1038,7 @@ func (m providerTUIModel) result() providerTUIResult { url: m.manualURLInput.Value(), model: m.manualModelInput.Value(), apiKey: m.manualTokenInput.Value(), + protocol: cpProtocols[m.manualProtocolIdx], } } @@ -1181,6 +1270,7 @@ func (m providerTUIModel) viewManualTab(s *strings.Builder) { fields := []field{ {"URL", m.manualURLInput.Value(), m.manualStep == manualStepURL}, + {"Protocol", cpProtocols[m.manualProtocolIdx], m.manualStep == manualStepProtocol}, {"Model", m.manualModelInput.Value(), m.manualStep == manualStepModel}, {"Auth Token", strings.Repeat("*", len(m.manualTokenInput.Value())), m.manualStep == manualStepAuthToken}, } @@ -1191,6 +1281,18 @@ func (m providerTUIModel) viewManualTab(s *strings.Builder) { switch m.manualStep { case manualStepURL: s.WriteString(" " + m.manualURLInput.View() + "\n") + case manualStepProtocol: + for i, proto := range cpProtocols { + cur := " " + if i == m.manualProtocolIdx { + cur = " " + tuiCursorStyle.Render(tuiCursor) + " " + } + if i == m.manualProtocolIdx { + s.WriteString(cur + tuiSelectedItemStyle.Render(proto) + "\n") + } else { + s.WriteString(cur + tuiItemStyle.Render(proto) + "\n") + } + } case manualStepModel: s.WriteString(" " + m.manualModelInput.View() + "\n") case manualStepAuthToken: @@ -1240,7 +1342,16 @@ func (m providerTUIModel) viewModel(s *strings.Builder) { } s.WriteString("\n") - s.WriteString(tuiHelpStyle.Render(" ↑/↓ Select Enter Confirm Esc Back")) + + if m.confirmingDeleteModel { + s.WriteString(" " + tuiSelectedItemStyle.Render(fmt.Sprintf("Delete %q? (y/n)", m.deleteModelName))) + s.WriteString("\n") + s.WriteString(tuiHelpStyle.Render(" y Confirm · n/Esc Cancel")) + } else if m.activeTab == tabCustom && m.customIdx < len(m.customProviders) { + s.WriteString(tuiHelpStyle.Render(" ↑/↓ Select Enter Confirm d Delete Esc Back")) + } else { + s.WriteString(tuiHelpStyle.Render(" ↑/↓ Select Enter Confirm Esc Back")) + } s.WriteString("\n") } @@ -1411,11 +1522,15 @@ func (m modelTUIModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case "up", "k": if m.modelIdx > 0 { m.modelIdx-- + } else { + m.modelIdx = m.itemCount() - 1 } return m, nil case "down", "j": if m.modelIdx < m.itemCount()-1 { m.modelIdx++ + } else { + m.modelIdx = 0 } return m, nil } From db84d2723e28a252139ce13aec27e2049d76dd19 Mon Sep 17 00:00:00 2001 From: wxw <1406771356@qq.com> Date: Wed, 24 Jun 2026 20:42:46 +0400 Subject: [PATCH 02/11] fix: address code review feedback - Restore Blur() calls in handleManualFormEnter to prevent visual focus artifacts when transitioning between manual form steps - Add empty-providers guard in handleUp/handleDown to avoid negative index - Consolidate redundant protocolIdx checks into single if/else in viewManualTab and viewCustomProviderForm for clarity --- cmd/opencodereview/provider_tui.go | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/cmd/opencodereview/provider_tui.go b/cmd/opencodereview/provider_tui.go index 707348b..14555d5 100644 --- a/cmd/opencodereview/provider_tui.go +++ b/cmd/opencodereview/provider_tui.go @@ -758,6 +758,7 @@ func (m providerTUIModel) handleManualFormEnter() (tea.Model, tea.Cmd) { if m.manualURLInput.Value() == "" { return m, nil } + m.manualURLInput.Blur() m.manualStep = manualStepProtocol return m, nil case manualStepProtocol: @@ -767,6 +768,7 @@ func (m providerTUIModel) handleManualFormEnter() (tea.Model, tea.Cmd) { if m.manualModelInput.Value() == "" { return m, nil } + m.manualModelInput.Blur() m.manualStep = manualStepAuthToken return m, m.manualTokenInput.Focus() case manualStepAuthToken: @@ -879,7 +881,7 @@ func (m providerTUIModel) handleUp() (tea.Model, tea.Cmd) { case tabOfficial: if m.officialIdx > 0 { m.officialIdx-- - } else { + } else if len(m.providers) > 0 { m.officialIdx = len(m.providers) - 1 } case tabCustom: @@ -906,7 +908,7 @@ func (m providerTUIModel) handleDown() (tea.Model, tea.Cmd) { case tabOfficial: if m.officialIdx < len(m.providers)-1 { m.officialIdx++ - } else { + } else if len(m.providers) > 0 { m.officialIdx = 0 } case tabCustom: @@ -1215,13 +1217,11 @@ func (m providerTUIModel) viewCustomProviderForm(s *strings.Builder) { s.WriteString(" " + m.cpNameInput.View() + "\n") case cpStepProtocol: for i, proto := range cpProtocols { - cur := " " - if i == m.cpProtocolIdx { - cur = " " + tuiCursorStyle.Render(tuiCursor) + " " - } if i == m.cpProtocolIdx { + cur := " " + tuiCursorStyle.Render(tuiCursor) + " " s.WriteString(cur + tuiSelectedItemStyle.Render(proto) + "\n") } else { + cur := " " s.WriteString(cur + tuiItemStyle.Render(proto) + "\n") } } @@ -1283,13 +1283,11 @@ func (m providerTUIModel) viewManualTab(s *strings.Builder) { s.WriteString(" " + m.manualURLInput.View() + "\n") case manualStepProtocol: for i, proto := range cpProtocols { - cur := " " - if i == m.manualProtocolIdx { - cur = " " + tuiCursorStyle.Render(tuiCursor) + " " - } if i == m.manualProtocolIdx { + cur := " " + tuiCursorStyle.Render(tuiCursor) + " " s.WriteString(cur + tuiSelectedItemStyle.Render(proto) + "\n") } else { + cur := " " s.WriteString(cur + tuiItemStyle.Render(proto) + "\n") } } From 19065bc59823122a4cef65c7a4ecc2335e7c697f Mon Sep 17 00:00:00 2001 From: wxw <1406771356@qq.com> Date: Thu, 25 Jun 2026 07:30:47 +0400 Subject: [PATCH 03/11] fix: persist custom model deletions and support masked auth token editing - Track deleted custom models in TUI and apply them on exit (both confirm and cancel paths) to keep config.json in sync - Clear custom provider's active Model field when that model is deleted, preventing stale "model" reference in the provider list label - Add masked display for manual config Auth Token to allow easy re-entry, matching the official provider flow (any key clears and starts fresh) --- cmd/opencodereview/provider_cmd.go | 62 +++++++++++++++++++++++++++++- cmd/opencodereview/provider_tui.go | 52 ++++++++++++++++++++----- 2 files changed, 104 insertions(+), 10 deletions(-) diff --git a/cmd/opencodereview/provider_cmd.go b/cmd/opencodereview/provider_cmd.go index af2ab85..34a9c73 100644 --- a/cmd/opencodereview/provider_cmd.go +++ b/cmd/opencodereview/provider_cmd.go @@ -42,8 +42,14 @@ func runConfigProvider() error { } } + if len(final.deletedModels) > 0 { + if err := applyModelDeletions(configPath, cfg, final.deletedModels); err != nil { + return err + } + } + if !final.confirmed { - if len(final.deletedProviders) > 0 { + if len(final.deletedProviders) > 0 || len(final.deletedModels) > 0 { return nil } fmt.Println("Cancelled.") @@ -82,6 +88,60 @@ func applyProviderDeletions(configPath string, cfg *Config, names []string) (boo return clearedActive, nil } +func applyModelDeletions(configPath string, cfg *Config, deletedModels map[string][]string) error { + if cfg.CustomProviders == nil { + return nil + } + changed := false + for name, models := range deletedModels { + entry, ok := cfg.CustomProviders[name] + if !ok { + continue + } + original := entry.Models + entry.Models = removeModels(entry.Models, models) + modelsChanged := len(entry.Models) != len(original) + + entryChanged := modelsChanged + if modelListContains(models, entry.Model) { + entry.Model = "" + entryChanged = true + } + if cfg.Provider == name && modelListContains(models, cfg.Model) { + cfg.Model = "" + changed = true + } + if entryChanged { + cfg.CustomProviders[name] = entry + changed = true + for _, m := range original { + if !modelListContains(entry.Models, m) { + fmt.Printf("Deleted model %q from custom provider %q.\n", m, name) + } + } + } + } + if !changed { + return nil + } + return saveConfig(configPath, cfg) +} + +func removeModels(existing, toRemove []string) []string { + removeSet := make(map[string]struct{}, len(toRemove)) + for _, m := range toRemove { + removeSet[m] = struct{}{} + } + result := make([]string, 0, len(existing)) + for _, m := range existing { + if _, found := removeSet[m]; found { + continue + } + result = append(result, m) + } + return result +} + func applyManualConfig(configPath string, cfg *Config, result providerTUIResult) error { if result.url == "" { return fmt.Errorf("URL is required for manual configuration") diff --git a/cmd/opencodereview/provider_tui.go b/cmd/opencodereview/provider_tui.go index 14555d5..2f70e91 100644 --- a/cmd/opencodereview/provider_tui.go +++ b/cmd/opencodereview/provider_tui.go @@ -94,12 +94,14 @@ type providerTUIModel struct { cpAuthInput textinput.Model // --- tab: manual --- - inManualForm bool - manualStep manualStep - manualProtocolIdx int - manualURLInput textinput.Model - manualModelInput textinput.Model - manualTokenInput textinput.Model + inManualForm bool + manualStep manualStep + manualProtocolIdx int + manualURLInput textinput.Model + manualModelInput textinput.Model + manualTokenInput textinput.Model + manualTokenMasked bool + manualTokenOriginal string // --- shared model/api-key steps (official + existing custom) --- modelIdx int @@ -121,6 +123,7 @@ type providerTUIModel struct { deletedProviders []string confirmingDeleteModel bool deleteModelName string + deletedModels map[string][]string } func collectCustomProviders(cfg *Config) []customProviderListItem { @@ -262,7 +265,9 @@ func newProviderTUI(cfg *Config) providerTUIModel { m.manualURLInput.SetValue(cfg.Llm.URL) m.manualModelInput.SetValue(cfg.Llm.Model) if cfg.Llm.AuthToken != "" { - m.manualTokenInput.SetValue(cfg.Llm.AuthToken) + m.manualTokenOriginal = cfg.Llm.AuthToken + m.manualTokenMasked = true + m.manualTokenInput.SetValue(strings.Repeat("*", 20)) } if cfg.Llm.UseAnthropic == nil || *cfg.Llm.UseAnthropic { m.manualProtocolIdx = 0 // anthropic @@ -669,11 +674,21 @@ func (m providerTUIModel) updateManualForm(key string, msg tea.KeyPressMsg) (tea if m.existingCfg != nil { m.manualURLInput.SetValue(m.existingCfg.Llm.URL) m.manualModelInput.SetValue(m.existingCfg.Llm.Model) - m.manualTokenInput.SetValue(m.existingCfg.Llm.AuthToken) + if m.existingCfg.Llm.AuthToken != "" { + m.manualTokenOriginal = m.existingCfg.Llm.AuthToken + m.manualTokenMasked = true + m.manualTokenInput.SetValue(strings.Repeat("*", 20)) + } else { + m.manualTokenInput.SetValue("") + m.manualTokenMasked = false + m.manualTokenOriginal = "" + } } else { m.manualURLInput.SetValue("") m.manualModelInput.SetValue("") m.manualTokenInput.SetValue("") + m.manualTokenMasked = false + m.manualTokenOriginal = "" } return m, nil } @@ -693,6 +708,14 @@ func (m providerTUIModel) updateManualForm(key string, msg tea.KeyPressMsg) (tea } return m, nil default: + if m.manualStep == manualStepAuthToken && m.manualTokenMasked { + if len(key) == 1 { + m.manualTokenMasked = false + m.manualTokenInput.SetValue("") + } else { + return m, nil + } + } return m.passThroughManualInput(msg) } } @@ -734,8 +757,15 @@ func (m providerTUIModel) updateDeleteModelConfirm(key string) (tea.Model, tea.C models := m.models() if m.modelIdx < len(models) { cp := m.customProviders[m.customIdx] + if cp.entry.Model == m.deleteModelName { + cp.entry.Model = "" + } cp.entry.Models = removeFromSlice(cp.entry.Models, m.modelIdx) m.customProviders[m.customIdx] = cp + if m.deletedModels == nil { + m.deletedModels = make(map[string][]string) + } + m.deletedModels[cp.name] = append(m.deletedModels[cp.name], m.deleteModelName) if m.modelIdx >= len(cp.entry.Models) && m.modelIdx > 0 { m.modelIdx = len(cp.entry.Models) - 1 } @@ -1035,11 +1065,15 @@ func (m providerTUIModel) result() providerTUIResult { return providerTUIResult{} case tabManual: + apiKey := m.manualTokenInput.Value() + if m.manualTokenMasked { + apiKey = m.manualTokenOriginal + } return providerTUIResult{ isManual: true, url: m.manualURLInput.Value(), model: m.manualModelInput.Value(), - apiKey: m.manualTokenInput.Value(), + apiKey: apiKey, protocol: cpProtocols[m.manualProtocolIdx], } } From 202259e22b39b4f77174765500563913aaeea1c3 Mon Sep 17 00:00:00 2001 From: wxw <1406771356@qq.com> Date: Thu, 25 Jun 2026 07:41:34 +0400 Subject: [PATCH 04/11] test: update manual form tests for masked auth token behavior Update TestProviderTUI_ManualFormPrefilledValues, TestProviderTUI_ManualFormEscRestoresOriginalValues, and TestProviderTUI_ManualFormPrefilledWhenProviderSet to assert the new masked display state (manualTokenMasked + manualTokenOriginal) instead of the raw token value. --- cmd/opencodereview/provider_tui_test.go | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/cmd/opencodereview/provider_tui_test.go b/cmd/opencodereview/provider_tui_test.go index cb54b83..142f66c 100644 --- a/cmd/opencodereview/provider_tui_test.go +++ b/cmd/opencodereview/provider_tui_test.go @@ -293,8 +293,11 @@ func TestProviderTUI_ManualFormEscRestoresOriginalValues(t *testing.T) { if m3.manualModelInput.Value() != "test-model" { t.Errorf("Model not restored: got %q, want %q", m3.manualModelInput.Value(), "test-model") } - if m3.manualTokenInput.Value() != "token-123" { - t.Errorf("Token not restored: got %q, want %q", m3.manualTokenInput.Value(), "token-123") + if !m3.manualTokenMasked { + t.Error("Token should be masked after Esc restore") + } + if m3.manualTokenOriginal != "token-123" { + t.Errorf("Token original not restored: got %q, want %q", m3.manualTokenOriginal, "token-123") } } @@ -317,8 +320,14 @@ func TestProviderTUI_ManualFormPrefilledValues(t *testing.T) { if m.manualModelInput.Value() != "test-model" { t.Errorf("Model not prefilled: got %q", m.manualModelInput.Value()) } - if m.manualTokenInput.Value() != "token-123" { - t.Errorf("Token not prefilled: got %q", m.manualTokenInput.Value()) + if !m.manualTokenMasked { + t.Error("Token should be masked when prefilled") + } + if m.manualTokenOriginal != "token-123" { + t.Errorf("Token original not prefilled: got %q, want %q", m.manualTokenOriginal, "token-123") + } + if m.manualTokenInput.Value() != strings.Repeat("*", 20) { + t.Errorf("Token input not masked display: got %q", m.manualTokenInput.Value()) } } @@ -372,8 +381,11 @@ func TestProviderTUI_ManualFormPrefilledWhenProviderSet(t *testing.T) { if m.manualModelInput.Value() != "manual-model" { t.Errorf("Model not prefilled: got %q", m.manualModelInput.Value()) } - if m.manualTokenInput.Value() != "manual-token" { - t.Errorf("Token not prefilled: got %q", m.manualTokenInput.Value()) + if !m.manualTokenMasked { + t.Error("Token should be masked when prefilled") + } + if m.manualTokenOriginal != "manual-token" { + t.Errorf("Token original not prefilled: got %q, want %q", m.manualTokenOriginal, "manual-token") } } From efc3f0ccbf6d0dcd74998c244989463bf858ada0 Mon Sep 17 00:00:00 2001 From: wxw <1406771356@qq.com> Date: Thu, 25 Jun 2026 16:08:54 +0400 Subject: [PATCH 05/11] =?UTF-8?q?feat:=20refine=20config=20provider=20TUI?= =?UTF-8?q?=20flows=20and=20session=20persistence=20Custom=20provider=20in?= =?UTF-8?q?teractions:=20-=20Simplify=20create/edit=20form=20to=20Name=20?= =?UTF-8?q?=E2=86=92=20Protocol=20=E2=86=92=20URL=20=E2=86=92=20API=20Key?= =?UTF-8?q?=20=E2=86=92=20Auth=20Header=20-=20After=20create=20or=20edit?= =?UTF-8?q?=20save,=20jump=20straight=20into=20the=20model=20list=20for=20?= =?UTF-8?q?that=20provider=20-=20Support=20comma-separated=20model=20names?= =?UTF-8?q?=20in=20the=20custom=20model=20input=20-=20Show=20masked=20API?= =?UTF-8?q?=20key=20in=20edit=20form;=20empty=20Auth=20Header=20defaults?= =?UTF-8?q?=20to=20(Authorization)=20-=20Populate=20edit=20form=20from=20e?= =?UTF-8?q?xistingCfg=20to=20avoid=20stale=20list=20data=20after=20in-sess?= =?UTF-8?q?ion=20saves=20Model=20selection=20and=20highlight:=20-=20Green?= =?UTF-8?q?=20highlight=20follows=20the=20persisted=20active=20model,=20no?= =?UTF-8?q?t=20the=20cursor=20position=20-=20Prefer=20provider=20entry.mod?= =?UTF-8?q?el=20over=20global=20cfg.model=20when=20resolving=20active=20mo?= =?UTF-8?q?del=20-=20Deleting=20a=20non-active=20model=20keeps=20the=20cur?= =?UTF-8?q?rent=20green=20highlight=20unchanged=20Delete=20and=20navigatio?= =?UTF-8?q?n:=20-=20Delete=20custom=20providers=20(d)=20and=20custom=20mod?= =?UTF-8?q?els=20(d)=20with=20confirmation=20prompts=20-=20Persist=20creat?= =?UTF-8?q?e/edit/model-select/add/delete=20changes=20to=20disk=20during?= =?UTF-8?q?=20the=20session=20-=20Fix=20custom=20provider=20deletion=20not?= =?UTF-8?q?=20surviving=20Esc=20exe-entry=20Tests:=20-=20Add=20coverage=20?= =?UTF-8?q?for=20model=20highlight,=20delete-model=20behavior,=20and=20cre?= =?UTF-8?q?ate=E2=86=92model-list=20flow=20-=20Update=20manual/custom=20fo?= =?UTF-8?q?rm=20tests=20for=20masked=20token=20and=20session=20save=20beha?= =?UTF-8?q?vior?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cmd/opencodereview/config_cmd.go | 27 + cmd/opencodereview/provider_cmd.go | 117 +-- cmd/opencodereview/provider_tui.go | 952 +++++++++++++++++++----- cmd/opencodereview/provider_tui_test.go | 181 ++++- 4 files changed, 1041 insertions(+), 236 deletions(-) diff --git a/cmd/opencodereview/config_cmd.go b/cmd/opencodereview/config_cmd.go index ccd3afe..0811920 100644 --- a/cmd/opencodereview/config_cmd.go +++ b/cmd/opencodereview/config_cmd.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "sort" "strconv" "strings" @@ -370,6 +371,32 @@ func parseModelListValue(value string) ([]string, error) { return normalizeModelList(strings.Split(value, ",")), nil } +func activeModelForProvider(cfg *Config, providerName string, entry ProviderEntry) string { + if entry.Model != "" { + return entry.Model + } + if cfg != nil && cfg.Provider == providerName && cfg.Model != "" { + return cfg.Model + } + return "" +} + +func sortModelsByName(models []string) []string { + if len(models) < 2 { + return models + } + sorted := append([]string(nil), models...) + sort.SliceStable(sorted, func(i, j int) bool { + left := strings.ToLower(sorted[i]) + right := strings.ToLower(sorted[j]) + if left == right { + return sorted[i] < sorted[j] + } + return left < right + }) + return sorted +} + func normalizeModelList(models []string) []string { out := make([]string, 0, len(models)) seen := make(map[string]struct{}, len(models)) diff --git a/cmd/opencodereview/provider_cmd.go b/cmd/opencodereview/provider_cmd.go index 34a9c73..a005f1b 100644 --- a/cmd/opencodereview/provider_cmd.go +++ b/cmd/opencodereview/provider_cmd.go @@ -22,7 +22,7 @@ func runConfigProvider() error { return fmt.Errorf("load config: %w", err) } - m := newProviderTUI(cfg) + m := newProviderTUI(cfg, configPath) p := tea.NewProgram(m) finalModel, err := p.Run() if err != nil { @@ -31,31 +31,26 @@ func runConfigProvider() error { final := finalModel.(providerTUIModel) - if len(final.deletedProviders) > 0 { - clearedActive, err := applyProviderDeletions(configPath, cfg, final.deletedProviders) - if err != nil { - return err - } - if clearedActive && !final.confirmed { - fmt.Fprintf(os.Stderr, "[ocr] WARNING: active provider was deleted; 'provider' and 'model' have been cleared.\n") - fmt.Fprintf(os.Stderr, "[ocr] Run 'ocr config provider' to select a new provider.\n") - } - } - - if len(final.deletedModels) > 0 { - if err := applyModelDeletions(configPath, cfg, final.deletedModels); err != nil { - return err - } - } - if !final.confirmed { - if len(final.deletedProviders) > 0 || len(final.deletedModels) > 0 { + // TUI persists changes (create/edit/model/add/delete) directly to disk + // during the session, so the on-disk file is already up to date for any + // savedInSession operation. No additional post-TUI apply step is needed. + if final.savedInSession { return nil } fmt.Println("Cancelled.") return nil } + // Provider deletions are deferred to the final step (matching the TUI's + // in-memory list mutation, not its disk save, since deletes are not + // written through during the session). + if len(final.deletedProviders) > 0 { + if _, err := applyProviderDeletions(configPath, cfg, final.deletedProviders); err != nil { + return err + } + } + result := final.result() if result.isManual { @@ -89,41 +84,39 @@ func applyProviderDeletions(configPath string, cfg *Config, names []string) (boo } func applyModelDeletions(configPath string, cfg *Config, deletedModels map[string][]string) error { - if cfg.CustomProviders == nil { + if len(deletedModels) == 0 { return nil } - changed := false - for name, models := range deletedModels { - entry, ok := cfg.CustomProviders[name] - if !ok { - continue - } - original := entry.Models - entry.Models = removeModels(entry.Models, models) - modelsChanged := len(entry.Models) != len(original) - - entryChanged := modelsChanged - if modelListContains(models, entry.Model) { - entry.Model = "" - entryChanged = true - } - if cfg.Provider == name && modelListContains(models, cfg.Model) { - cfg.Model = "" - changed = true - } - if entryChanged { - cfg.CustomProviders[name] = entry - changed = true - for _, m := range original { - if !modelListContains(entry.Models, m) { - fmt.Printf("Deleted model %q from custom provider %q.\n", m, name) + if cfg.CustomProviders != nil { + for name, models := range deletedModels { + entry, ok := cfg.CustomProviders[name] + if !ok { + continue + } + original := entry.Models + entry.Models = removeModels(entry.Models, models) + modelsChanged := len(entry.Models) != len(original) + + entryChanged := modelsChanged + if modelListContains(models, entry.Model) { + entry.Model = "" + entryChanged = true + } + if cfg.Provider == name && modelListContains(models, cfg.Model) { + cfg.Model = "" + } + if entryChanged { + cfg.CustomProviders[name] = entry + for _, m := range original { + if !modelListContains(entry.Models, m) { + fmt.Printf("Deleted model %q from custom provider %q.\n", m, name) + } } } } } - if !changed { - return nil - } + // Always persist when deletions were requested; in-session TUI edits may + // have already applied changes to cfg, making a diff-based changed flag unreliable. return saveConfig(configPath, cfg) } @@ -213,15 +206,28 @@ func applyCustomProviderConfig(configPath string, cfg *Config, result providerTU } cfg.CustomProviders[result.provider] = entry - if cfg.Provider != result.provider { - cfg.Model = "" + if !result.isEdit { + cfg.Provider = result.provider + cfg.Model = result.model + } else if cfg.Provider == result.provider { + cfg.Model = result.model } - cfg.Provider = result.provider if err := saveConfig(configPath, cfg); err != nil { return err } + if result.isEdit { + if cfg.Provider == result.provider { + fmt.Printf("\nActive provider %q updated.\n", result.provider) + } else { + fmt.Printf("\nCustom provider %q updated (not currently active).\n", result.provider) + } + fmt.Printf("Model: %s\n", result.model) + fmt.Println("\nTip: run 'ocr config model' to switch model later.") + return nil + } + fmt.Printf("\nProvider set to: %s (custom)\n", result.provider) fmt.Printf("Model: %s\n", result.model) @@ -271,6 +277,7 @@ func applyOfficialProviderConfig(configPath string, cfg *Config, result provider cfg.Model = "" } cfg.Provider = result.provider + cfg.Model = result.model if err := saveConfig(configPath, cfg); err != nil { return err @@ -311,7 +318,7 @@ func runConfigModel() error { if preset, isPreset := llm.LookupProvider(cfg.Provider); isPreset { provider = preset if entry, ok := cfg.Providers[cfg.Provider]; ok { - currentModel = entry.Model + currentModel = activeModelForProvider(cfg, cfg.Provider, entry) provider.Models = mergeModelLists(provider.Models, entry.Models) } } else { @@ -320,15 +327,12 @@ func runConfigModel() error { if !ok { return fmt.Errorf("provider %q is not configured in custom_providers", cfg.Provider) } - currentModel = entry.Model + currentModel = activeModelForProvider(cfg, cfg.Provider, entry) provider.DisplayName = cfg.Provider + " (custom)" provider.Protocol = entry.Protocol provider.BaseURL = entry.URL provider.Models = mergeModelLists(entry.Models) } - if currentModel == "" { - currentModel = cfg.Model - } m := newModelTUI(provider, currentModel) p := tea.NewProgram(m) @@ -367,6 +371,7 @@ func runConfigModel() error { } cfg.Providers[cfg.Provider] = entry } + cfg.Model = selectedModel if err := saveConfig(configPath, cfg); err != nil { return err diff --git a/cmd/opencodereview/provider_tui.go b/cmd/opencodereview/provider_tui.go index 2f70e91..b115061 100644 --- a/cmd/opencodereview/provider_tui.go +++ b/cmd/opencodereview/provider_tui.go @@ -36,8 +36,6 @@ const ( cpStepName customProviderStep = iota cpStepProtocol cpStepBaseURL - cpStepModel - cpStepModels cpStepAPIKey cpStepAuthHeader ) @@ -59,15 +57,17 @@ type customProviderListItem struct { } type providerTUIResult struct { - provider string - model string - models []string - apiKey string - isCustom bool - isManual bool - url string - protocol string - authHeader string + provider string + model string + models []string + apiKey string + isCustom bool + isEdit bool + editTargetName string + isManual bool + url string + protocol string + authHeader string } type providerTUIModel struct { @@ -85,13 +85,13 @@ type providerTUIModel struct { customProviders []customProviderListItem customIdx int creatingCustom bool + editingCustom bool + editTargetName string cpStep customProviderStep cpProtocolIdx int - cpNameInput textinput.Model - cpURLInput textinput.Model - cpModelInput textinput.Model - cpModelsInput textinput.Model - cpAuthInput textinput.Model + cpNameInput textinput.Model + cpURLInput textinput.Model + cpAuthInput textinput.Model // --- tab: manual --- inManualForm bool @@ -112,9 +112,12 @@ type providerTUIModel struct { apiKeyMasked bool apiKeyOriginal string - existingCfg *Config - confirmed bool - cancelled bool + existingCfg *Config + configPath string + confirmed bool + cancelled bool + formError string + savedInSession bool // --- delete confirmation --- confirmingDelete bool @@ -126,6 +129,22 @@ type providerTUIModel struct { deletedModels map[string][]string } +func (m providerTUIModel) customProviderNameTaken(name string) bool { + if m.existingCfg == nil || m.existingCfg.CustomProviders == nil { + return false + } + _, exists := m.existingCfg.CustomProviders[name] + return exists +} + +func (m providerTUIModel) customProviderActiveModel(cp customProviderListItem) string { + if m.existingCfg == nil || m.existingCfg.Provider != cp.name { + return "" + } + entry := m.customProviderEntry(cp.name, cp.entry) + return activeModelForProvider(m.existingCfg, cp.name, entry) +} + func collectCustomProviders(cfg *Config) []customProviderListItem { if cfg == nil || cfg.CustomProviders == nil { return nil @@ -138,7 +157,7 @@ func collectCustomProviders(cfg *Config) []customProviderListItem { return out } -func newProviderTUI(cfg *Config) providerTUIModel { +func newProviderTUI(cfg *Config, configPath ...string) providerTUIModel { providers := llm.ListProviders() sort.SliceStable(providers, func(i, j int) bool { left := strings.ToLower(providers[i].DisplayName) @@ -150,8 +169,8 @@ func newProviderTUI(cfg *Config) providerTUIModel { }) mi := textinput.New() - mi.Placeholder = "model name" - mi.SetWidth(40) + mi.Placeholder = "model name(s), comma-separated" + mi.SetWidth(50) ai := textinput.New() ai.Placeholder = "paste your API key here" @@ -167,14 +186,6 @@ func newProviderTUI(cfg *Config) providerTUIModel { cpURL.Placeholder = "enter your API base URL" cpURL.SetWidth(50) - cpModel := textinput.New() - cpModel.Placeholder = "model name" - cpModel.SetWidth(40) - - cpModels := textinput.New() - cpModels.Placeholder = "optional comma-separated models" - cpModels.SetWidth(50) - cpAuth := textinput.New() cpAuth.Placeholder = "optional, leave empty for default (Authorization)" cpAuth.SetWidth(55) @@ -200,8 +211,6 @@ func newProviderTUI(cfg *Config) providerTUIModel { apiKeyInput: ai, cpNameInput: cpName, cpURLInput: cpURL, - cpModelInput: cpModel, - cpModelsInput: cpModels, cpAuthInput: cpAuth, manualURLInput: manualURL, manualModelInput: manualModel, @@ -210,6 +219,7 @@ func newProviderTUI(cfg *Config) providerTUIModel { height: 24, activeTab: tabOfficial, customProviders: collectCustomProviders(cfg), + configPath: configPathFromArgs(configPath), } providerFound := false @@ -260,6 +270,10 @@ func newProviderTUI(cfg *Config) providerTUIModel { if cfg.Provider == "" && cfg.Llm.URL != "" { m.activeTab = tabManual } + // Intentionally do not auto-switch activeTab to tabCustom when only custom + // providers exist — leave the cursor on Official so users navigate + // explicitly via Tab/Right. This also avoids treating custom-only setups + // as a hidden "active Manual" highlight via globalConfig(). if cfg.Llm.URL != "" { m.manualURLInput.SetValue(cfg.Llm.URL) @@ -320,10 +334,10 @@ func (m providerTUIModel) models() []string { models = mergeModelLists(models, entry.Models) } } - return models + return sortModelsByName(models) case tabCustom: if cp, ok := m.selectedCustomProvider(); ok { - return cp.entry.Models + return sortModelsByName(cp.entry.Models) } } return nil @@ -350,6 +364,63 @@ func (m *providerTUIModel) prepareModelSelection(currentModel string) { m.modelInput.SetValue(currentModel) } +func (m *providerTUIModel) customProviderEntry(name string, fallback ProviderEntry) ProviderEntry { + if m.existingCfg != nil { + if entry, ok := m.existingCfg.CustomProviders[name]; ok { + return entry + } + } + return fallback +} + +func (m *providerTUIModel) syncSessionModelSelection() error { + if m.existingCfg == nil { + return nil + } + model := m.selectedModelFromState() + if model == "" { + return nil + } + + switch m.activeTab { + case tabCustom: + cp, ok := m.selectedCustomProvider() + if !ok { + return nil + } + entry := m.customProviderEntry(cp.name, cp.entry) + entry.Model = model + if m.existingCfg.CustomProviders == nil { + m.existingCfg.CustomProviders = make(map[string]ProviderEntry) + } + m.existingCfg.CustomProviders[cp.name] = entry + cp.entry = entry + m.customProviders[m.customIdx] = cp + if m.existingCfg.Provider == cp.name { + m.existingCfg.Model = model + } + case tabOfficial: + provider := m.currentProvider() + if m.existingCfg.Providers == nil { + m.existingCfg.Providers = make(map[string]ProviderEntry) + } + entry := m.existingCfg.Providers[provider.Name] + entry.Model = model + m.existingCfg.Providers[provider.Name] = entry + if m.existingCfg.Provider == provider.Name { + m.existingCfg.Model = model + } + } + + if m.configPath != "" { + if err := saveConfig(m.configPath, m.existingCfg); err != nil { + return fmt.Errorf("Failed to save: %w", err) + } + } + m.savedInSession = true + return nil +} + func (m providerTUIModel) isCustomModelItem(idx int) bool { return idx == len(m.models()) } @@ -382,7 +453,7 @@ func (m providerTUIModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m.updateAPIKeyInput(key, msg) } - if m.step == stepProvider && m.creatingCustom { + if m.step == stepProvider && (m.creatingCustom || m.editingCustom) { return m.updateCustomProviderForm(key, msg) } @@ -457,10 +528,17 @@ func (m providerTUIModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } } return m, nil + + case "e": + if m.step == stepProvider && m.activeTab == tabCustom && !m.creatingCustom && m.customIdx < len(m.customProviders) { + m.enterEditCustomProvider() + return m, m.cpNameInput.Focus() + } + return m, nil } default: - if m.step == stepProvider && m.creatingCustom { + if m.step == stepProvider && (m.creatingCustom || m.editingCustom) { return m.passThroughCPInput(msg) } if m.step == stepProvider && m.inManualForm { @@ -488,13 +566,38 @@ func (m providerTUIModel) updateCustomModelInput(key string, msg tea.KeyPressMsg m.modelInput.SetValue("") return m, nil case "enter": - if m.modelInput.Value() != "" { - m.customModel = false - m.modelInput.Blur() - m.step = stepAPIKey - m.loadExistingAPIKey() - return m, m.apiKeyInput.Focus() + raw := strings.TrimSpace(m.modelInput.Value()) + if raw == "" { + return m, nil + } + // Accept comma-separated model names; trim whitespace and drop empties. + parts := strings.Split(raw, ",") + models := make([]string, 0, len(parts)) + seen := make(map[string]struct{}, len(parts)) + for _, p := range parts { + name := strings.TrimSpace(p) + if name == "" { + continue + } + if _, ok := seen[name]; ok { + continue + } + seen[name] = struct{}{} + models = append(models, name) + } + if len(models) == 0 { + return m, nil } + if err := m.addCustomModelsToSession(models); err != nil { + m.formError = err.Error() + return m, nil + } + m.customModel = false + m.modelInput.Blur() + m.modelInput.SetValue("") + // Reposition the cursor on the first newly-added model so the user + // can see what just landed. + m.refreshModelSelectionForCustom() return m, nil default: var cmd tea.Cmd @@ -503,6 +606,49 @@ func (m providerTUIModel) updateCustomModelInput(key string, msg tea.KeyPressMsg } } +// addCustomModelsToSession merges the given models into the current custom +// provider's Models list and persists in-memory state. It does not change the +// active model — the user picks that explicitly from the list afterwards. +func (m *providerTUIModel) addCustomModelsToSession(models []string) error { + if m.existingCfg == nil { + return nil + } + cp, ok := m.selectedCustomProvider() + if !ok { + return nil + } + entry := m.customProviderEntry(cp.name, cp.entry) + prevEntry := cloneProviderEntry(entry) + entry.Models = mergeModelLists(entry.Models, models) + if m.existingCfg.CustomProviders == nil { + m.existingCfg.CustomProviders = make(map[string]ProviderEntry) + } + m.existingCfg.CustomProviders[cp.name] = entry + cp.entry = entry + m.customProviders[m.customIdx] = cp + if m.configPath != "" { + if err := saveConfig(m.configPath, m.existingCfg); err != nil { + m.existingCfg.CustomProviders[cp.name] = prevEntry + cp.entry = prevEntry + m.customProviders[m.customIdx] = cp + return fmt.Errorf("failed to save models: %w", err) + } + } + m.savedInSession = true + return nil +} + +// refreshModelSelectionForCustom moves the cursor to "Enter custom model name..." +// after the user adds models via the input field. +func (m *providerTUIModel) refreshModelSelectionForCustom() { + models := m.models() + m.modelIdx = 0 + if len(models) == 0 { + return + } + m.modelIdx = len(models) // land on "Enter custom model name..." +} + func (m providerTUIModel) updateAPIKeyInput(key string, msg tea.KeyPressMsg) (tea.Model, tea.Cmd) { switch key { case "esc": @@ -538,11 +684,24 @@ func (m providerTUIModel) updateCustomProviderForm(key string, msg tea.KeyPressM case "esc": if m.cpStep == cpStepName { m.creatingCustom = false + m.editingCustom = false + m.editTargetName = "" m.cpNameInput.Blur() + m.cpNameInput.SetValue("") + m.cpURLInput.SetValue("") + m.cpAuthInput.SetValue("") + m.apiKeyInput.SetValue("") + m.apiKeyMasked = false + m.apiKeyOriginal = "" + m.formError = "" return m, nil } m.blurCPStep() - m.cpStep-- + if m.editingCustom && m.cpStep == cpStepAPIKey { + m.cpStep = cpStepBaseURL + } else { + m.cpStep-- + } return m, m.focusCPStep() case "enter": return m.handleCustomFormEnter() @@ -557,16 +716,65 @@ func (m providerTUIModel) updateCustomProviderForm(key string, msg tea.KeyPressM } return m, nil default: + if m.cpStep == cpStepAPIKey { + if m.apiKeyMasked { + if len(key) == 1 { + m.apiKeyMasked = false + m.apiKeyInput.SetValue("") + } else { + return m, nil + } + } + var cmd tea.Cmd + m.apiKeyInput, cmd = m.apiKeyInput.Update(msg) + return m, cmd + } return m.passThroughCPInput(msg) } } +func (m *providerTUIModel) enterEditCustomProvider() { + cp := m.customProviders[m.customIdx] + entry := m.customProviderEntry(cp.name, cp.entry) + m.editingCustom = true + m.editTargetName = cp.name + m.cpStep = cpStepName + m.formError = "" + protoIdx := 1 + if entry.Protocol == "anthropic" { + protoIdx = 0 + } + m.cpProtocolIdx = protoIdx + m.cpNameInput.SetValue(cp.name) + m.cpURLInput.SetValue(entry.URL) + m.cpAuthInput.SetValue(entry.AuthHeader) + if entry.APIKey != "" { + m.apiKeyOriginal = entry.APIKey + m.apiKeyMasked = true + m.apiKeyInput.SetValue(strings.Repeat("*", 20)) + } else { + m.apiKeyInput.SetValue("") + m.apiKeyMasked = false + m.apiKeyOriginal = "" + } +} + func (m providerTUIModel) handleCustomFormEnter() (tea.Model, tea.Cmd) { switch m.cpStep { case cpStepName: - if m.cpNameInput.Value() == "" { + name := m.cpNameInput.Value() + if name == "" { + return m, nil + } + if m.creatingCustom && m.customProviderNameTaken(name) { + m.formError = fmt.Sprintf(`Provider "%s" already exists`, name) + return m, nil + } + if m.editingCustom && name != m.editTargetName && m.customProviderNameTaken(name) { + m.formError = fmt.Sprintf(`Provider "%s" already exists`, name) return m, nil } + m.formError = "" m.cpNameInput.Blur() m.cpStep = cpStepProtocol return m, nil @@ -578,43 +786,228 @@ func (m providerTUIModel) handleCustomFormEnter() (tea.Model, tea.Cmd) { return m, nil } m.cpURLInput.Blur() - m.cpStep = cpStepModel - return m, m.cpModelInput.Focus() - case cpStepModel: - if m.cpModelInput.Value() == "" { - return m, nil - } - m.cpModelInput.Blur() - m.cpStep = cpStepModels - return m, m.cpModelsInput.Focus() - case cpStepModels: - m.cpModelsInput.Blur() m.cpStep = cpStepAPIKey - m.apiKeyInput.SetValue("") - m.apiKeyMasked = false - return m, m.apiKeyInput.Focus() + if m.creatingCustom { + m.apiKeyInput.SetValue("") + m.apiKeyMasked = false + } + return m, m.focusCPStep() case cpStepAPIKey: m.apiKeyInput.Blur() m.cpStep = cpStepAuthHeader return m, m.cpAuthInput.Focus() case cpStepAuthHeader: m.cpAuthInput.Blur() + if m.editingCustom { + r := m.result() + if m.applyEditCustomProviderSave() { + return m, nil + } + // Edit succeeded — drop the user into the model list for this provider. + m.editingCustom = false + m.editTargetName = "" + if idx := m.findCustomIdx(r.provider); idx >= 0 { + m.customIdx = idx + } + m.step = stepModel + m.prepareModelSelection(m.customProviderEntry(r.provider, ProviderEntry{}).Model) + return m, nil + } + if m.creatingCustom { + return m.applyCreateCustomProvider() + } m.confirmed = true return m, tea.Quit } return m, nil } +func (m providerTUIModel) applyCreateCustomProvider() (tea.Model, tea.Cmd) { + if m.existingCfg == nil { + m.formError = "Failed to save: config not loaded" + return m, nil + } + if m.configPath == "" { + m.formError = "Failed to save: config path not available" + return m, nil + } + r := m.result() + if r.provider == "" { + m.formError = "Provider name is required" + m.cpStep = cpStepName + return m, m.cpNameInput.Focus() + } + if m.customProviderNameTaken(r.provider) { + m.formError = fmt.Sprintf(`Provider "%s" already exists`, r.provider) + m.cpStep = cpStepName + return m, m.cpNameInput.Focus() + } + + if m.existingCfg.CustomProviders == nil { + m.existingCfg.CustomProviders = make(map[string]ProviderEntry) + } + + entry := ProviderEntry{ + URL: r.url, + Protocol: r.protocol, + AuthHeader: r.authHeader, + } + if r.apiKey != "" { + entry.APIKey = r.apiKey + } + m.existingCfg.CustomProviders[r.provider] = entry + + if err := saveConfig(m.configPath, m.existingCfg); err != nil { + m.formError = fmt.Sprintf("Failed to save: %v", err) + return m, nil + } + + m.customProviders = collectCustomProviders(m.existingCfg) + if idx := m.findCustomIdx(r.provider); idx >= 0 { + m.customIdx = idx + } + m.creatingCustom = false + m.cpNameInput.SetValue("") + m.cpURLInput.SetValue("") + m.cpAuthInput.SetValue("") + m.apiKeyInput.SetValue("") + m.apiKeyMasked = false + m.apiKeyOriginal = "" + m.formError = "" + m.cpStep = cpStepName + m.savedInSession = true + // Drop into the model selection step so the user picks/adds a model for + // the newly created provider right away. + m.step = stepModel + m.prepareModelSelection("") + return m, nil +} + +// applyEditCustomProviderSave persists the edited provider to disk. Returns +// true if a hard validation error stopped the save (caller should keep the +// user in the form); false means the save was applied successfully and the +// caller may navigate away (e.g. into the model list). +func cloneProviderEntry(v ProviderEntry) ProviderEntry { + out := ProviderEntry{ + APIKey: v.APIKey, + URL: v.URL, + Protocol: v.Protocol, + Model: v.Model, + Models: append([]string(nil), v.Models...), + AuthHeader: v.AuthHeader, + } + if v.ExtraBody != nil { + out.ExtraBody = make(map[string]any, len(v.ExtraBody)) + for k, val := range v.ExtraBody { + out.ExtraBody[k] = val + } + } + return out +} + +func cloneCustomProvidersMap(src map[string]ProviderEntry) map[string]ProviderEntry { + if src == nil { + return nil + } + out := make(map[string]ProviderEntry, len(src)) + for k, v := range src { + out[k] = cloneProviderEntry(v) + } + return out +} + +func cloneCustomProviderList(src []customProviderListItem) []customProviderListItem { + out := make([]customProviderListItem, len(src)) + for i, cp := range src { + out[i] = customProviderListItem{name: cp.name, entry: cloneProviderEntry(cp.entry)} + } + return out +} + +func (m *providerTUIModel) applyEditCustomProviderSave() bool { + if m.existingCfg == nil { + m.formError = "Failed to save: config not loaded" + return true + } + if m.configPath == "" { + m.formError = "Failed to save: config path not available" + return true + } + r := m.result() + backupProviders := cloneCustomProvidersMap(m.existingCfg.CustomProviders) + backupActiveProvider := m.existingCfg.Provider + backupActiveModel := m.existingCfg.Model + backupCustomList := cloneCustomProviderList(m.customProviders) + + if m.existingCfg.CustomProviders == nil { + m.existingCfg.CustomProviders = make(map[string]ProviderEntry) + } + entry := m.existingCfg.CustomProviders[r.editTargetName] + if r.model != "" { + entry.Model = r.model + } + if len(r.models) > 0 { + entry.Models = mergeModelLists([]string{r.model}, r.models) + } + // Optional fields are always applied so users can intentionally clear them. + // To detect "user cleared the API key" vs "user left it masked/untouched", + // apiKey is only overwritten when the user actively typed something. + entry.URL = r.url + entry.Protocol = r.protocol + entry.AuthHeader = r.authHeader + if r.apiKey != "" { + entry.APIKey = r.apiKey + } + // If name changed, delete old key + if r.editTargetName != "" && r.editTargetName != r.provider { + if _, exists := m.existingCfg.CustomProviders[r.provider]; exists { + m.formError = fmt.Sprintf(`Provider "%s" already exists`, r.provider) + return true + } + delete(m.existingCfg.CustomProviders, r.editTargetName) + if m.existingCfg.Provider == r.editTargetName { + m.existingCfg.Provider = r.provider + m.existingCfg.Model = "" + } + } + m.existingCfg.CustomProviders[r.provider] = entry + + if err := saveConfig(m.configPath, m.existingCfg); err != nil { + m.formError = fmt.Sprintf("Failed to save: %v", err) + if reloaded, reloadErr := loadOrCreateConfig(m.configPath); reloadErr == nil { + m.existingCfg = reloaded + m.customProviders = collectCustomProviders(reloaded) + } else { + m.existingCfg.CustomProviders = backupProviders + m.existingCfg.Provider = backupActiveProvider + m.existingCfg.Model = backupActiveModel + m.customProviders = backupCustomList + } + return true + } + m.customProviders = collectCustomProviders(m.existingCfg) + if idx := m.findCustomIdx(r.provider); idx >= 0 { + m.customIdx = idx + } + m.savedInSession = true + return false +} + +func (m providerTUIModel) findCustomIdx(name string) int { + for i, cp := range m.customProviders { + if cp.name == name { + return i + } + } + return -1 +} + func (m *providerTUIModel) blurCPStep() { switch m.cpStep { case cpStepName: m.cpNameInput.Blur() case cpStepBaseURL: m.cpURLInput.Blur() - case cpStepModel: - m.cpModelInput.Blur() - case cpStepModels: - m.cpModelsInput.Blur() case cpStepAPIKey: m.apiKeyInput.Blur() case cpStepAuthHeader: @@ -622,16 +1015,12 @@ func (m *providerTUIModel) blurCPStep() { } } -func (m providerTUIModel) focusCPStep() tea.Cmd { +func (m *providerTUIModel) focusCPStep() tea.Cmd { switch m.cpStep { case cpStepName: return m.cpNameInput.Focus() case cpStepBaseURL: return m.cpURLInput.Focus() - case cpStepModel: - return m.cpModelInput.Focus() - case cpStepModels: - return m.cpModelsInput.Focus() case cpStepAPIKey: return m.apiKeyInput.Focus() case cpStepAuthHeader: @@ -647,14 +1036,8 @@ func (m providerTUIModel) passThroughCPInput(msg tea.Msg) (tea.Model, tea.Cmd) { m.cpNameInput, cmd = m.cpNameInput.Update(msg) case cpStepBaseURL: m.cpURLInput, cmd = m.cpURLInput.Update(msg) - case cpStepModel: - m.cpModelInput, cmd = m.cpModelInput.Update(msg) - case cpStepModels: - m.cpModelsInput, cmd = m.cpModelsInput.Update(msg) case cpStepAPIKey: - if m.apiKeyMasked { - return m, nil - } + // masked unlock is handled in updateCustomProviderForm default branch m.apiKeyInput, cmd = m.apiKeyInput.Update(msg) case cpStepAuthHeader: m.cpAuthInput, cmd = m.cpAuthInput.Update(msg) @@ -735,6 +1118,27 @@ func (m providerTUIModel) updateDeleteConfirm(key string) (tea.Model, tea.Cmd) { if m.customIdx >= len(m.customProviders) && m.customIdx > 0 { m.customIdx = len(m.customProviders) - 1 } + if m.existingCfg != nil { + if m.existingCfg.CustomProviders != nil { + delete(m.existingCfg.CustomProviders, m.deleteTargetName) + } + if m.existingCfg.Provider == m.deleteTargetName { + m.existingCfg.Provider = "" + m.existingCfg.Model = "" + } + if m.configPath != "" { + if err := saveConfig(m.configPath, m.existingCfg); err != nil { + if reloaded, reloadErr := loadOrCreateConfig(m.configPath); reloadErr == nil { + m.existingCfg = reloaded + m.customProviders = collectCustomProviders(reloaded) + } + m.formError = fmt.Sprintf("Failed to save: %v", err) + m.confirmingDelete = false + return m, nil + } + } + } + m.savedInSession = true m.confirmingDelete = false return m, nil case "n", "N", "esc": @@ -757,19 +1161,47 @@ func (m providerTUIModel) updateDeleteModelConfirm(key string) (tea.Model, tea.C models := m.models() if m.modelIdx < len(models) { cp := m.customProviders[m.customIdx] + cp.entry.Models = removeModels(cp.entry.Models, []string{m.deleteModelName}) if cp.entry.Model == m.deleteModelName { cp.entry.Model = "" } - cp.entry.Models = removeFromSlice(cp.entry.Models, m.modelIdx) + if m.existingCfg != nil && m.existingCfg.Provider == cp.name && + m.existingCfg.Model == m.deleteModelName { + m.existingCfg.Model = "" + } m.customProviders[m.customIdx] = cp + if m.existingCfg != nil { + if m.existingCfg.CustomProviders == nil { + m.existingCfg.CustomProviders = make(map[string]ProviderEntry) + } + m.existingCfg.CustomProviders[cp.name] = cp.entry + } + if m.configPath != "" { + if err := saveConfig(m.configPath, m.existingCfg); err != nil { + if reloaded, reloadErr := loadOrCreateConfig(m.configPath); reloadErr == nil { + m.existingCfg = reloaded + m.customProviders = collectCustomProviders(reloaded) + } + m.formError = fmt.Sprintf("Failed to save: %v", err) + m.confirmingDeleteModel = false + return m, nil + } + } if m.deletedModels == nil { m.deletedModels = make(map[string][]string) } m.deletedModels[cp.name] = append(m.deletedModels[cp.name], m.deleteModelName) - if m.modelIdx >= len(cp.entry.Models) && m.modelIdx > 0 { - m.modelIdx = len(cp.entry.Models) - 1 + m.savedInSession = true + if len(m.models()) > 0 { + if remaining := len(m.models()); m.modelIdx >= remaining { + if remaining > 0 { + m.modelIdx = remaining - 1 + } else { + m.modelIdx = 0 + } } } + } m.confirmingDeleteModel = false return m, nil case "n", "N", "esc": @@ -859,8 +1291,8 @@ func (m providerTUIModel) handleEnter() (tea.Model, tea.Cmd) { m.step = stepModel currentModel := "" if m.existingCfg != nil { - if entry, ok := m.existingCfg.Providers[m.currentProvider().Name]; ok && entry.Model != "" { - currentModel = entry.Model + if entry, ok := m.existingCfg.Providers[m.currentProvider().Name]; ok { + currentModel = activeModelForProvider(m.existingCfg, m.currentProvider().Name, entry) } } m.prepareModelSelection(currentModel) @@ -872,10 +1304,9 @@ func (m providerTUIModel) handleEnter() (tea.Model, tea.Cmd) { m.creatingCustom = true m.cpStep = cpStepName m.cpProtocolIdx = 1 // default openai + m.formError = "" m.cpNameInput.SetValue("") m.cpURLInput.SetValue("") - m.cpModelInput.SetValue("") - m.cpModelsInput.SetValue("") m.cpAuthInput.SetValue("") m.apiKeyInput.SetValue("") m.apiKeyMasked = false @@ -883,7 +1314,8 @@ func (m providerTUIModel) handleEnter() (tea.Model, tea.Cmd) { } cp := m.customProviders[m.customIdx] m.step = stepModel - m.prepareModelSelection(cp.entry.Model) + entry := m.customProviderEntry(cp.name, cp.entry) + m.prepareModelSelection(activeModelForProvider(m.existingCfg, cp.name, entry)) return m, nil case tabManual: @@ -897,6 +1329,10 @@ func (m providerTUIModel) handleEnter() (tea.Model, tea.Cmd) { m.customModel = true return m, m.modelInput.Focus() } + if err := m.syncSessionModelSelection(); err != nil { + m.formError = err.Error() + return m, nil + } m.step = stepAPIKey m.loadExistingAPIKey() return m, m.apiKeyInput.Focus() @@ -958,6 +1394,13 @@ func (m providerTUIModel) handleDown() (tea.Model, tea.Cmd) { return m, nil } +func configPathFromArgs(args []string) string { + if len(args) > 0 { + return args[0] + } + return "" +} + func removeFromSlice(s []string, idx int) []string { if idx < 0 || idx >= len(s) { return s @@ -1022,22 +1465,31 @@ func (m providerTUIModel) result() providerTUIResult { } case tabCustom: - if m.creatingCustom { + if m.creatingCustom || m.editingCustom { protocol := cpProtocols[m.cpProtocolIdx] - models := mergeModelLists( - []string{m.cpModelInput.Value()}, - strings.Split(m.cpModelsInput.Value(), ","), - ) - return providerTUIResult{ - provider: m.cpNameInput.Value(), - model: m.cpModelInput.Value(), - models: models, - apiKey: m.apiKeyInput.Value(), - isCustom: true, - url: m.cpURLInput.Value(), - protocol: protocol, - authHeader: m.cpAuthInput.Value(), + apiKey := m.apiKeyInput.Value() + if m.apiKeyMasked { + apiKey = m.apiKeyOriginal } + r := providerTUIResult{ + provider: m.cpNameInput.Value(), + apiKey: apiKey, + isCustom: true, + isEdit: m.editingCustom, + editTargetName: m.editTargetName, + url: m.cpURLInput.Value(), + protocol: protocol, + authHeader: m.cpAuthInput.Value(), + } + // Models are managed in the model selection step, not in the + // create/edit form. Preserve existing model/models when editing. + if m.editingCustom { + if idx := m.findCustomIdx(m.editTargetName); idx >= 0 { + r.model = m.customProviders[idx].entry.Model + r.models = m.customProviders[idx].entry.Models + } + } + return r } if m.customIdx < len(m.customProviders) { cp := m.customProviders[m.customIdx] @@ -1081,6 +1533,131 @@ func (m providerTUIModel) result() providerTUIResult { return providerTUIResult{} } +type globalConfigInfo struct { + tab providerTab + officialIdx int + customIdx int + ok bool +} + +func (m providerTUIModel) globalConfig() globalConfigInfo { + if m.existingCfg == nil { + return globalConfigInfo{} + } + if m.existingCfg.Provider != "" { + for i, p := range m.providers { + if p.Name == m.existingCfg.Provider { + return globalConfigInfo{tab: tabOfficial, officialIdx: i, ok: true} + } + } + if idx := m.findCustomIdx(m.existingCfg.Provider); idx >= 0 { + return globalConfigInfo{tab: tabCustom, customIdx: idx, ok: true} + } + // Provider configured but not in list (e.g. deleted in-session). + return globalConfigInfo{tab: tabCustom, customIdx: -1, ok: true} + } + // No active provider. If a Manual URL is configured, treat Manual as the + // globally-active tab so the green highlight reflects the user's real + // configuration. + if m.existingCfg.Llm.URL != "" { + return globalConfigInfo{tab: tabManual, ok: true} + } + // No active provider and no manual URL. Don't claim any tab as globally + // active — keeps the green highlight strictly tied to a real config. + return globalConfigInfo{} +} + +func (m providerTUIModel) isViewingGlobalActiveProvider() bool { + global := m.globalConfig() + if !global.ok || global.tab == tabManual { + return false + } + switch global.tab { + case tabOfficial: + return m.activeTab == tabOfficial && m.officialIdx == global.officialIdx + case tabCustom: + return m.activeTab == tabCustom && global.customIdx >= 0 && + m.customIdx == global.customIdx && m.customIdx < len(m.customProviders) + } + return false +} + +func (m providerTUIModel) modelListActiveModelName() string { + if m.existingCfg == nil || m.step != stepModel { + return "" + } + models := m.models() + + switch m.activeTab { + case tabCustom: + cp, ok := m.selectedCustomProvider() + if !ok || m.existingCfg.Provider != cp.name { + return "" + } + entry := m.customProviderEntry(cp.name, cp.entry) + name := activeModelForProvider(m.existingCfg, cp.name, entry) + if name == "" || !modelListContains(models, name) { + return "" + } + return name + case tabOfficial: + if m.existingCfg.Provider == "" { + return "" + } + p := m.currentProvider() + if p.Name != m.existingCfg.Provider { + return "" + } + entry := m.existingCfg.Providers[p.Name] + name := activeModelForProvider(m.existingCfg, p.Name, entry) + if name == "" || !modelListContains(models, name) { + return "" + } + return name + } + return "" +} + +func (m providerTUIModel) globalActiveModelName() string { + if !m.isViewingGlobalActiveProvider() || m.existingCfg == nil || m.existingCfg.Provider == "" { + return "" + } + models := m.models() + if entry, ok := m.existingCfg.Providers[m.existingCfg.Provider]; ok { + name := activeModelForProvider(m.existingCfg, m.existingCfg.Provider, entry) + if name == "" || !modelListContains(models, name) { + return "" + } + return name + } + if entry, ok := m.existingCfg.CustomProviders[m.existingCfg.Provider]; ok { + name := activeModelForProvider(m.existingCfg, m.existingCfg.Provider, entry) + if name == "" || !modelListContains(models, name) { + return "" + } + return name + } + return "" +} + +func listCursorPrefix(isCursor bool) string { + if isCursor { + return " " + tuiCursorStyle.Render(tuiCursor) + " " + } + return " " +} + +func renderListName(name string, isCursor, isGlobal bool) string { + switch { + case isGlobal: + return tuiGlobalActiveStyle.Render(name) + case isCursor: + return tuiSelectedItemStyle.Render(name) + default: + return tuiItemStyle.Render(name) + } +} + // --- View --- func (m providerTUIModel) View() tea.View { @@ -1101,7 +1678,7 @@ func (m providerTUIModel) View() tea.View { return v } -func renderTabBar(active providerTab) string { +func renderTabBar(active providerTab, global globalConfigInfo) string { tabs := []struct { label string tab providerTab @@ -1113,9 +1690,17 @@ func renderTabBar(active providerTab) string { var parts []string for _, t := range tabs { - if t.tab == active { + isCursor := t.tab == active + isGlobal := global.ok && t.tab == global.tab + + switch { + case isCursor && isGlobal: + parts = append(parts, tuiCursorStyle.Render("◉")+" "+tuiGlobalActiveTabStyle.Render(t.label)) + case isCursor: parts = append(parts, tuiActiveTabStyle.Render("◉ "+t.label)) - } else { + case isGlobal: + parts = append(parts, tuiGlobalActiveTabStyle.Render("○ "+t.label)) + default: parts = append(parts, tuiInactiveTabStyle.Render("○ "+t.label)) } } @@ -1123,7 +1708,8 @@ func renderTabBar(active providerTab) string { } func (m providerTUIModel) viewProvider(s *strings.Builder) { - s.WriteString(renderTabBar(m.activeTab)) + global := m.globalConfig() + s.WriteString(renderTabBar(m.activeTab, global)) s.WriteString("\n\n") switch m.activeTab { @@ -1136,12 +1722,12 @@ func (m providerTUIModel) viewProvider(s *strings.Builder) { } s.WriteString("\n") - if m.creatingCustom || m.inManualForm { + if m.creatingCustom || m.editingCustom || m.inManualForm { s.WriteString(tuiHelpStyle.Render(" Enter Confirm · Esc Back")) } else if m.confirmingDelete { s.WriteString(tuiHelpStyle.Render(" y Confirm · n/Esc Cancel")) } else if m.activeTab == tabCustom && m.customIdx < len(m.customProviders) { - s.WriteString(tuiHelpStyle.Render(" Enter Select · d Delete · Tab/Arrow Navigate · Esc Cancel")) + s.WriteString(tuiHelpStyle.Render(" Enter Select · e Edit · d Delete · Tab/Arrow Navigate · Esc Cancel")) } else { s.WriteString(tuiHelpStyle.Render(" Enter to select · Tab/Arrow keys to navigate · Esc to cancel")) } @@ -1152,23 +1738,22 @@ func (m providerTUIModel) viewOfficialTab(s *strings.Builder) { s.WriteString(tuiTitleStyle.Render(" Select a provider")) s.WriteString("\n\n") + global := m.globalConfig() + globalIdx := -1 + if global.ok && global.tab == tabOfficial { + globalIdx = global.officialIdx + } + for i, p := range m.providers { - cursor := " " - if i == m.officialIdx { - cursor = " " + tuiCursorStyle.Render(tuiCursor) + " " - } - name := p.DisplayName - if i == m.officialIdx { - s.WriteString(cursor + tuiSelectedItemStyle.Render(name)) - } else { - s.WriteString(cursor + tuiItemStyle.Render(name)) - } + isCursor := i == m.officialIdx + isGlobal := i == globalIdx + s.WriteString(listCursorPrefix(isCursor) + renderListName(p.DisplayName, isCursor, isGlobal)) s.WriteString("\n") } } func (m providerTUIModel) viewCustomTab(s *strings.Builder) { - if m.creatingCustom { + if m.creatingCustom || m.editingCustom { m.viewCustomProviderForm(s) return } @@ -1176,22 +1761,20 @@ func (m providerTUIModel) viewCustomTab(s *strings.Builder) { s.WriteString(tuiTitleStyle.Render(" Select a provider")) s.WriteString("\n\n") + global := m.globalConfig() + globalIdx := -1 + if global.ok && global.tab == tabCustom && global.customIdx >= 0 { + globalIdx = global.customIdx + } + for i, cp := range m.customProviders { - cursor := " " - if i == m.customIdx { - cursor = " " + tuiCursorStyle.Render(tuiCursor) + " " - } - label := cp.name - if cp.entry.Model != "" { - label += " " + tuiDimStyle.Render("("+cp.entry.Model+")") - } - if i == m.customIdx { - s.WriteString(cursor + tuiSelectedItemStyle.Render(cp.name)) - if cp.entry.Model != "" { - s.WriteString(" " + tuiDimStyle.Render("("+cp.entry.Model+")")) - } - } else { - s.WriteString(cursor + label) + isCursor := i == m.customIdx + isGlobal := i == globalIdx + activeModel := m.customProviderActiveModel(cp) + + s.WriteString(listCursorPrefix(isCursor) + renderListName(cp.name, isCursor, isGlobal)) + if activeModel != "" { + s.WriteString(" " + tuiDimStyle.Render("("+activeModel+")")) } s.WriteString("\n") } @@ -1224,7 +1807,11 @@ func (m providerTUIModel) viewCustomTab(s *strings.Builder) { } func (m providerTUIModel) viewCustomProviderForm(s *strings.Builder) { - s.WriteString(tuiTitleStyle.Render(" Add Custom Provider")) + title := " Add Custom Provider" + if m.editingCustom { + title = fmt.Sprintf(" Edit Custom Provider (%s)", m.editTargetName) + } + s.WriteString(tuiTitleStyle.Render(title)) s.WriteString("\n\n") type field struct { @@ -1237,8 +1824,6 @@ func (m providerTUIModel) viewCustomProviderForm(s *strings.Builder) { {"Provider name", m.cpNameInput.Value(), m.cpStep == cpStepName}, {"Protocol", cpProtocols[m.cpProtocolIdx], m.cpStep == cpStepProtocol}, {"Base URL", m.cpURLInput.Value(), m.cpStep == cpStepBaseURL}, - {"Model", m.cpModelInput.Value(), m.cpStep == cpStepModel}, - {"Models", m.cpModelsInput.Value(), m.cpStep == cpStepModels}, {"API Key", strings.Repeat("*", len(m.apiKeyInput.Value())), m.cpStep == cpStepAPIKey}, {"Auth Header", m.cpAuthInput.Value(), m.cpStep == cpStepAuthHeader}, } @@ -1261,19 +1846,29 @@ func (m providerTUIModel) viewCustomProviderForm(s *strings.Builder) { } case cpStepBaseURL: s.WriteString(" " + m.cpURLInput.View() + "\n") - case cpStepModel: - s.WriteString(" " + m.cpModelInput.View() + "\n") - case cpStepModels: - s.WriteString(" " + m.cpModelsInput.View() + "\n") case cpStepAPIKey: s.WriteString(" " + m.apiKeyInput.View() + "\n") case cpStepAuthHeader: s.WriteString(" " + m.cpAuthInput.View() + "\n") } - } else if f.value != "" { - s.WriteString(" " + tuiDimStyle.Render(f.label+": "+f.value) + "\n") + } else { + display := f.value + if display == "" && f.label == "Auth Header" { + display = "(Authorization)" + } + if display == "" { + s.WriteString(" " + tuiDimStyle.Render(f.label+":") + "\n") + } else { + s.WriteString(" " + tuiDimStyle.Render(f.label+": "+display) + "\n") + } } } + + if m.formError != "" { + s.WriteString("\n") + s.WriteString(tuiErrorStyle.Render(" " + m.formError)) + s.WriteString("\n") + } } func (m providerTUIModel) viewManualTab(s *strings.Builder) { @@ -1341,29 +1936,22 @@ func (m providerTUIModel) viewModel(s *strings.Builder) { s.WriteString("\n\n") models := m.models() + globalModel := m.modelListActiveModelName() + for i, model := range models { - cursor := " " - if i == m.modelIdx { - cursor = " " + tuiCursorStyle.Render(tuiCursor) + " " - } - if i == m.modelIdx { - s.WriteString(cursor + tuiSelectedItemStyle.Render(model)) - } else { - s.WriteString(cursor + tuiItemStyle.Render(model)) - } + isCursor := i == m.modelIdx + isGlobal := globalModel != "" && model == globalModel + s.WriteString(listCursorPrefix(isCursor) + renderListName(model, isCursor, isGlobal)) s.WriteString("\n") } customIdx := len(models) - cursor := " " - if m.modelIdx == customIdx { - cursor = " " + tuiCursorStyle.Render(tuiCursor) + " " - } + isCursor := m.modelIdx == customIdx customLabel := "Enter custom model name..." - if m.modelIdx == customIdx { - s.WriteString(cursor + tuiSelectedItemStyle.Render(customLabel)) + if isCursor { + s.WriteString(listCursorPrefix(isCursor) + tuiSelectedItemStyle.Render(customLabel)) } else { - s.WriteString(cursor + tuiDimStyle.Render(customLabel)) + s.WriteString(listCursorPrefix(isCursor) + tuiDimStyle.Render(customLabel)) } s.WriteString("\n") @@ -1449,6 +2037,17 @@ var ( tuiInactiveTabStyle = lipgloss.NewStyle(). Foreground(lipgloss.Color("8")) + + tuiGlobalActiveStyle = lipgloss.NewStyle(). + Bold(true). + Foreground(lipgloss.Color("10")) + + tuiGlobalActiveTabStyle = lipgloss.NewStyle(). + Bold(true). + Foreground(lipgloss.Color("10")) + + tuiErrorStyle = lipgloss.NewStyle(). + Foreground(lipgloss.Color("9")) ) // --- Model-only TUI (for `ocr config model`) --- @@ -1462,6 +2061,7 @@ type modelTUIModel struct { modelIdx int customModel bool modelInput textinput.Model + activeModel string confirmed bool cancelled bool @@ -1469,20 +2069,21 @@ type modelTUIModel struct { func newModelTUI(provider llm.Provider, currentModel string) modelTUIModel { mi := textinput.New() - mi.Placeholder = "model name" - mi.SetWidth(40) + mi.Placeholder = "model name(s), comma-separated" + mi.SetWidth(50) m := modelTUIModel{ - provider: provider, - models: provider.Models, - width: 80, - height: 24, - modelInput: mi, + provider: provider, + models: sortModelsByName(provider.Models), + width: 80, + height: 24, + modelInput: mi, + activeModel: currentModel, } if currentModel != "" { found := false - for i, model := range provider.Models { + for i, model := range m.models { if model == currentModel { m.modelIdx = i found = true @@ -1490,7 +2091,7 @@ func newModelTUI(provider llm.Provider, currentModel string) modelTUIModel { } } if !found { - m.modelIdx = len(provider.Models) + m.modelIdx = len(m.models) m.modelInput.SetValue(currentModel) } } @@ -1593,29 +2194,24 @@ func (m modelTUIModel) View() tea.View { s.WriteString(tuiTitleStyle.Render(fmt.Sprintf(" Select a model (%s)", m.provider.DisplayName))) s.WriteString("\n\n") + isGlobalCustom := m.activeModel != "" && !modelListContains(m.models, m.activeModel) + for i, model := range m.models { - cursor := " " - if i == m.modelIdx { - cursor = " " + tuiCursorStyle.Render(tuiCursor) + " " - } - if i == m.modelIdx { - s.WriteString(cursor + tuiSelectedItemStyle.Render(model)) - } else { - s.WriteString(cursor + tuiItemStyle.Render(model)) - } + isCursor := i == m.modelIdx + isGlobal := m.activeModel != "" && model == m.activeModel + s.WriteString(listCursorPrefix(isCursor) + renderListName(model, isCursor, isGlobal)) s.WriteString("\n") } customIdx := len(m.models) - cursor := " " - if m.modelIdx == customIdx { - cursor = " " + tuiCursorStyle.Render(tuiCursor) + " " - } + isCursor := m.modelIdx == customIdx customLabel := "Enter custom model name..." - if m.modelIdx == customIdx { - s.WriteString(cursor + tuiSelectedItemStyle.Render(customLabel)) + if isGlobalCustom { + s.WriteString(listCursorPrefix(isCursor) + renderListName(customLabel, isCursor, true)) + } else if isCursor { + s.WriteString(listCursorPrefix(isCursor) + tuiSelectedItemStyle.Render(customLabel)) } else { - s.WriteString(cursor + tuiDimStyle.Render(customLabel)) + s.WriteString(listCursorPrefix(isCursor) + tuiDimStyle.Render(customLabel)) } s.WriteString("\n") diff --git a/cmd/opencodereview/provider_tui_test.go b/cmd/opencodereview/provider_tui_test.go index 142f66c..494983e 100644 --- a/cmd/opencodereview/provider_tui_test.go +++ b/cmd/opencodereview/provider_tui_test.go @@ -1,6 +1,8 @@ package main import ( + "os" + "path/filepath" "sort" "strings" "testing" @@ -448,6 +450,107 @@ func TestProviderTUI_CustomFormEscFromNameExitsForm(t *testing.T) { } } +func TestProviderTUI_CustomFormRejectsDuplicateName(t *testing.T) { + cfg := &Config{ + Provider: "stepfun", + CustomProviders: map[string]ProviderEntry{ + "stepfun": {Model: "xxx"}, + }, + } + m := newProviderTUI(cfg) + + result, _ := m.Update(downKey()) + m2 := result.(providerTUIModel) + + result, _ = m2.Update(enterKey()) + m3 := result.(providerTUIModel) + if !m3.creatingCustom { + t.Fatal("should be creating custom") + } + + m3.cpNameInput.SetValue("stepfun") + result, _ = m3.Update(enterKey()) + m4 := result.(providerTUIModel) + if m4.cpStep != cpStepName { + t.Errorf("cpStep = %d, want %d", m4.cpStep, cpStepName) + } + if m4.formError == "" { + t.Error("expected formError for duplicate name") + } + if !strings.Contains(m4.formError, "stepfun") { + t.Errorf("formError = %q, want to mention stepfun", m4.formError) + } + + result, _ = m4.Update(charKey('x')) + m4b := result.(providerTUIModel) + if m4b.formError == "" { + t.Error("formError should persist while typing") + } + + m4b.cpNameInput.SetValue("stepfun2") + result, _ = m4b.Update(enterKey()) + m5 := result.(providerTUIModel) + if m5.cpStep != cpStepProtocol { + t.Errorf("cpStep = %d, want %d", m5.cpStep, cpStepProtocol) + } + if m5.formError != "" { + t.Errorf("formError = %q, want empty after valid name", m5.formError) + } +} + +func TestProviderTUI_CustomFormCreateReturnsToModelList(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config.json") + cfg := &Config{} + m := newProviderTUI(cfg, configPath) + + result, _ := m.Update(rightKey()) + m2 := result.(providerTUIModel) + result, _ = m2.Update(enterKey()) + m3 := result.(providerTUIModel) + + m3.cpNameInput.SetValue("my-new") + result, _ = m3.Update(enterKey()) // name -> protocol + m4 := result.(providerTUIModel) + result, _ = m4.Update(enterKey()) // protocol -> URL + m5 := result.(providerTUIModel) + m5.cpURLInput.SetValue("https://api.example.com") + result, _ = m5.Update(enterKey()) // URL -> API key + m6 := result.(providerTUIModel) + m6.apiKeyInput.SetValue("key-123") + result, _ = m6.Update(enterKey()) // API key -> auth header + m7 := result.(providerTUIModel) + result, cmd := m7.Update(enterKey()) // auth header -> save + m8 := result.(providerTUIModel) + + if cmd != nil { + t.Error("create should not quit TUI") + } + if m8.creatingCustom { + t.Error("creatingCustom should be false after create") + } + // Create should drop the user into the model selection step for the new + // provider so they can pick/add a model right away. + if m8.step != stepModel { + t.Errorf("step = %d, want stepModel", m8.step) + } + if len(m8.customProviders) != 1 { + t.Fatalf("expected 1 custom provider, got %d", len(m8.customProviders)) + } + if m8.customProviders[0].name != "my-new" { + t.Errorf("provider name = %q, want %q", m8.customProviders[0].name, "my-new") + } + if cfg.Provider != "" { + t.Error("active provider should not be set when only creating") + } + if !m8.savedInSession { + t.Error("savedInSession should be true after create") + } + if _, err := os.Stat(configPath); err != nil { + t.Fatalf("config should be saved: %v", err) + } +} + func TestProviderTUI_CustomProviderExistsInList(t *testing.T) { cfg := &Config{ Provider: "my-llm", @@ -494,8 +597,9 @@ func TestProviderTUI_SelectExistingCustomGoesToModel(t *testing.T) { if m2.step != stepModel { t.Errorf("step = %d, want %d (stepModel)", m2.step, stepModel) } - if m2.models()[0] != "custom-model" { - t.Errorf("first model = %q, want %q", m2.models()[0], "custom-model") + gotModels := m2.models() + if len(gotModels) != 2 || gotModels[0] != "custom-fast" || gotModels[1] != "custom-model" { + t.Errorf("models = %v, want [custom-fast custom-model] (sorted)", gotModels) } } @@ -609,6 +713,7 @@ func TestProviderTUI_DeleteCustomProviderCancel(t *testing.T) { } m := newProviderTUI(cfg) + // Force custom tab so this test is independent of init-time tab routing. // Switch to custom tab, select provider, press d result, _ := m.Update(rightKey()) m2 := result.(providerTUIModel) @@ -708,3 +813,75 @@ func TestProviderTUI_DeleteEscCancels(t *testing.T) { t.Error("no providers should be deleted after Esc") } } + +func TestActiveModelForProvider_PrefersEntryModel(t *testing.T) { + cfg := &Config{Provider: "stepfun", Model: "step-3.7-flash"} + entry := ProviderEntry{Model: "step-3.5-flash"} + got := activeModelForProvider(cfg, "stepfun", entry) + if got != "step-3.5-flash" { + t.Errorf("got %q, want step-3.5-flash", got) + } +} + +func TestActiveModelForProvider_FallsBackToCfgModel(t *testing.T) { + cfg := &Config{Provider: "stepfun", Model: "step-3.5-flash"} + entry := ProviderEntry{} + got := activeModelForProvider(cfg, "stepfun", entry) + if got != "step-3.5-flash" { + t.Errorf("got %q, want step-3.5-flash", got) + } +} + +func TestProviderTUI_ModelListActiveModelName(t *testing.T) { + cfg := &Config{ + Provider: "stepfun", + Model: "step-3.5-flash", + CustomProviders: map[string]ProviderEntry{ + "stepfun": { + Model: "step-3.5-flash", + Models: []string{"step-3.5-flash", "step-3.7-flash"}, + }, + }, + } + m := newProviderTUI(cfg) + m.activeTab = tabCustom + m.customIdx = 0 + m.step = stepModel + + if got := m.modelListActiveModelName(); got != "step-3.5-flash" { + t.Errorf("modelListActiveModelName = %q, want step-3.5-flash", got) + } +} + +func TestProviderTUI_DeleteModelPreservesActiveHighlight(t *testing.T) { + cfg := &Config{ + Provider: "stepfun", + Model: "step-3.5-flash", + CustomProviders: map[string]ProviderEntry{ + "stepfun": { + Model: "step-3.5-flash", + Models: []string{"step-3.5-flash", "aaa"}, + }, + }, + } + m := newProviderTUI(cfg) + m.activeTab = tabCustom + m.customIdx = 0 + m.step = stepModel + m.modelIdx = 1 // aaa + + m.confirmingDeleteModel = true + m.deleteModelName = "aaa" + result, _ := m.Update(yKey()) + m2 := result.(providerTUIModel) + + if m2.existingCfg.CustomProviders["stepfun"].Model != "step-3.5-flash" { + t.Errorf("entry.Model = %q, want step-3.5-flash", m2.existingCfg.CustomProviders["stepfun"].Model) + } + if m2.existingCfg.Model != "step-3.5-flash" { + t.Errorf("cfg.Model = %q, want step-3.5-flash", m2.existingCfg.Model) + } + if got := m2.modelListActiveModelName(); got != "step-3.5-flash" { + t.Errorf("modelListActiveModelName = %q, want step-3.5-flash", got) + } +} From ceaca90300ef992d0b238d35a95d9100c4ecfc9b Mon Sep 17 00:00:00 2001 From: wxw <1406771356@qq.com> Date: Thu, 25 Jun 2026 23:38:09 +0800 Subject: [PATCH 06/11] refactor: address provider TUI code review feedback - Remove dead helper removeFromSlice (superseded by removeModels) - Move misplaced doc comment to applyEditCustomProviderSave - Drop redundant applyProviderDeletions post-TUI call so provider deletions rely solely on the in-session save - Fix brace/indent drift in updateDeleteModelConfirm and cache the model list instead of recomputing m.models() twice --- cmd/opencodereview/provider_cmd.go | 9 --------- cmd/opencodereview/provider_tui.go | 31 +++++++----------------------- 2 files changed, 7 insertions(+), 33 deletions(-) diff --git a/cmd/opencodereview/provider_cmd.go b/cmd/opencodereview/provider_cmd.go index a005f1b..bba573b 100644 --- a/cmd/opencodereview/provider_cmd.go +++ b/cmd/opencodereview/provider_cmd.go @@ -42,15 +42,6 @@ func runConfigProvider() error { return nil } - // Provider deletions are deferred to the final step (matching the TUI's - // in-memory list mutation, not its disk save, since deletes are not - // written through during the session). - if len(final.deletedProviders) > 0 { - if _, err := applyProviderDeletions(configPath, cfg, final.deletedProviders); err != nil { - return err - } - } - result := final.result() if result.isManual { diff --git a/cmd/opencodereview/provider_tui.go b/cmd/opencodereview/provider_tui.go index b115061..ea5db0c 100644 --- a/cmd/opencodereview/provider_tui.go +++ b/cmd/opencodereview/provider_tui.go @@ -883,10 +883,9 @@ func (m providerTUIModel) applyCreateCustomProvider() (tea.Model, tea.Cmd) { return m, nil } -// applyEditCustomProviderSave persists the edited provider to disk. Returns -// true if a hard validation error stopped the save (caller should keep the -// user in the form); false means the save was applied successfully and the -// caller may navigate away (e.g. into the model list). +// cloneProviderEntry deep-copies a ProviderEntry so callers (rollback paths, +// map cloning) can safely mutate the returned value without aliasing the +// original's slice or map fields. func cloneProviderEntry(v ProviderEntry) ProviderEntry { out := ProviderEntry{ APIKey: v.APIKey, @@ -1187,21 +1186,15 @@ func (m providerTUIModel) updateDeleteModelConfirm(key string) (tea.Model, tea.C return m, nil } } - if m.deletedModels == nil { - m.deletedModels = make(map[string][]string) - } - m.deletedModels[cp.name] = append(m.deletedModels[cp.name], m.deleteModelName) - m.savedInSession = true - if len(m.models()) > 0 { - if remaining := len(m.models()); m.modelIdx >= remaining { - if remaining > 0 { - m.modelIdx = remaining - 1 + updated := m.models() + if m.modelIdx >= len(updated) { + if len(updated) > 0 { + m.modelIdx = len(updated) - 1 } else { m.modelIdx = 0 } } } - } m.confirmingDeleteModel = false return m, nil case "n", "N", "esc": @@ -1401,16 +1394,6 @@ func configPathFromArgs(args []string) string { return "" } -func removeFromSlice(s []string, idx int) []string { - if idx < 0 || idx >= len(s) { - return s - } - result := make([]string, 0, len(s)-1) - result = append(result, s[:idx]...) - result = append(result, s[idx+1:]...) - return result -} - func (m *providerTUIModel) loadExistingAPIKey() { m.apiKeyMasked = false m.apiKeyOriginal = "" From ba27fbe60420ebf57a3388af86ac3ff40421c05b Mon Sep 17 00:00:00 2001 From: wxw <1406771356@qq.com> Date: Fri, 26 Jun 2026 14:23:38 +0800 Subject: [PATCH 07/11] =?UTF-8?q?feat:=20refine=20provider=20TUI=20flows?= =?UTF-8?q?=20and=20manual=20config=20form=20Custom=20provider=20flow:=20-?= =?UTF-8?q?=20Default=20protocol=20to=20anthropic=20on=20the=20new-provide?= =?UTF-8?q?r=20form=20-=20Single-name=20model=20input;=20reject=20duplicat?= =?UTF-8?q?es=20with=20inline=20error=20and=20=20=20preserve=20the=20typed?= =?UTF-8?q?=20value=20so=20the=20user=20can=20edit=20instead=20of=20re-typ?= =?UTF-8?q?ing=20-=20Drop=20the=20global=20green=20highlight;=20cursor/blu?= =?UTF-8?q?e=20is=20the=20only=20selection=20cue=20Manual=20configuration?= =?UTF-8?q?=20form:=20-=20Add=20Auth=20Header=20step=20(URL=20=E2=86=92=20?= =?UTF-8?q?Protocol=20=E2=86=92=20Model=20=E2=86=92=20Auth=20Token=20?= =?UTF-8?q?=E2=86=92=20Auth=20Header)=20=20=20and=20persist=20it=20to=20Ll?= =?UTF-8?q?m.AuthHeader=20on=20confirm=20-=20Reorder=20so=20Auth=20Header?= =?UTF-8?q?=20is=20always=20entered=20last=20-=20Make=20Auth=20Token=20req?= =?UTF-8?q?uired;=20empty=20Enter=20stays=20on=20the=20field=20-=20Show=20?= =?UTF-8?q?every=20field's=20label=20on=20every=20render,=20even=20when=20?= =?UTF-8?q?empty,=20matching=20=20=20the=20custom-provider=20form=20style?= =?UTF-8?q?=20Tests:=20-=20Cover=20custom=20model=20input=20add=20/=20dupl?= =?UTF-8?q?icate=20paths=20-=20Cover=20manual=20form=20prefill=20of=20Llm.?= =?UTF-8?q?AuthHeader=20-=20Cover=20that=20deleting=20a=20non-active=20mod?= =?UTF-8?q?el=20keeps=20the=20active=20model=20intact?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cmd/opencodereview/provider_cmd.go | 1 + cmd/opencodereview/provider_tui.go | 400 ++++++++++-------------- cmd/opencodereview/provider_tui_test.go | 297 +++++++++++++++++- 3 files changed, 451 insertions(+), 247 deletions(-) diff --git a/cmd/opencodereview/provider_cmd.go b/cmd/opencodereview/provider_cmd.go index bba573b..44c21e0 100644 --- a/cmd/opencodereview/provider_cmd.go +++ b/cmd/opencodereview/provider_cmd.go @@ -139,6 +139,7 @@ func applyManualConfig(configPath string, cfg *Config, result providerTUIResult) cfg.Llm.URL = result.url cfg.Llm.Model = result.model cfg.Llm.AuthToken = result.apiKey + cfg.Llm.AuthHeader = result.authHeader if result.protocol == "anthropic" { useAnthropic := true cfg.Llm.UseAnthropic = &useAnthropic diff --git a/cmd/opencodereview/provider_tui.go b/cmd/opencodereview/provider_tui.go index ea5db0c..c716455 100644 --- a/cmd/opencodereview/provider_tui.go +++ b/cmd/opencodereview/provider_tui.go @@ -47,6 +47,7 @@ const ( manualStepProtocol manualStepModel manualStepAuthToken + manualStepAuthHeader ) var cpProtocols = []string{"anthropic", "openai"} @@ -95,13 +96,14 @@ type providerTUIModel struct { // --- tab: manual --- inManualForm bool - manualStep manualStep - manualProtocolIdx int - manualURLInput textinput.Model - manualModelInput textinput.Model - manualTokenInput textinput.Model - manualTokenMasked bool - manualTokenOriginal string + manualStep manualStep + manualProtocolIdx int + manualURLInput textinput.Model + manualModelInput textinput.Model + manualAuthHeaderInput textinput.Model + manualTokenInput textinput.Model + manualTokenMasked bool + manualTokenOriginal string // --- shared model/api-key steps (official + existing custom) --- modelIdx int @@ -169,7 +171,7 @@ func newProviderTUI(cfg *Config, configPath ...string) providerTUIModel { }) mi := textinput.New() - mi.Placeholder = "model name(s), comma-separated" + mi.Placeholder = "enter model name" mi.SetWidth(50) ai := textinput.New() @@ -198,6 +200,10 @@ func newProviderTUI(cfg *Config, configPath ...string) providerTUIModel { manualModel.Placeholder = "enter model name" manualModel.SetWidth(40) + manualAuthHeader := textinput.New() + manualAuthHeader.Placeholder = "optional, leave empty for default (Authorization)" + manualAuthHeader.SetWidth(55) + manualToken := textinput.New() manualToken.Placeholder = "enter your auth token" manualToken.SetWidth(50) @@ -205,16 +211,17 @@ func newProviderTUI(cfg *Config, configPath ...string) providerTUIModel { manualToken.EchoCharacter = '*' m := providerTUIModel{ - providers: providers, - existingCfg: cfg, - modelInput: mi, - apiKeyInput: ai, - cpNameInput: cpName, - cpURLInput: cpURL, - cpAuthInput: cpAuth, - manualURLInput: manualURL, - manualModelInput: manualModel, - manualTokenInput: manualToken, + providers: providers, + existingCfg: cfg, + modelInput: mi, + apiKeyInput: ai, + cpNameInput: cpName, + cpURLInput: cpURL, + cpAuthInput: cpAuth, + manualURLInput: manualURL, + manualModelInput: manualModel, + manualAuthHeaderInput: manualAuthHeader, + manualTokenInput: manualToken, width: 80, height: 24, activeTab: tabOfficial, @@ -272,12 +279,12 @@ func newProviderTUI(cfg *Config, configPath ...string) providerTUIModel { } // Intentionally do not auto-switch activeTab to tabCustom when only custom // providers exist — leave the cursor on Official so users navigate - // explicitly via Tab/Right. This also avoids treating custom-only setups - // as a hidden "active Manual" highlight via globalConfig(). + // explicitly via Tab/Right. if cfg.Llm.URL != "" { m.manualURLInput.SetValue(cfg.Llm.URL) m.manualModelInput.SetValue(cfg.Llm.Model) + m.manualAuthHeaderInput.SetValue(cfg.Llm.AuthHeader) if cfg.Llm.AuthToken != "" { m.manualTokenOriginal = cfg.Llm.AuthToken m.manualTokenMasked = true @@ -480,6 +487,7 @@ func (m providerTUIModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, tea.Quit } m.step-- + m.formError = "" return m, nil case "enter": @@ -495,6 +503,7 @@ func (m providerTUIModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if m.step == stepProvider { if m.activeTab > 0 { m.activeTab-- + m.formError = "" } } return m, nil @@ -503,6 +512,7 @@ func (m providerTUIModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if m.step == stepProvider { if m.activeTab < tabCount-1 { m.activeTab++ + m.formError = "" } } return m, nil @@ -510,6 +520,7 @@ func (m providerTUIModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case "tab": if m.step == stepProvider { m.activeTab = (m.activeTab + 1) % tabCount + m.formError = "" } return m, nil @@ -564,31 +575,21 @@ func (m providerTUIModel) updateCustomModelInput(key string, msg tea.KeyPressMsg m.customModel = false m.modelInput.Blur() m.modelInput.SetValue("") + m.formError = "" return m, nil case "enter": - raw := strings.TrimSpace(m.modelInput.Value()) - if raw == "" { + name := strings.TrimSpace(m.modelInput.Value()) + if name == "" { return m, nil } - // Accept comma-separated model names; trim whitespace and drop empties. - parts := strings.Split(raw, ",") - models := make([]string, 0, len(parts)) - seen := make(map[string]struct{}, len(parts)) - for _, p := range parts { - name := strings.TrimSpace(p) - if name == "" { - continue - } - if _, ok := seen[name]; ok { - continue + for _, existing := range m.models() { + if existing == name { + m.formError = fmt.Sprintf("Already in list: %s", name) + return m, nil } - seen[name] = struct{}{} - models = append(models, name) } - if len(models) == 0 { - return m, nil - } - if err := m.addCustomModelsToSession(models); err != nil { + m.formError = "" + if err := m.addCustomModelToSession(name); err != nil { m.formError = err.Error() return m, nil } @@ -602,14 +603,16 @@ func (m providerTUIModel) updateCustomModelInput(key string, msg tea.KeyPressMsg default: var cmd tea.Cmd m.modelInput, cmd = m.modelInput.Update(msg) + m.formError = "" return m, cmd } } -// addCustomModelsToSession merges the given models into the current custom -// provider's Models list and persists in-memory state. It does not change the -// active model — the user picks that explicitly from the list afterwards. -func (m *providerTUIModel) addCustomModelsToSession(models []string) error { +// addCustomModelToSession appends a single model name to the current custom +// provider's Models list and persists in-memory state to disk. It does not +// change the active model — the user picks that explicitly from the list +// afterwards. +func (m *providerTUIModel) addCustomModelToSession(name string) error { if m.existingCfg == nil { return nil } @@ -619,7 +622,7 @@ func (m *providerTUIModel) addCustomModelsToSession(models []string) error { } entry := m.customProviderEntry(cp.name, cp.entry) prevEntry := cloneProviderEntry(entry) - entry.Models = mergeModelLists(entry.Models, models) + entry.Models = append(entry.Models, name) if m.existingCfg.CustomProviders == nil { m.existingCfg.CustomProviders = make(map[string]ProviderEntry) } @@ -654,6 +657,7 @@ func (m providerTUIModel) updateAPIKeyInput(key string, msg tea.KeyPressMsg) (te case "esc": m.apiKeyInput.Blur() m.step = stepModel + m.formError = "" return m, nil case "enter": m.confirmed = true @@ -672,6 +676,7 @@ func (m providerTUIModel) updateAPIKeyInput(key string, msg tea.KeyPressMsg) (te } var cmd tea.Cmd m.apiKeyInput, cmd = m.apiKeyInput.Update(msg) + m.formError = "" return m, cmd } } @@ -702,20 +707,25 @@ func (m providerTUIModel) updateCustomProviderForm(key string, msg tea.KeyPressM } else { m.cpStep-- } + m.formError = "" return m, m.focusCPStep() case "enter": return m.handleCustomFormEnter() - case "up", "k": - if m.cpStep == cpStepProtocol && m.cpProtocolIdx > 0 { - m.cpProtocolIdx-- - } - return m, nil - case "down", "j": - if m.cpStep == cpStepProtocol && m.cpProtocolIdx < len(cpProtocols)-1 { - m.cpProtocolIdx++ - } - return m, nil default: + if m.cpStep == cpStepProtocol { + switch key { + case "up", "k": + if m.cpProtocolIdx > 0 { + m.cpProtocolIdx-- + } + return m, nil + case "down", "j": + if m.cpProtocolIdx < len(cpProtocols)-1 { + m.cpProtocolIdx++ + } + return m, nil + } + } if m.cpStep == cpStepAPIKey { if m.apiKeyMasked { if len(key) == 1 { @@ -759,6 +769,13 @@ func (m *providerTUIModel) enterEditCustomProvider() { } } +func authHeaderFormError(raw string) string { + return fmt.Sprintf( + "Unsupported Auth Header %q. Use 'authorization' (default), 'x-api-key', or leave empty.", + strings.TrimSpace(raw), + ) +} + func (m providerTUIModel) handleCustomFormEnter() (tea.Model, tea.Cmd) { switch m.cpStep { case cpStepName: @@ -797,6 +814,11 @@ func (m providerTUIModel) handleCustomFormEnter() (tea.Model, tea.Cmd) { m.cpStep = cpStepAuthHeader return m, m.cpAuthInput.Focus() case cpStepAuthHeader: + raw := m.cpAuthInput.Value() + if _, err := llm.NormalizeAuthHeader(raw); err != nil { + m.formError = authHeaderFormError(raw) + return m, nil + } m.cpAuthInput.Blur() if m.editingCustom { r := m.result() @@ -1041,6 +1063,9 @@ func (m providerTUIModel) passThroughCPInput(msg tea.Msg) (tea.Model, tea.Cmd) { case cpStepAuthHeader: m.cpAuthInput, cmd = m.cpAuthInput.Update(msg) } + if _, ok := msg.(tea.KeyPressMsg); ok { + m.formError = "" + } return m, cmd } @@ -1056,6 +1081,7 @@ func (m providerTUIModel) updateManualForm(key string, msg tea.KeyPressMsg) (tea if m.existingCfg != nil { m.manualURLInput.SetValue(m.existingCfg.Llm.URL) m.manualModelInput.SetValue(m.existingCfg.Llm.Model) + m.manualAuthHeaderInput.SetValue(m.existingCfg.Llm.AuthHeader) if m.existingCfg.Llm.AuthToken != "" { m.manualTokenOriginal = m.existingCfg.Llm.AuthToken m.manualTokenMasked = true @@ -1068,28 +1094,35 @@ func (m providerTUIModel) updateManualForm(key string, msg tea.KeyPressMsg) (tea } else { m.manualURLInput.SetValue("") m.manualModelInput.SetValue("") + m.manualAuthHeaderInput.SetValue("") m.manualTokenInput.SetValue("") m.manualTokenMasked = false m.manualTokenOriginal = "" } + m.formError = "" return m, nil } m.blurManualStep() m.manualStep-- + m.formError = "" return m, m.focusManualStep() case "enter": return m.handleManualFormEnter() - case "up", "k": - if m.manualStep == manualStepProtocol && m.manualProtocolIdx > 0 { - m.manualProtocolIdx-- - } - return m, nil - case "down", "j": - if m.manualStep == manualStepProtocol && m.manualProtocolIdx < len(cpProtocols)-1 { - m.manualProtocolIdx++ - } - return m, nil default: + if m.manualStep == manualStepProtocol { + switch key { + case "up", "k": + if m.manualProtocolIdx > 0 { + m.manualProtocolIdx-- + } + return m, nil + case "down", "j": + if m.manualProtocolIdx < len(cpProtocols)-1 { + m.manualProtocolIdx++ + } + return m, nil + } + } if m.manualStep == manualStepAuthToken && m.manualTokenMasked { if len(key) == 1 { m.manualTokenMasked = false @@ -1227,7 +1260,19 @@ func (m providerTUIModel) handleManualFormEnter() (tea.Model, tea.Cmd) { m.manualStep = manualStepAuthToken return m, m.manualTokenInput.Focus() case manualStepAuthToken: + if m.manualTokenInput.Value() == "" && m.manualTokenOriginal == "" { + return m, nil + } m.manualTokenInput.Blur() + m.manualStep = manualStepAuthHeader + return m, m.manualAuthHeaderInput.Focus() + case manualStepAuthHeader: + raw := m.manualAuthHeaderInput.Value() + if _, err := llm.NormalizeAuthHeader(raw); err != nil { + m.formError = authHeaderFormError(raw) + return m, nil + } + m.manualAuthHeaderInput.Blur() m.confirmed = true return m, tea.Quit } @@ -1244,6 +1289,8 @@ func (m *providerTUIModel) blurManualStep() { m.manualModelInput.Blur() case manualStepAuthToken: m.manualTokenInput.Blur() + case manualStepAuthHeader: + m.manualAuthHeaderInput.Blur() } } @@ -1257,6 +1304,8 @@ func (m providerTUIModel) focusManualStep() tea.Cmd { return m.manualModelInput.Focus() case manualStepAuthToken: return m.manualTokenInput.Focus() + case manualStepAuthHeader: + return m.manualAuthHeaderInput.Focus() } return nil } @@ -1272,6 +1321,11 @@ func (m providerTUIModel) passThroughManualInput(msg tea.Msg) (tea.Model, tea.Cm m.manualModelInput, cmd = m.manualModelInput.Update(msg) case manualStepAuthToken: m.manualTokenInput, cmd = m.manualTokenInput.Update(msg) + case manualStepAuthHeader: + m.manualAuthHeaderInput, cmd = m.manualAuthHeaderInput.Update(msg) + } + if _, ok := msg.(tea.KeyPressMsg); ok { + m.formError = "" } return m, cmd } @@ -1296,7 +1350,7 @@ func (m providerTUIModel) handleEnter() (tea.Model, tea.Cmd) { if m.customIdx == addIdx { m.creatingCustom = true m.cpStep = cpStepName - m.cpProtocolIdx = 1 // default openai + m.cpProtocolIdx = 0 // default anthropic m.formError = "" m.cpNameInput.SetValue("") m.cpURLInput.SetValue("") @@ -1327,6 +1381,7 @@ func (m providerTUIModel) handleEnter() (tea.Model, tea.Cmd) { return m, nil } m.step = stepAPIKey + m.formError = "" m.loadExistingAPIKey() return m, m.apiKeyInput.Focus() } @@ -1454,6 +1509,7 @@ func (m providerTUIModel) result() providerTUIResult { if m.apiKeyMasked { apiKey = m.apiKeyOriginal } + authHeader, _ := llm.NormalizeAuthHeader(m.cpAuthInput.Value()) r := providerTUIResult{ provider: m.cpNameInput.Value(), apiKey: apiKey, @@ -1462,7 +1518,7 @@ func (m providerTUIModel) result() providerTUIResult { editTargetName: m.editTargetName, url: m.cpURLInput.Value(), protocol: protocol, - authHeader: m.cpAuthInput.Value(), + authHeader: authHeader, } // Models are managed in the model selection step, not in the // create/edit form. Preserve existing model/models when editing. @@ -1501,128 +1557,23 @@ func (m providerTUIModel) result() providerTUIResult { case tabManual: apiKey := m.manualTokenInput.Value() - if m.manualTokenMasked { + if m.manualTokenMasked || (apiKey == "" && m.manualTokenOriginal != "") { apiKey = m.manualTokenOriginal } + authHeader, _ := llm.NormalizeAuthHeader(m.manualAuthHeaderInput.Value()) return providerTUIResult{ - isManual: true, - url: m.manualURLInput.Value(), - model: m.manualModelInput.Value(), - apiKey: apiKey, - protocol: cpProtocols[m.manualProtocolIdx], + isManual: true, + url: m.manualURLInput.Value(), + model: m.manualModelInput.Value(), + apiKey: apiKey, + protocol: cpProtocols[m.manualProtocolIdx], + authHeader: authHeader, } } return providerTUIResult{} } -type globalConfigInfo struct { - tab providerTab - officialIdx int - customIdx int - ok bool -} - -func (m providerTUIModel) globalConfig() globalConfigInfo { - if m.existingCfg == nil { - return globalConfigInfo{} - } - if m.existingCfg.Provider != "" { - for i, p := range m.providers { - if p.Name == m.existingCfg.Provider { - return globalConfigInfo{tab: tabOfficial, officialIdx: i, ok: true} - } - } - if idx := m.findCustomIdx(m.existingCfg.Provider); idx >= 0 { - return globalConfigInfo{tab: tabCustom, customIdx: idx, ok: true} - } - // Provider configured but not in list (e.g. deleted in-session). - return globalConfigInfo{tab: tabCustom, customIdx: -1, ok: true} - } - // No active provider. If a Manual URL is configured, treat Manual as the - // globally-active tab so the green highlight reflects the user's real - // configuration. - if m.existingCfg.Llm.URL != "" { - return globalConfigInfo{tab: tabManual, ok: true} - } - // No active provider and no manual URL. Don't claim any tab as globally - // active — keeps the green highlight strictly tied to a real config. - return globalConfigInfo{} -} - -func (m providerTUIModel) isViewingGlobalActiveProvider() bool { - global := m.globalConfig() - if !global.ok || global.tab == tabManual { - return false - } - switch global.tab { - case tabOfficial: - return m.activeTab == tabOfficial && m.officialIdx == global.officialIdx - case tabCustom: - return m.activeTab == tabCustom && global.customIdx >= 0 && - m.customIdx == global.customIdx && m.customIdx < len(m.customProviders) - } - return false -} - -func (m providerTUIModel) modelListActiveModelName() string { - if m.existingCfg == nil || m.step != stepModel { - return "" - } - models := m.models() - - switch m.activeTab { - case tabCustom: - cp, ok := m.selectedCustomProvider() - if !ok || m.existingCfg.Provider != cp.name { - return "" - } - entry := m.customProviderEntry(cp.name, cp.entry) - name := activeModelForProvider(m.existingCfg, cp.name, entry) - if name == "" || !modelListContains(models, name) { - return "" - } - return name - case tabOfficial: - if m.existingCfg.Provider == "" { - return "" - } - p := m.currentProvider() - if p.Name != m.existingCfg.Provider { - return "" - } - entry := m.existingCfg.Providers[p.Name] - name := activeModelForProvider(m.existingCfg, p.Name, entry) - if name == "" || !modelListContains(models, name) { - return "" - } - return name - } - return "" -} - -func (m providerTUIModel) globalActiveModelName() string { - if !m.isViewingGlobalActiveProvider() || m.existingCfg == nil || m.existingCfg.Provider == "" { - return "" - } - models := m.models() - if entry, ok := m.existingCfg.Providers[m.existingCfg.Provider]; ok { - name := activeModelForProvider(m.existingCfg, m.existingCfg.Provider, entry) - if name == "" || !modelListContains(models, name) { - return "" - } - return name - } - if entry, ok := m.existingCfg.CustomProviders[m.existingCfg.Provider]; ok { - name := activeModelForProvider(m.existingCfg, m.existingCfg.Provider, entry) - if name == "" || !modelListContains(models, name) { - return "" - } - return name - } - return "" -} - func listCursorPrefix(isCursor bool) string { if isCursor { return " " + tuiCursorStyle.Render(tuiCursor) + " " @@ -1630,15 +1581,11 @@ func listCursorPrefix(isCursor bool) string { return " " } -func renderListName(name string, isCursor, isGlobal bool) string { - switch { - case isGlobal: - return tuiGlobalActiveStyle.Render(name) - case isCursor: +func renderListName(name string, isCursor bool) string { + if isCursor { return tuiSelectedItemStyle.Render(name) - default: - return tuiItemStyle.Render(name) } + return tuiItemStyle.Render(name) } // --- View --- @@ -1661,7 +1608,7 @@ func (m providerTUIModel) View() tea.View { return v } -func renderTabBar(active providerTab, global globalConfigInfo) string { +func renderTabBar(active providerTab) string { tabs := []struct { label string tab providerTab @@ -1673,17 +1620,9 @@ func renderTabBar(active providerTab, global globalConfigInfo) string { var parts []string for _, t := range tabs { - isCursor := t.tab == active - isGlobal := global.ok && t.tab == global.tab - - switch { - case isCursor && isGlobal: - parts = append(parts, tuiCursorStyle.Render("◉")+" "+tuiGlobalActiveTabStyle.Render(t.label)) - case isCursor: + if t.tab == active { parts = append(parts, tuiActiveTabStyle.Render("◉ "+t.label)) - case isGlobal: - parts = append(parts, tuiGlobalActiveTabStyle.Render("○ "+t.label)) - default: + } else { parts = append(parts, tuiInactiveTabStyle.Render("○ "+t.label)) } } @@ -1691,8 +1630,7 @@ func renderTabBar(active providerTab, global globalConfigInfo) string { } func (m providerTUIModel) viewProvider(s *strings.Builder) { - global := m.globalConfig() - s.WriteString(renderTabBar(m.activeTab, global)) + s.WriteString(renderTabBar(m.activeTab)) s.WriteString("\n\n") switch m.activeTab { @@ -1721,16 +1659,9 @@ func (m providerTUIModel) viewOfficialTab(s *strings.Builder) { s.WriteString(tuiTitleStyle.Render(" Select a provider")) s.WriteString("\n\n") - global := m.globalConfig() - globalIdx := -1 - if global.ok && global.tab == tabOfficial { - globalIdx = global.officialIdx - } - for i, p := range m.providers { isCursor := i == m.officialIdx - isGlobal := i == globalIdx - s.WriteString(listCursorPrefix(isCursor) + renderListName(p.DisplayName, isCursor, isGlobal)) + s.WriteString(listCursorPrefix(isCursor) + renderListName(p.DisplayName, isCursor)) s.WriteString("\n") } } @@ -1744,18 +1675,11 @@ func (m providerTUIModel) viewCustomTab(s *strings.Builder) { s.WriteString(tuiTitleStyle.Render(" Select a provider")) s.WriteString("\n\n") - global := m.globalConfig() - globalIdx := -1 - if global.ok && global.tab == tabCustom && global.customIdx >= 0 { - globalIdx = global.customIdx - } - for i, cp := range m.customProviders { isCursor := i == m.customIdx - isGlobal := i == globalIdx activeModel := m.customProviderActiveModel(cp) - s.WriteString(listCursorPrefix(isCursor) + renderListName(cp.name, isCursor, isGlobal)) + s.WriteString(listCursorPrefix(isCursor) + renderListName(cp.name, isCursor)) if activeModel != "" { s.WriteString(" " + tuiDimStyle.Render("("+activeModel+")")) } @@ -1885,6 +1809,7 @@ func (m providerTUIModel) viewManualTab(s *strings.Builder) { {"Protocol", cpProtocols[m.manualProtocolIdx], m.manualStep == manualStepProtocol}, {"Model", m.manualModelInput.Value(), m.manualStep == manualStepModel}, {"Auth Token", strings.Repeat("*", len(m.manualTokenInput.Value())), m.manualStep == manualStepAuthToken}, + {"Auth Header", m.manualAuthHeaderInput.Value(), m.manualStep == manualStepAuthHeader}, } for _, f := range fields { @@ -1907,11 +1832,27 @@ func (m providerTUIModel) viewManualTab(s *strings.Builder) { s.WriteString(" " + m.manualModelInput.View() + "\n") case manualStepAuthToken: s.WriteString(" " + m.manualTokenInput.View() + "\n") + case manualStepAuthHeader: + s.WriteString(" " + m.manualAuthHeaderInput.View() + "\n") + } + } else { + display := f.value + if display == "" && f.label == "Auth Header" { + display = "(Authorization)" + } + if display == "" { + s.WriteString(" " + tuiDimStyle.Render(f.label+":") + "\n") + } else { + s.WriteString(" " + tuiDimStyle.Render(f.label+": "+display) + "\n") } - } else if f.value != "" { - s.WriteString(" " + tuiDimStyle.Render(f.label+": "+f.value) + "\n") } } + + if m.formError != "" { + s.WriteString("\n") + s.WriteString(tuiErrorStyle.Render(" " + m.formError)) + s.WriteString("\n") + } } func (m providerTUIModel) viewModel(s *strings.Builder) { @@ -1919,12 +1860,10 @@ func (m providerTUIModel) viewModel(s *strings.Builder) { s.WriteString("\n\n") models := m.models() - globalModel := m.modelListActiveModelName() for i, model := range models { isCursor := i == m.modelIdx - isGlobal := globalModel != "" && model == globalModel - s.WriteString(listCursorPrefix(isCursor) + renderListName(model, isCursor, isGlobal)) + s.WriteString(listCursorPrefix(isCursor) + renderListName(model, isCursor)) s.WriteString("\n") } @@ -1941,6 +1880,10 @@ func (m providerTUIModel) viewModel(s *strings.Builder) { if m.customModel { s.WriteString("\n") s.WriteString(" " + m.modelInput.View()) + if m.formError != "" { + s.WriteString("\n") + s.WriteString(" " + tuiErrorStyle.Render(m.formError)) + } s.WriteString("\n") } @@ -2021,14 +1964,6 @@ var ( tuiInactiveTabStyle = lipgloss.NewStyle(). Foreground(lipgloss.Color("8")) - tuiGlobalActiveStyle = lipgloss.NewStyle(). - Bold(true). - Foreground(lipgloss.Color("10")) - - tuiGlobalActiveTabStyle = lipgloss.NewStyle(). - Bold(true). - Foreground(lipgloss.Color("10")) - tuiErrorStyle = lipgloss.NewStyle(). Foreground(lipgloss.Color("9")) ) @@ -2177,21 +2112,16 @@ func (m modelTUIModel) View() tea.View { s.WriteString(tuiTitleStyle.Render(fmt.Sprintf(" Select a model (%s)", m.provider.DisplayName))) s.WriteString("\n\n") - isGlobalCustom := m.activeModel != "" && !modelListContains(m.models, m.activeModel) - for i, model := range m.models { isCursor := i == m.modelIdx - isGlobal := m.activeModel != "" && model == m.activeModel - s.WriteString(listCursorPrefix(isCursor) + renderListName(model, isCursor, isGlobal)) + s.WriteString(listCursorPrefix(isCursor) + renderListName(model, isCursor)) s.WriteString("\n") } customIdx := len(m.models) isCursor := m.modelIdx == customIdx customLabel := "Enter custom model name..." - if isGlobalCustom { - s.WriteString(listCursorPrefix(isCursor) + renderListName(customLabel, isCursor, true)) - } else if isCursor { + if isCursor { s.WriteString(listCursorPrefix(isCursor) + tuiSelectedItemStyle.Render(customLabel)) } else { s.WriteString(listCursorPrefix(isCursor) + tuiDimStyle.Render(customLabel)) diff --git a/cmd/opencodereview/provider_tui_test.go b/cmd/opencodereview/provider_tui_test.go index 494983e..fce0402 100644 --- a/cmd/opencodereview/provider_tui_test.go +++ b/cmd/opencodereview/provider_tui_test.go @@ -35,7 +35,7 @@ func tabKeyMsg() tea.KeyPressMsg { } func charKey(c rune) tea.KeyPressMsg { - return tea.KeyPressMsg{Code: c} + return tea.KeyPressMsg{Code: c, Text: string(c)} } // --- Tab switching tests --- @@ -391,6 +391,65 @@ func TestProviderTUI_ManualFormPrefilledWhenProviderSet(t *testing.T) { } } +func TestProviderTUI_ManualFormPrefillsAuthHeader(t *testing.T) { + cfg := &Config{ + Llm: LlmConfig{ + URL: "https://manual.example.com/v1", + Model: "manual-model", + AuthToken: "manual-token", + AuthHeader: "X-Custom-Auth", + }, + } + m := newProviderTUI(cfg) + + if got := m.manualAuthHeaderInput.Value(); got != "X-Custom-Auth" { + t.Errorf("manualAuthHeaderInput not prefilled: got %q, want %q", got, "X-Custom-Auth") + } +} + +func TestProviderTUI_ManualFormSkipsEmptyTokenWhenOriginalExists(t *testing.T) { + cfg := &Config{ + Llm: LlmConfig{ + URL: "https://example.com/v1", + Model: "test-model", + AuthToken: "token-123", + }, + } + m := newProviderTUI(cfg) + m.inManualForm = true + m.manualStep = manualStepAuthToken + m.manualTokenOriginal = "token-123" + m.manualTokenMasked = false + m.manualTokenInput.SetValue("") + m.manualTokenInput.Focus() + + result, _ := m.Update(enterKey()) + m2 := result.(providerTUIModel) + if m2.manualStep != manualStepAuthHeader { + t.Errorf("manualStep = %d, want %d", m2.manualStep, manualStepAuthHeader) + } + + m2.confirmed = true + r := m2.result() + if r.apiKey != "token-123" { + t.Errorf("result apiKey = %q, want %q", r.apiKey, "token-123") + } +} + +func TestProviderTUI_ManualFormRequiresTokenOnFirstSetup(t *testing.T) { + m := newProviderTUI(&Config{}) + m.inManualForm = true + m.manualStep = manualStepAuthToken + m.manualTokenInput.SetValue("") + m.manualTokenInput.Focus() + + result, _ := m.Update(enterKey()) + m2 := result.(providerTUIModel) + if m2.manualStep != manualStepAuthToken { + t.Errorf("should stay on auth token step, got %d", m2.manualStep) + } +} + // --- Custom tab tests --- func TestProviderTUI_CustomTabShowsAddOption(t *testing.T) { @@ -483,8 +542,8 @@ func TestProviderTUI_CustomFormRejectsDuplicateName(t *testing.T) { result, _ = m4.Update(charKey('x')) m4b := result.(providerTUIModel) - if m4b.formError == "" { - t.Error("formError should persist while typing") + if m4b.formError != "" { + t.Errorf("formError should clear on keystroke, got %q", m4b.formError) } m4b.cpNameInput.SetValue("stepfun2") @@ -498,6 +557,92 @@ func TestProviderTUI_CustomFormRejectsDuplicateName(t *testing.T) { } } +func TestProviderTUI_CustomFormRejectsInvalidAuthHeader(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config.json") + cfg := &Config{} + m := newProviderTUI(cfg, configPath) + + result, _ := m.Update(rightKey()) + m2 := result.(providerTUIModel) + result, _ = m2.Update(enterKey()) + m3 := result.(providerTUIModel) + + m3.cpNameInput.SetValue("my-new") + result, _ = m3.Update(enterKey()) + m4 := result.(providerTUIModel) + result, _ = m4.Update(enterKey()) + m5 := result.(providerTUIModel) + m5.cpURLInput.SetValue("https://api.example.com") + result, _ = m5.Update(enterKey()) + m6 := result.(providerTUIModel) + result, _ = m6.Update(enterKey()) + m7 := result.(providerTUIModel) + if m7.cpStep != cpStepAuthHeader { + t.Fatalf("cpStep = %d, want %d", m7.cpStep, cpStepAuthHeader) + } + + for _, c := range "bad-header" { + result, _ = m7.Update(charKey(c)) + m7 = result.(providerTUIModel) + } + result, _ = m7.Update(enterKey()) + m8 := result.(providerTUIModel) + + if m8.cpStep != cpStepAuthHeader { + t.Errorf("cpStep = %d, want %d", m8.cpStep, cpStepAuthHeader) + } + if m8.formError == "" { + t.Error("expected formError for invalid auth header") + } + if !strings.Contains(m8.formError, "Unsupported Auth Header") { + t.Errorf("formError = %q, want unsupported auth header message", m8.formError) + } + if !m8.creatingCustom { + t.Error("creatingCustom should remain true when validation fails") + } + if _, err := os.Stat(configPath); err == nil { + t.Error("config should not be saved for invalid auth header") + } +} + +func TestProviderTUI_CustomFormEditRejectsInvalidAuthHeader(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config.json") + cfg := &Config{ + CustomProviders: map[string]ProviderEntry{ + "stepfun": { + URL: "https://api.example.com", + Protocol: "anthropic", + AuthHeader: "authorization", + }, + }, + } + m := newProviderTUI(cfg, configPath) + m.activeTab = tabCustom + m.customIdx = 0 + m.enterEditCustomProvider() + m.cpStep = cpStepAuthHeader + m.cpAuthInput.SetValue("bad-header") + m.cpAuthInput.Focus() + + result, _ := m.Update(enterKey()) + m2 := result.(providerTUIModel) + + if m2.cpStep != cpStepAuthHeader { + t.Errorf("cpStep = %d, want %d", m2.cpStep, cpStepAuthHeader) + } + if m2.formError == "" { + t.Error("expected formError for invalid auth header") + } + if !m2.editingCustom { + t.Error("editingCustom should remain true when validation fails") + } + if got := cfg.CustomProviders["stepfun"].AuthHeader; got != "authorization" { + t.Errorf("AuthHeader = %q, want unchanged %q", got, "authorization") + } +} + func TestProviderTUI_CustomFormCreateReturnsToModelList(t *testing.T) { dir := t.TempDir() configPath := filepath.Join(dir, "config.json") @@ -832,28 +977,159 @@ func TestActiveModelForProvider_FallsBackToCfgModel(t *testing.T) { } } -func TestProviderTUI_ModelListActiveModelName(t *testing.T) { +func TestProviderTUI_CustomModelInput_AddsSingleName(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config.json") cfg := &Config{ Provider: "stepfun", Model: "step-3.5-flash", CustomProviders: map[string]ProviderEntry{ "stepfun": { + URL: "https://api.stepfun.com/v1", Model: "step-3.5-flash", - Models: []string{"step-3.5-flash", "step-3.7-flash"}, + Models: []string{"step-3.5-flash"}, }, }, } - m := newProviderTUI(cfg) + m := newProviderTUI(cfg, configPath) + m.activeTab = tabCustom + m.customIdx = 0 + m.step = stepModel + m.modelIdx = len(m.models()) // land on "Enter custom model name..." + m.customModel = true + m.modelInput.SetValue("newmodel") + m.modelInput.Focus() + + result, _ := m.Update(enterKey()) + m2 := result.(providerTUIModel) + + if m2.customModel { + t.Error("customModel should be cleared after Enter") + } + if m2.formError != "" { + t.Errorf("formError = %q, want empty", m2.formError) + } + got := m2.existingCfg.CustomProviders["stepfun"].Models + want := []string{"step-3.5-flash", "newmodel"} + if len(got) != len(want) || got[0] != want[0] || got[1] != want[1] { + t.Errorf("Models = %v, want %v", got, want) + } + if !m2.savedInSession { + t.Error("savedInSession should be true after add") + } + + diskCfg, err := loadOrCreateConfig(configPath) + if err != nil { + t.Fatalf("load disk config: %v", err) + } + diskModels := diskCfg.CustomProviders["stepfun"].Models + if len(diskModels) != 2 || diskModels[1] != "newmodel" { + t.Errorf("disk Models = %v, want last=step-3.5-flash,newmodel", diskModels) + } +} + +func TestProviderTUI_CustomModelInput_RejectsDuplicate(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config.json") + cfg := &Config{ + Provider: "stepfun", + Model: "step-3.5-flash", + CustomProviders: map[string]ProviderEntry{ + "stepfun": { + URL: "https://api.stepfun.com/v1", + Model: "step-3.5-flash", + Models: []string{"step-3.5-flash"}, + }, + }, + } + m := newProviderTUI(cfg, configPath) m.activeTab = tabCustom m.customIdx = 0 m.step = stepModel + m.modelIdx = len(m.models()) + m.customModel = true + m.modelInput.SetValue("step-3.5-flash") + m.modelInput.Focus() + + result, _ := m.Update(enterKey()) + m2 := result.(providerTUIModel) + + if !m2.customModel { + t.Error("customModel should stay true after duplicate reject") + } + if m2.formError != "Already in list: step-3.5-flash" { + t.Errorf("formError = %q, want %q", m2.formError, "Already in list: step-3.5-flash") + } + if m2.modelInput.Value() != "step-3.5-flash" { + t.Errorf("input should be preserved on dup; got %q", m2.modelInput.Value()) + } + if len(m2.existingCfg.CustomProviders["stepfun"].Models) != 1 { + t.Errorf("Models mutated: %v", m2.existingCfg.CustomProviders["stepfun"].Models) + } + if _, err := os.Stat(configPath); err == nil { + t.Errorf("disk file should not exist; duplicate did not persist") + } + if m2.savedInSession { + t.Error("savedInSession should be false after rejected duplicate") + } +} + +func TestProviderTUI_ManualFormPassesKToAuthHeaderInput(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config.json") + cfg := &Config{Llm: LlmConfig{URL: "https://example.com/v1", Model: "m", AuthToken: "k"}} + m := newProviderTUI(cfg, configPath) + m.activeTab = tabManual + m.inManualForm = true + m.manualStep = manualStepAuthHeader + m.manualAuthHeaderInput.Focus() + + result, _ := m.Update(charKey('x')) + m2 := result.(providerTUIModel) + result, _ = m2.Update(charKey('-')) + m3 := result.(providerTUIModel) + result, _ = m3.Update(charKey('a')) + m4 := result.(providerTUIModel) + result, _ = m4.Update(charKey('p')) + m5 := result.(providerTUIModel) + result, _ = m5.Update(charKey('i')) + m6 := result.(providerTUIModel) + result, _ = m6.Update(charKey('-')) + m7 := result.(providerTUIModel) + result, _ = m7.Update(charKey('k')) + m8 := result.(providerTUIModel) + result, _ = m8.Update(charKey('e')) + m9 := result.(providerTUIModel) + result, _ = m9.Update(charKey('y')) + m10 := result.(providerTUIModel) - if got := m.modelListActiveModelName(); got != "step-3.5-flash" { - t.Errorf("modelListActiveModelName = %q, want step-3.5-flash", got) + if got := m10.manualAuthHeaderInput.Value(); got != "x-api-key" { + t.Errorf("manualAuthHeaderInput.Value() = %q, want %q", got, "x-api-key") } } -func TestProviderTUI_DeleteModelPreservesActiveHighlight(t *testing.T) { +func TestProviderTUI_CustomFormPassesKToAuthHeaderInput(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config.json") + cfg := &Config{} + m := newProviderTUI(cfg, configPath) + m.creatingCustom = true + m.cpStep = cpStepAuthHeader + m.cpAuthInput.Focus() + + result, _ := m.Update(charKey('k')) + m2 := result.(providerTUIModel) + result, _ = m2.Update(charKey('e')) + m3 := result.(providerTUIModel) + result, _ = m3.Update(charKey('y')) + m4 := result.(providerTUIModel) + + if got := m4.cpAuthInput.Value(); got != "key" { + t.Errorf("cpAuthInput.Value() = %q, want %q", got, "key") + } +} + +func TestProviderTUI_DeleteModelPreservesActiveModel(t *testing.T) { cfg := &Config{ Provider: "stepfun", Model: "step-3.5-flash", @@ -881,7 +1157,4 @@ func TestProviderTUI_DeleteModelPreservesActiveHighlight(t *testing.T) { if m2.existingCfg.Model != "step-3.5-flash" { t.Errorf("cfg.Model = %q, want step-3.5-flash", m2.existingCfg.Model) } - if got := m2.modelListActiveModelName(); got != "step-3.5-flash" { - t.Errorf("modelListActiveModelName = %q, want step-3.5-flash", got) - } } From 605eca8c47103b1c74fb4744f9a43679c569448b Mon Sep 17 00:00:00 2001 From: wxw <1406771356@qq.com> Date: Fri, 26 Jun 2026 14:49:45 +0800 Subject: [PATCH 08/11] Improve provider TUI validation, persistence, and code clarity. Address code review feedback: align custom Auth Header validation with manual mode; fix savedInSession after model deletion; refactor applyEditCustomProviderSave to return error; remove dead applyModelDeletions/deletedModels; document ExtraBody shallow-copy limit. Also fix manual/custom form UX (token skip on edit, k key input, formError scoping, switch indentation). --- cmd/opencodereview/provider_cmd.go | 37 ------------------------- cmd/opencodereview/provider_tui.go | 17 ++++++------ cmd/opencodereview/provider_tui_test.go | 3 ++ 3 files changed, 12 insertions(+), 45 deletions(-) diff --git a/cmd/opencodereview/provider_cmd.go b/cmd/opencodereview/provider_cmd.go index 44c21e0..4936f5b 100644 --- a/cmd/opencodereview/provider_cmd.go +++ b/cmd/opencodereview/provider_cmd.go @@ -74,43 +74,6 @@ func applyProviderDeletions(configPath string, cfg *Config, names []string) (boo return clearedActive, nil } -func applyModelDeletions(configPath string, cfg *Config, deletedModels map[string][]string) error { - if len(deletedModels) == 0 { - return nil - } - if cfg.CustomProviders != nil { - for name, models := range deletedModels { - entry, ok := cfg.CustomProviders[name] - if !ok { - continue - } - original := entry.Models - entry.Models = removeModels(entry.Models, models) - modelsChanged := len(entry.Models) != len(original) - - entryChanged := modelsChanged - if modelListContains(models, entry.Model) { - entry.Model = "" - entryChanged = true - } - if cfg.Provider == name && modelListContains(models, cfg.Model) { - cfg.Model = "" - } - if entryChanged { - cfg.CustomProviders[name] = entry - for _, m := range original { - if !modelListContains(entry.Models, m) { - fmt.Printf("Deleted model %q from custom provider %q.\n", m, name) - } - } - } - } - } - // Always persist when deletions were requested; in-session TUI edits may - // have already applied changes to cfg, making a diff-based changed flag unreliable. - return saveConfig(configPath, cfg) -} - func removeModels(existing, toRemove []string) []string { removeSet := make(map[string]struct{}, len(toRemove)) for _, m := range toRemove { diff --git a/cmd/opencodereview/provider_tui.go b/cmd/opencodereview/provider_tui.go index c716455..b277eaa 100644 --- a/cmd/opencodereview/provider_tui.go +++ b/cmd/opencodereview/provider_tui.go @@ -128,7 +128,6 @@ type providerTUIModel struct { deletedProviders []string confirmingDeleteModel bool deleteModelName string - deletedModels map[string][]string } func (m providerTUIModel) customProviderNameTaken(name string) bool { @@ -822,7 +821,7 @@ func (m providerTUIModel) handleCustomFormEnter() (tea.Model, tea.Cmd) { m.cpAuthInput.Blur() if m.editingCustom { r := m.result() - if m.applyEditCustomProviderSave() { + if err := m.applyEditCustomProviderSave(); err != nil { return m, nil } // Edit succeeded — drop the user into the model list for this provider. @@ -920,6 +919,7 @@ func cloneProviderEntry(v ProviderEntry) ProviderEntry { if v.ExtraBody != nil { out.ExtraBody = make(map[string]any, len(v.ExtraBody)) for k, val := range v.ExtraBody { + // Shallow copy only: nested maps/slices inside val are not cloned. out.ExtraBody[k] = val } } @@ -945,14 +945,14 @@ func cloneCustomProviderList(src []customProviderListItem) []customProviderListI return out } -func (m *providerTUIModel) applyEditCustomProviderSave() bool { +func (m *providerTUIModel) applyEditCustomProviderSave() error { if m.existingCfg == nil { m.formError = "Failed to save: config not loaded" - return true + return fmt.Errorf("config not loaded") } if m.configPath == "" { m.formError = "Failed to save: config path not available" - return true + return fmt.Errorf("config path not available") } r := m.result() backupProviders := cloneCustomProvidersMap(m.existingCfg.CustomProviders) @@ -983,7 +983,7 @@ func (m *providerTUIModel) applyEditCustomProviderSave() bool { if r.editTargetName != "" && r.editTargetName != r.provider { if _, exists := m.existingCfg.CustomProviders[r.provider]; exists { m.formError = fmt.Sprintf(`Provider "%s" already exists`, r.provider) - return true + return fmt.Errorf("provider %q already exists", r.provider) } delete(m.existingCfg.CustomProviders, r.editTargetName) if m.existingCfg.Provider == r.editTargetName { @@ -1004,14 +1004,14 @@ func (m *providerTUIModel) applyEditCustomProviderSave() bool { m.existingCfg.Model = backupActiveModel m.customProviders = backupCustomList } - return true + return fmt.Errorf("save config: %w", err) } m.customProviders = collectCustomProviders(m.existingCfg) if idx := m.findCustomIdx(r.provider); idx >= 0 { m.customIdx = idx } m.savedInSession = true - return false + return nil } func (m providerTUIModel) findCustomIdx(name string) int { @@ -1228,6 +1228,7 @@ func (m providerTUIModel) updateDeleteModelConfirm(key string) (tea.Model, tea.C } } } + m.savedInSession = true m.confirmingDeleteModel = false return m, nil case "n", "N", "esc": diff --git a/cmd/opencodereview/provider_tui_test.go b/cmd/opencodereview/provider_tui_test.go index fce0402..c8bca3a 100644 --- a/cmd/opencodereview/provider_tui_test.go +++ b/cmd/opencodereview/provider_tui_test.go @@ -1157,4 +1157,7 @@ func TestProviderTUI_DeleteModelPreservesActiveModel(t *testing.T) { if m2.existingCfg.Model != "step-3.5-flash" { t.Errorf("cfg.Model = %q, want step-3.5-flash", m2.existingCfg.Model) } + if !m2.savedInSession { + t.Error("savedInSession should be true after deleting a model") + } } From 9e8ef01bfe7158aa955169988dc046513a92de96 Mon Sep 17 00:00:00 2001 From: wxw <1406771356@qq.com> Date: Fri, 26 Jun 2026 15:46:14 +0800 Subject: [PATCH 09/11] Fix provider/model TUI list ordering and add test coverage. Address review and UX feedback: remove model list sorting in provider and config model TUIs; preserve Models list order when selecting active model (ensureModelInList); add test for duplicate rename on custom provider edit. Includes prior review fixes for savedInSession, applyEditCustomProviderSave error return, and dead code removal. --- cmd/opencodereview/config_cmd.go | 30 ++++----- cmd/opencodereview/config_cmd_test.go | 20 ++++++ cmd/opencodereview/provider_cmd.go | 7 +- cmd/opencodereview/provider_tui.go | 11 ++-- cmd/opencodereview/provider_tui_test.go | 85 ++++++++++++++++++++++++- 5 files changed, 126 insertions(+), 27 deletions(-) diff --git a/cmd/opencodereview/config_cmd.go b/cmd/opencodereview/config_cmd.go index 0811920..8d95295 100644 --- a/cmd/opencodereview/config_cmd.go +++ b/cmd/opencodereview/config_cmd.go @@ -5,7 +5,6 @@ import ( "fmt" "os" "path/filepath" - "sort" "strconv" "strings" @@ -381,22 +380,6 @@ func activeModelForProvider(cfg *Config, providerName string, entry ProviderEntr return "" } -func sortModelsByName(models []string) []string { - if len(models) < 2 { - return models - } - sorted := append([]string(nil), models...) - sort.SliceStable(sorted, func(i, j int) bool { - left := strings.ToLower(sorted[i]) - right := strings.ToLower(sorted[j]) - if left == right { - return sorted[i] < sorted[j] - } - return left < right - }) - return sorted -} - func normalizeModelList(models []string) []string { out := make([]string, 0, len(models)) seen := make(map[string]struct{}, len(models)) @@ -422,6 +405,19 @@ func mergeModelLists(lists ...[]string) []string { return normalizeModelList(merged) } +// ensureModelInList appends model to the end when missing; never reorders existing entries. +func ensureModelInList(models []string, model string) []string { + model = strings.TrimSpace(model) + if model == "" { + return models + } + if modelListContains(models, model) { + return models + } + out := append([]string(nil), models...) + return append(out, model) +} + func modelListContains(models []string, target string) bool { target = strings.TrimSpace(target) for _, model := range models { diff --git a/cmd/opencodereview/config_cmd_test.go b/cmd/opencodereview/config_cmd_test.go index 875b2f6..741794c 100644 --- a/cmd/opencodereview/config_cmd_test.go +++ b/cmd/opencodereview/config_cmd_test.go @@ -339,3 +339,23 @@ func TestUnsetInvalidKey(t *testing.T) { }) } } + +func TestEnsureModelInList(t *testing.T) { + models := []string{"test-model", "test-model-2", "bbb", "aaa", "test-model-3"} + + got := ensureModelInList(models, "test-model-3") + if len(got) != len(models) { + t.Fatalf("existing model should not reorder: got %v", got) + } + for i := range models { + if got[i] != models[i] { + t.Errorf("models[%d] = %q, want %q", i, got[i], models[i]) + } + } + + got = ensureModelInList(models, "new-model") + want := append(append([]string(nil), models...), "new-model") + if len(got) != len(want) || got[len(got)-1] != "new-model" { + t.Errorf("new model should append: got %v, want %v", got, want) + } +} diff --git a/cmd/opencodereview/provider_cmd.go b/cmd/opencodereview/provider_cmd.go index 4936f5b..5a3ba70 100644 --- a/cmd/opencodereview/provider_cmd.go +++ b/cmd/opencodereview/provider_cmd.go @@ -145,8 +145,9 @@ func applyCustomProviderConfig(configPath string, cfg *Config, result providerTU entry := cfg.CustomProviders[result.provider] entry.Model = result.model if len(result.models) > 0 { - entry.Models = mergeModelLists([]string{result.model}, result.models) + entry.Models = append([]string(nil), result.models...) } + entry.Models = ensureModelInList(entry.Models, result.model) if result.url != "" { entry.URL = result.url } @@ -313,7 +314,7 @@ func runConfigModel() error { } entry := cfg.CustomProviders[cfg.Provider] entry.Model = selectedModel - entry.Models = mergeModelLists([]string{selectedModel}, entry.Models) + entry.Models = ensureModelInList(entry.Models, selectedModel) cfg.CustomProviders[cfg.Provider] = entry } else { if cfg.Providers == nil { @@ -322,7 +323,7 @@ func runConfigModel() error { entry := cfg.Providers[cfg.Provider] entry.Model = selectedModel if !modelListContains(provider.Models, selectedModel) { - entry.Models = mergeModelLists([]string{selectedModel}, entry.Models) + entry.Models = ensureModelInList(entry.Models, selectedModel) } cfg.Providers[cfg.Provider] = entry } diff --git a/cmd/opencodereview/provider_tui.go b/cmd/opencodereview/provider_tui.go index b277eaa..959e639 100644 --- a/cmd/opencodereview/provider_tui.go +++ b/cmd/opencodereview/provider_tui.go @@ -340,10 +340,10 @@ func (m providerTUIModel) models() []string { models = mergeModelLists(models, entry.Models) } } - return sortModelsByName(models) + return models case tabCustom: if cp, ok := m.selectedCustomProvider(); ok { - return sortModelsByName(cp.entry.Models) + return cp.entry.Models } } return nil @@ -968,8 +968,9 @@ func (m *providerTUIModel) applyEditCustomProviderSave() error { entry.Model = r.model } if len(r.models) > 0 { - entry.Models = mergeModelLists([]string{r.model}, r.models) + entry.Models = append([]string(nil), r.models...) } + entry.Models = ensureModelInList(entry.Models, r.model) // Optional fields are always applied so users can intentionally clear them. // To detect "user cleared the API key" vs "user left it masked/untouched", // apiKey is only overwritten when the user actively typed something. @@ -1546,7 +1547,7 @@ func (m providerTUIModel) result() providerTUIResult { return providerTUIResult{ provider: cp.name, model: model, - models: mergeModelLists([]string{model}, cp.entry.Models), + models: append([]string(nil), cp.entry.Models...), apiKey: apiKey, isCustom: true, url: cp.entry.URL, @@ -1993,7 +1994,7 @@ func newModelTUI(provider llm.Provider, currentModel string) modelTUIModel { m := modelTUIModel{ provider: provider, - models: sortModelsByName(provider.Models), + models: provider.Models, width: 80, height: 24, modelInput: mi, diff --git a/cmd/opencodereview/provider_tui_test.go b/cmd/opencodereview/provider_tui_test.go index c8bca3a..bf8e486 100644 --- a/cmd/opencodereview/provider_tui_test.go +++ b/cmd/opencodereview/provider_tui_test.go @@ -643,6 +643,44 @@ func TestProviderTUI_CustomFormEditRejectsInvalidAuthHeader(t *testing.T) { } } +func TestProviderTUI_EditCustomProviderSaveRejectsDuplicateRename(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config.json") + cfg := &Config{ + CustomProviders: map[string]ProviderEntry{ + "stepfun": { + URL: "https://stepfun.example.com", + Protocol: "anthropic", + }, + "other": { + URL: "https://other.example.com", + Protocol: "openai", + }, + }, + } + m := newProviderTUI(cfg, configPath) + m.activeTab = tabCustom + m.editingCustom = true + m.editTargetName = "other" + m.cpProtocolIdx = 1 // openai + m.cpNameInput.SetValue("stepfun") + m.cpURLInput.SetValue("https://other.example.com") + + err := m.applyEditCustomProviderSave() + if err == nil { + t.Fatal("expected error when renaming to existing provider name") + } + if !strings.Contains(m.formError, "stepfun") { + t.Errorf("formError = %q, want to mention stepfun", m.formError) + } + if _, ok := cfg.CustomProviders["other"]; !ok { + t.Error("original provider 'other' should still exist") + } + if cfg.CustomProviders["other"].URL != "https://other.example.com" { + t.Errorf("provider 'other' URL = %q, want unchanged", cfg.CustomProviders["other"].URL) + } +} + func TestProviderTUI_CustomFormCreateReturnsToModelList(t *testing.T) { dir := t.TempDir() configPath := filepath.Join(dir, "config.json") @@ -743,8 +781,8 @@ func TestProviderTUI_SelectExistingCustomGoesToModel(t *testing.T) { t.Errorf("step = %d, want %d (stepModel)", m2.step, stepModel) } gotModels := m2.models() - if len(gotModels) != 2 || gotModels[0] != "custom-fast" || gotModels[1] != "custom-model" { - t.Errorf("models = %v, want [custom-fast custom-model] (sorted)", gotModels) + if len(gotModels) != 2 || gotModels[0] != "custom-model" || gotModels[1] != "custom-fast" { + t.Errorf("models = %v, want [custom-model custom-fast] (config order)", gotModels) } } @@ -1161,3 +1199,46 @@ func TestProviderTUI_DeleteModelPreservesActiveModel(t *testing.T) { t.Error("savedInSession should be true after deleting a model") } } + +func TestApplyCustomProviderConfigPreservesModelOrder(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config.json") + models := []string{"test-model", "test-model-2", "bbb", "aaa", "test-model-3"} + cfg := &Config{ + Provider: "test-provider", + Model: "test-model-2", + CustomProviders: map[string]ProviderEntry{ + "test-provider": { + Model: "test-model-2", + Models: append([]string(nil), models...), + }, + }, + } + if err := saveConfig(configPath, cfg); err != nil { + t.Fatalf("saveConfig: %v", err) + } + + result := providerTUIResult{ + provider: "test-provider", + model: "test-model-3", + models: append([]string(nil), models...), + isCustom: true, + isEdit: true, + } + if err := applyCustomProviderConfig(configPath, cfg, result); err != nil { + t.Fatalf("applyCustomProviderConfig: %v", err) + } + + got := cfg.CustomProviders["test-provider"].Models + if len(got) != len(models) { + t.Fatalf("Models length = %d, want %d: %v", len(got), len(models), got) + } + for i := range models { + if got[i] != models[i] { + t.Errorf("Models[%d] = %q, want %q", i, got[i], models[i]) + } + } + if cfg.CustomProviders["test-provider"].Model != "test-model-3" { + t.Errorf("entry.Model = %q, want test-model-3", cfg.CustomProviders["test-provider"].Model) + } +} From ebbeb1699337fb705665c9d714804aec70ccd038 Mon Sep 17 00:00:00 2001 From: wxw <1406771356@qq.com> Date: Fri, 26 Jun 2026 16:21:46 +0800 Subject: [PATCH 10/11] Normalize AuthHeader at apply layer and simplify UseAnthropic assignment. Call NormalizeAuthHeader in applyManualConfig and applyCustomProviderConfig before save; simplify UseAnthropic assignment in manual config; add unit tests. --- cmd/opencodereview/provider_cmd.go | 19 +++++---- cmd/opencodereview/provider_tui_test.go | 51 +++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/cmd/opencodereview/provider_cmd.go b/cmd/opencodereview/provider_cmd.go index 5a3ba70..995efb7 100644 --- a/cmd/opencodereview/provider_cmd.go +++ b/cmd/opencodereview/provider_cmd.go @@ -102,14 +102,13 @@ func applyManualConfig(configPath string, cfg *Config, result providerTUIResult) cfg.Llm.URL = result.url cfg.Llm.Model = result.model cfg.Llm.AuthToken = result.apiKey - cfg.Llm.AuthHeader = result.authHeader - if result.protocol == "anthropic" { - useAnthropic := true - cfg.Llm.UseAnthropic = &useAnthropic - } else { - useAnthropic := false - cfg.Llm.UseAnthropic = &useAnthropic + authHeader, err := llm.NormalizeAuthHeader(result.authHeader) + if err != nil { + return fmt.Errorf("invalid auth_header: %w", err) } + cfg.Llm.AuthHeader = authHeader + useAnthropic := result.protocol == "anthropic" + cfg.Llm.UseAnthropic = &useAnthropic if err := saveConfig(configPath, cfg); err != nil { return err @@ -155,7 +154,11 @@ func applyCustomProviderConfig(configPath string, cfg *Config, result providerTU entry.Protocol = result.protocol } if result.authHeader != "" { - entry.AuthHeader = result.authHeader + authHeader, err := llm.NormalizeAuthHeader(result.authHeader) + if err != nil { + return fmt.Errorf("invalid auth_header: %w", err) + } + entry.AuthHeader = authHeader } if result.apiKey != "" { entry.APIKey = result.apiKey diff --git a/cmd/opencodereview/provider_tui_test.go b/cmd/opencodereview/provider_tui_test.go index bf8e486..5e39dd7 100644 --- a/cmd/opencodereview/provider_tui_test.go +++ b/cmd/opencodereview/provider_tui_test.go @@ -1242,3 +1242,54 @@ func TestApplyCustomProviderConfigPreservesModelOrder(t *testing.T) { t.Errorf("entry.Model = %q, want test-model-3", cfg.CustomProviders["test-provider"].Model) } } + +func TestApplyManualConfigNormalizesAuthHeader(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config.json") + cfg := &Config{} + + result := providerTUIResult{ + isManual: true, + url: "https://example.com/v1", + model: "test-model", + apiKey: "token", + protocol: "anthropic", + authHeader: "X-Api-Key", + } + if err := applyManualConfig(configPath, cfg, result); err != nil { + t.Fatalf("applyManualConfig: %v", err) + } + if got := cfg.Llm.AuthHeader; got != "x-api-key" { + t.Errorf("Llm.AuthHeader = %q, want %q", got, "x-api-key") + } + useAnthropic := true + if cfg.Llm.UseAnthropic == nil || *cfg.Llm.UseAnthropic != useAnthropic { + t.Errorf("UseAnthropic = %v, want %v", cfg.Llm.UseAnthropic, useAnthropic) + } +} + +func TestApplyCustomProviderConfigNormalizesAuthHeader(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config.json") + cfg := &Config{ + CustomProviders: map[string]ProviderEntry{ + "test-provider": {URL: "https://example.com", Model: "m"}, + }, + } + + result := providerTUIResult{ + provider: "test-provider", + model: "m", + url: "https://example.com", + protocol: "anthropic", + authHeader: "Authorization", + isCustom: true, + isEdit: true, + } + if err := applyCustomProviderConfig(configPath, cfg, result); err != nil { + t.Fatalf("applyCustomProviderConfig: %v", err) + } + if got := cfg.CustomProviders["test-provider"].AuthHeader; got != "authorization" { + t.Errorf("AuthHeader = %q, want %q", got, "authorization") + } +} From 4d2ccc8c93531115ad5a27ed57106a6b893be433 Mon Sep 17 00:00:00 2001 From: wxw <1406771356@qq.com> Date: Fri, 26 Jun 2026 16:52:17 +0800 Subject: [PATCH 11/11] Address review: lowercase error strings and newProviderTUI signature. Use lowercase "failed to save" errors; replace variadic configPath with string and remove configPathFromArgs; pass "" in tests when no path; gofmt provider_tui.go. --- cmd/opencodereview/provider_tui.go | 81 +++++++++++-------------- cmd/opencodereview/provider_tui_test.go | 58 +++++++++--------- 2 files changed, 66 insertions(+), 73 deletions(-) diff --git a/cmd/opencodereview/provider_tui.go b/cmd/opencodereview/provider_tui.go index 959e639..2e0b795 100644 --- a/cmd/opencodereview/provider_tui.go +++ b/cmd/opencodereview/provider_tui.go @@ -90,20 +90,20 @@ type providerTUIModel struct { editTargetName string cpStep customProviderStep cpProtocolIdx int - cpNameInput textinput.Model - cpURLInput textinput.Model - cpAuthInput textinput.Model + cpNameInput textinput.Model + cpURLInput textinput.Model + cpAuthInput textinput.Model // --- tab: manual --- - inManualForm bool - manualStep manualStep - manualProtocolIdx int - manualURLInput textinput.Model - manualModelInput textinput.Model - manualAuthHeaderInput textinput.Model - manualTokenInput textinput.Model - manualTokenMasked bool - manualTokenOriginal string + inManualForm bool + manualStep manualStep + manualProtocolIdx int + manualURLInput textinput.Model + manualModelInput textinput.Model + manualAuthHeaderInput textinput.Model + manualTokenInput textinput.Model + manualTokenMasked bool + manualTokenOriginal string // --- shared model/api-key steps (official + existing custom) --- modelIdx int @@ -158,7 +158,7 @@ func collectCustomProviders(cfg *Config) []customProviderListItem { return out } -func newProviderTUI(cfg *Config, configPath ...string) providerTUIModel { +func newProviderTUI(cfg *Config, configPath string) providerTUIModel { providers := llm.ListProviders() sort.SliceStable(providers, func(i, j int) bool { left := strings.ToLower(providers[i].DisplayName) @@ -210,22 +210,22 @@ func newProviderTUI(cfg *Config, configPath ...string) providerTUIModel { manualToken.EchoCharacter = '*' m := providerTUIModel{ - providers: providers, - existingCfg: cfg, - modelInput: mi, - apiKeyInput: ai, - cpNameInput: cpName, - cpURLInput: cpURL, - cpAuthInput: cpAuth, - manualURLInput: manualURL, - manualModelInput: manualModel, + providers: providers, + existingCfg: cfg, + modelInput: mi, + apiKeyInput: ai, + cpNameInput: cpName, + cpURLInput: cpURL, + cpAuthInput: cpAuth, + manualURLInput: manualURL, + manualModelInput: manualModel, manualAuthHeaderInput: manualAuthHeader, - manualTokenInput: manualToken, - width: 80, - height: 24, - activeTab: tabOfficial, - customProviders: collectCustomProviders(cfg), - configPath: configPathFromArgs(configPath), + manualTokenInput: manualToken, + width: 80, + height: 24, + activeTab: tabOfficial, + customProviders: collectCustomProviders(cfg), + configPath: configPath, } providerFound := false @@ -420,7 +420,7 @@ func (m *providerTUIModel) syncSessionModelSelection() error { if m.configPath != "" { if err := saveConfig(m.configPath, m.existingCfg); err != nil { - return fmt.Errorf("Failed to save: %w", err) + return fmt.Errorf("failed to save: %w", err) } } m.savedInSession = true @@ -845,11 +845,11 @@ func (m providerTUIModel) handleCustomFormEnter() (tea.Model, tea.Cmd) { func (m providerTUIModel) applyCreateCustomProvider() (tea.Model, tea.Cmd) { if m.existingCfg == nil { - m.formError = "Failed to save: config not loaded" + m.formError = "failed to save: config not loaded" return m, nil } if m.configPath == "" { - m.formError = "Failed to save: config path not available" + m.formError = "failed to save: config path not available" return m, nil } r := m.result() @@ -879,7 +879,7 @@ func (m providerTUIModel) applyCreateCustomProvider() (tea.Model, tea.Cmd) { m.existingCfg.CustomProviders[r.provider] = entry if err := saveConfig(m.configPath, m.existingCfg); err != nil { - m.formError = fmt.Sprintf("Failed to save: %v", err) + m.formError = fmt.Sprintf("failed to save: %v", err) return m, nil } @@ -947,11 +947,11 @@ func cloneCustomProviderList(src []customProviderListItem) []customProviderListI func (m *providerTUIModel) applyEditCustomProviderSave() error { if m.existingCfg == nil { - m.formError = "Failed to save: config not loaded" + m.formError = "failed to save: config not loaded" return fmt.Errorf("config not loaded") } if m.configPath == "" { - m.formError = "Failed to save: config path not available" + m.formError = "failed to save: config path not available" return fmt.Errorf("config path not available") } r := m.result() @@ -995,7 +995,7 @@ func (m *providerTUIModel) applyEditCustomProviderSave() error { m.existingCfg.CustomProviders[r.provider] = entry if err := saveConfig(m.configPath, m.existingCfg); err != nil { - m.formError = fmt.Sprintf("Failed to save: %v", err) + m.formError = fmt.Sprintf("failed to save: %v", err) if reloaded, reloadErr := loadOrCreateConfig(m.configPath); reloadErr == nil { m.existingCfg = reloaded m.customProviders = collectCustomProviders(reloaded) @@ -1165,7 +1165,7 @@ func (m providerTUIModel) updateDeleteConfirm(key string) (tea.Model, tea.Cmd) { m.existingCfg = reloaded m.customProviders = collectCustomProviders(reloaded) } - m.formError = fmt.Sprintf("Failed to save: %v", err) + m.formError = fmt.Sprintf("failed to save: %v", err) m.confirmingDelete = false return m, nil } @@ -1215,7 +1215,7 @@ func (m providerTUIModel) updateDeleteModelConfirm(key string) (tea.Model, tea.C m.existingCfg = reloaded m.customProviders = collectCustomProviders(reloaded) } - m.formError = fmt.Sprintf("Failed to save: %v", err) + m.formError = fmt.Sprintf("failed to save: %v", err) m.confirmingDeleteModel = false return m, nil } @@ -1444,13 +1444,6 @@ func (m providerTUIModel) handleDown() (tea.Model, tea.Cmd) { return m, nil } -func configPathFromArgs(args []string) string { - if len(args) > 0 { - return args[0] - } - return "" -} - func (m *providerTUIModel) loadExistingAPIKey() { m.apiKeyMasked = false m.apiKeyOriginal = "" diff --git a/cmd/opencodereview/provider_tui_test.go b/cmd/opencodereview/provider_tui_test.go index 5e39dd7..42e9d41 100644 --- a/cmd/opencodereview/provider_tui_test.go +++ b/cmd/opencodereview/provider_tui_test.go @@ -41,7 +41,7 @@ func charKey(c rune) tea.KeyPressMsg { // --- Tab switching tests --- func TestProviderTUI_TabSwitchRight(t *testing.T) { - m := newProviderTUI(&Config{}) + m := newProviderTUI(&Config{}, "") if m.activeTab != tabOfficial { t.Fatalf("initial tab = %d, want %d", m.activeTab, tabOfficial) } @@ -67,7 +67,7 @@ func TestProviderTUI_TabSwitchRight(t *testing.T) { } func TestProviderTUI_TabSwitchLeft(t *testing.T) { - m := newProviderTUI(&Config{}) + m := newProviderTUI(&Config{}, "") // Go to manual tab first result, _ := m.Update(rightKey()) @@ -99,7 +99,7 @@ func TestProviderTUI_TabSwitchLeft(t *testing.T) { } func TestProviderTUI_TabKeyCycles(t *testing.T) { - m := newProviderTUI(&Config{}) + m := newProviderTUI(&Config{}, "") result, _ := m.Update(tabKeyMsg()) m2 := result.(providerTUIModel) @@ -121,7 +121,7 @@ func TestProviderTUI_TabKeyCycles(t *testing.T) { } func TestProviderTUI_TabSwitchOnlyOnStepProvider(t *testing.T) { - m := newProviderTUI(&Config{}) + m := newProviderTUI(&Config{}, "") // Advance to stepModel result, _ := m.Update(enterKey()) @@ -141,7 +141,7 @@ func TestProviderTUI_TabSwitchOnlyOnStepProvider(t *testing.T) { // --- Official tab tests (updated from original) --- func TestProviderTUI_OfficialProvidersSortedByDisplayName(t *testing.T) { - m := newProviderTUI(&Config{}) + m := newProviderTUI(&Config{}, "") displayNames := make([]string, len(m.providers)) normalized := make([]string, len(m.providers)) @@ -156,7 +156,7 @@ func TestProviderTUI_OfficialProvidersSortedByDisplayName(t *testing.T) { } func TestProviderTUI_EscFromModelGoesBackToProvider(t *testing.T) { - m := newProviderTUI(&Config{}) + m := newProviderTUI(&Config{}, "") result, _ := m.Update(enterKey()) m2 := result.(providerTUIModel) @@ -175,7 +175,7 @@ func TestProviderTUI_EscFromModelGoesBackToProvider(t *testing.T) { } func TestProviderTUI_EscFromAPIKeyGoesBackToModel(t *testing.T) { - m := newProviderTUI(&Config{}) + m := newProviderTUI(&Config{}, "") result, _ := m.Update(enterKey()) m2 := result.(providerTUIModel) @@ -194,7 +194,7 @@ func TestProviderTUI_EscFromAPIKeyGoesBackToModel(t *testing.T) { } func TestProviderTUI_EscFromProviderCancels(t *testing.T) { - m := newProviderTUI(&Config{}) + m := newProviderTUI(&Config{}, "") result, cmd := m.Update(escKey()) m2 := result.(providerTUIModel) @@ -216,7 +216,7 @@ func TestProviderTUI_EscKeyString(t *testing.T) { // --- Manual tab tests --- func TestProviderTUI_ManualTabEnterStartsForm(t *testing.T) { - m := newProviderTUI(&Config{}) + m := newProviderTUI(&Config{}, "") // Switch to manual tab result, _ := m.Update(rightKey()) @@ -239,7 +239,7 @@ func TestProviderTUI_ManualTabEnterStartsForm(t *testing.T) { } func TestProviderTUI_ManualFormEscFromURLExitsForm(t *testing.T) { - m := newProviderTUI(&Config{}) + m := newProviderTUI(&Config{}, "") // Switch to manual tab and enter form result, _ := m.Update(rightKey()) @@ -271,7 +271,7 @@ func TestProviderTUI_ManualFormEscRestoresOriginalValues(t *testing.T) { AuthToken: "token-123", }, } - m := newProviderTUI(cfg) + m := newProviderTUI(cfg, "") // Enter the form result, _ := m.Update(enterKey()) @@ -311,7 +311,7 @@ func TestProviderTUI_ManualFormPrefilledValues(t *testing.T) { AuthToken: "token-123", }, } - m := newProviderTUI(cfg) + m := newProviderTUI(cfg, "") if m.activeTab != tabManual { t.Fatalf("should auto-select manual tab when Llm.URL is set, got %d", m.activeTab) @@ -341,7 +341,7 @@ func TestProviderTUI_ManualResult(t *testing.T) { AuthToken: "token-123", }, } - m := newProviderTUI(cfg) + m := newProviderTUI(cfg, "") // Enter the form result, _ := m.Update(enterKey()) @@ -372,7 +372,7 @@ func TestProviderTUI_ManualFormPrefilledWhenProviderSet(t *testing.T) { AuthToken: "manual-token", }, } - m := newProviderTUI(cfg) + m := newProviderTUI(cfg, "") if m.activeTab != tabCustom { t.Fatalf("should auto-select custom tab, got %d", m.activeTab) @@ -400,7 +400,7 @@ func TestProviderTUI_ManualFormPrefillsAuthHeader(t *testing.T) { AuthHeader: "X-Custom-Auth", }, } - m := newProviderTUI(cfg) + m := newProviderTUI(cfg, "") if got := m.manualAuthHeaderInput.Value(); got != "X-Custom-Auth" { t.Errorf("manualAuthHeaderInput not prefilled: got %q, want %q", got, "X-Custom-Auth") @@ -415,7 +415,7 @@ func TestProviderTUI_ManualFormSkipsEmptyTokenWhenOriginalExists(t *testing.T) { AuthToken: "token-123", }, } - m := newProviderTUI(cfg) + m := newProviderTUI(cfg, "") m.inManualForm = true m.manualStep = manualStepAuthToken m.manualTokenOriginal = "token-123" @@ -437,7 +437,7 @@ func TestProviderTUI_ManualFormSkipsEmptyTokenWhenOriginalExists(t *testing.T) { } func TestProviderTUI_ManualFormRequiresTokenOnFirstSetup(t *testing.T) { - m := newProviderTUI(&Config{}) + m := newProviderTUI(&Config{}, "") m.inManualForm = true m.manualStep = manualStepAuthToken m.manualTokenInput.SetValue("") @@ -453,7 +453,7 @@ func TestProviderTUI_ManualFormRequiresTokenOnFirstSetup(t *testing.T) { // --- Custom tab tests --- func TestProviderTUI_CustomTabShowsAddOption(t *testing.T) { - m := newProviderTUI(&Config{}) + m := newProviderTUI(&Config{}, "") // Switch to custom tab result, _ := m.Update(rightKey()) @@ -469,7 +469,7 @@ func TestProviderTUI_CustomTabShowsAddOption(t *testing.T) { } func TestProviderTUI_CustomTabSelectAddStartsForm(t *testing.T) { - m := newProviderTUI(&Config{}) + m := newProviderTUI(&Config{}, "") // Switch to custom tab result, _ := m.Update(rightKey()) @@ -487,7 +487,7 @@ func TestProviderTUI_CustomTabSelectAddStartsForm(t *testing.T) { } func TestProviderTUI_CustomFormEscFromNameExitsForm(t *testing.T) { - m := newProviderTUI(&Config{}) + m := newProviderTUI(&Config{}, "") // Switch to custom tab and start form result, _ := m.Update(rightKey()) @@ -516,7 +516,7 @@ func TestProviderTUI_CustomFormRejectsDuplicateName(t *testing.T) { "stepfun": {Model: "xxx"}, }, } - m := newProviderTUI(cfg) + m := newProviderTUI(cfg, "") result, _ := m.Update(downKey()) m2 := result.(providerTUIModel) @@ -746,7 +746,7 @@ func TestProviderTUI_CustomProviderExistsInList(t *testing.T) { }, }, } - m := newProviderTUI(cfg) + m := newProviderTUI(cfg, "") if m.activeTab != tabCustom { t.Fatalf("should auto-select custom tab, got %d", m.activeTab) @@ -772,7 +772,7 @@ func TestProviderTUI_SelectExistingCustomGoesToModel(t *testing.T) { }, }, } - m := newProviderTUI(cfg) + m := newProviderTUI(cfg, "") // Enter on existing custom provider should go to model selection first. result, _ := m.Update(enterKey()) @@ -854,7 +854,7 @@ func TestProviderTUI_DeleteCustomProvider(t *testing.T) { "my-llm": {URL: "https://custom.api/v1", Protocol: "openai", Model: "custom-model"}, }, } - m := newProviderTUI(cfg) + m := newProviderTUI(cfg, "") // Switch to custom tab result, _ := m.Update(rightKey()) @@ -894,7 +894,7 @@ func TestProviderTUI_DeleteCustomProviderCancel(t *testing.T) { "my-llm": {URL: "https://custom.api/v1", Protocol: "openai", Model: "custom-model"}, }, } - m := newProviderTUI(cfg) + m := newProviderTUI(cfg, "") // Force custom tab so this test is independent of init-time tab routing. // Switch to custom tab, select provider, press d @@ -927,7 +927,7 @@ func TestProviderTUI_DeleteOnAddOptionIgnored(t *testing.T) { "my-llm": {URL: "https://custom.api/v1", Protocol: "openai"}, }, } - m := newProviderTUI(cfg) + m := newProviderTUI(cfg, "") // Switch to custom tab result, _ := m.Update(rightKey()) @@ -949,7 +949,7 @@ func TestProviderTUI_DeleteActiveCustomProvider(t *testing.T) { "my-llm": {URL: "https://custom.api/v1", Protocol: "openai", Model: "custom-model"}, }, } - m := newProviderTUI(cfg) + m := newProviderTUI(cfg, "") // Should auto-select custom tab with active provider if m.activeTab != tabCustom { @@ -978,7 +978,7 @@ func TestProviderTUI_DeleteEscCancels(t *testing.T) { "my-llm": {URL: "https://custom.api/v1", Protocol: "openai"}, }, } - m := newProviderTUI(cfg) + m := newProviderTUI(cfg, "") result, _ := m.Update(rightKey()) m2 := result.(providerTUIModel) @@ -1178,7 +1178,7 @@ func TestProviderTUI_DeleteModelPreservesActiveModel(t *testing.T) { }, }, } - m := newProviderTUI(cfg) + m := newProviderTUI(cfg, "") m.activeTab = tabCustom m.customIdx = 0 m.step = stepModel