From 867bebadf41e95bfc3a424163f43ed740ef8ad89 Mon Sep 17 00:00:00 2001 From: sisantaChhatoi Date: Sat, 26 Sep 2026 01:22:59 +0530 Subject: [PATCH 1/2] refactor(provider-tui): share model-delete and form rendering code Merge modelTUIModel's two identical delete-confirm variants, give the provider wizard's two delete paths one save-and-rollback helper, and extract the form field, option list and form error rendering that the custom provider and manual forms each copied. --- cmd/opencodereview/provider_tui.go | 251 +++++++----------- cmd/opencodereview/provider_tui_funcs_test.go | 4 +- .../provider_tui_modeltui_test.go | 8 +- .../provider_tui_persist_test.go | 2 +- .../provider_tui_savefail_test.go | 53 ++++ 5 files changed, 149 insertions(+), 169 deletions(-) diff --git a/cmd/opencodereview/provider_tui.go b/cmd/opencodereview/provider_tui.go index 02c0b05ad..0193b8d65 100644 --- a/cmd/opencodereview/provider_tui.go +++ b/cmd/opencodereview/provider_tui.go @@ -477,6 +477,13 @@ func clearCfgActiveModelIfDeleted(cfg *Config, providerName, name string) { } } +func cfgActiveModelFor(cfg *Config, providerName string) string { + if cfg != nil && cfg.Provider == providerName { + return cfg.Model + } + return "" +} + func rollbackCfgActiveModel(cfg *Config, providerName, prevModel string) { if cfg != nil && cfg.Provider == providerName { cfg.Model = prevModel @@ -1633,10 +1640,7 @@ func (m providerTUIModel) confirmDeleteCustomModel() (tea.Model, tea.Cmd) { if m.modelIdx < len(models) { cp := m.customProviders[m.customIdx] prevEntry := cloneProviderEntry(cp.entry) - prevCfgModel := "" - if m.existingCfg != nil && m.existingCfg.Provider == cp.name { - prevCfgModel = m.existingCfg.Model - } + prevCfgModel := cfgActiveModelFor(m.existingCfg, cp.name) cp.entry = applyModelDeleteToEntry(cp.entry, m.deleteModelName) clearCfgActiveModelIfDeleted(m.existingCfg, cp.name, m.deleteModelName) m.customProviders[m.customIdx] = cp @@ -1646,24 +1650,14 @@ func (m providerTUIModel) confirmDeleteCustomModel() (tea.Model, tea.Cmd) { } m.existingCfg.CustomProviders[cp.name] = cp.entry } - if m.configPath != "" { - if err := saveConfig(m.configPath, m.existingCfg); err != nil { - if !m.reloadConfigAfterSaveFailure() { - cp.entry = prevEntry - m.customProviders[m.customIdx] = cp - if m.existingCfg != nil { - m.existingCfg.CustomProviders[cp.name] = prevEntry - rollbackCfgActiveModel(m.existingCfg, cp.name, prevCfgModel) - } - } - m.formError = fmt.Sprintf("failed to save: %v", err) - m.adjustModelIdxAfterDelete() - m.confirmingDeleteModel = false - return m, nil + m.persistModelDelete(func() { + cp.entry = prevEntry + m.customProviders[m.customIdx] = cp + if m.existingCfg != nil { + m.existingCfg.CustomProviders[cp.name] = prevEntry + rollbackCfgActiveModel(m.existingCfg, cp.name, prevCfgModel) } - } - m.adjustModelIdxAfterDelete() - m.savedInSession = true + }) } m.confirmingDeleteModel = false return m, nil @@ -1683,30 +1677,34 @@ func (m providerTUIModel) confirmDeleteOfficialModel() (tea.Model, tea.Cmd) { m.existingCfg.Providers = make(map[string]ProviderEntry) } prevEntry := cloneProviderEntry(m.existingCfg.Providers[provider.Name]) - prevCfgModel := "" - if m.existingCfg.Provider == provider.Name { - prevCfgModel = m.existingCfg.Model - } + prevCfgModel := cfgActiveModelFor(m.existingCfg, provider.Name) entry := applyModelDeleteToEntry(m.existingCfg.Providers[provider.Name], m.deleteModelName) clearCfgActiveModelIfDeleted(m.existingCfg, provider.Name, m.deleteModelName) m.existingCfg.Providers[provider.Name] = entry + m.persistModelDelete(func() { + m.existingCfg.Providers[provider.Name] = prevEntry + rollbackCfgActiveModel(m.existingCfg, provider.Name, prevCfgModel) + }) + m.confirmingDeleteModel = false + return m, nil +} + +// rollback runs only when the save fails and the on-disk config cannot be +// reloaded in its place. +func (m *providerTUIModel) persistModelDelete(rollback func()) { if m.configPath != "" { if err := saveConfig(m.configPath, m.existingCfg); err != nil { if !m.reloadConfigAfterSaveFailure() { - m.existingCfg.Providers[provider.Name] = prevEntry - rollbackCfgActiveModel(m.existingCfg, provider.Name, prevCfgModel) + rollback() } m.formError = fmt.Sprintf("failed to save: %v", err) m.adjustModelIdxAfterDelete() - m.confirmingDeleteModel = false - return m, nil + return } } m.adjustModelIdxAfterDelete() // In-memory delete succeeded; configPath may be empty in tests (no disk write). m.savedInSession = true - m.confirmingDeleteModel = false - return m, nil } func (m *providerTUIModel) adjustModelIdxAfterDelete() { @@ -2266,6 +2264,43 @@ func (m providerTUIModel) viewCustomTab(s *strings.Builder) { } } +type formField struct { + label string + value string + active bool +} + +func writeInactiveFormField(s *strings.Builder, f formField) { + 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") + } +} + +func writeOptionList(s *strings.Builder, options []string, selected int) { + for i, opt := range options { + if i == selected { + s.WriteString(" " + tuiCursorStyle.Render(tuiCursor) + " " + tuiSelectedItemStyle.Render(opt) + "\n") + } else { + s.WriteString(" " + tuiItemStyle.Render(opt) + "\n") + } + } +} + +func writeFormError(s *strings.Builder, msg string) { + if msg == "" { + return + } + s.WriteString("\n") + s.WriteString(tuiErrorStyle.Render(" " + msg)) + s.WriteString("\n") +} + func (m providerTUIModel) viewCustomProviderForm(s *strings.Builder) { title := " Add Custom Provider" if m.editingCustom { @@ -2274,21 +2309,15 @@ func (m providerTUIModel) viewCustomProviderForm(s *strings.Builder) { s.WriteString(tuiTitleStyle.Render(title)) s.WriteString("\n\n") - type field struct { - label string - value string - active bool - } - - fields := []field{ + fields := []formField{ {"Provider name", m.cpNameInput.Value(), m.cpStep == cpStepName}, {"Protocol", cpProtocols[m.cpProtocolIdx], m.cpStep == cpStepProtocol}, } if !m.cpAmbientProtocol() { fields = append(fields, - field{"Base URL", m.cpURLInput.Value(), m.cpStep == cpStepBaseURL}, - field{"API Key", strings.Repeat("*", len(m.apiKeyInput.Value())), m.cpStep == cpStepAPIKey}, - field{"Auth Header", m.cpAuthInput.Value(), m.cpStep == cpStepAuthHeader}, + formField{"Base URL", m.cpURLInput.Value(), m.cpStep == cpStepBaseURL}, + formField{"API Key", strings.Repeat("*", len(m.apiKeyInput.Value())), m.cpStep == cpStepAPIKey}, + formField{"Auth Header", m.cpAuthInput.Value(), m.cpStep == cpStepAuthHeader}, ) } @@ -2299,15 +2328,7 @@ func (m providerTUIModel) viewCustomProviderForm(s *strings.Builder) { case cpStepName: s.WriteString(" " + m.cpNameInput.View() + "\n") case cpStepProtocol: - for i, proto := range cpProtocols { - if i == m.cpProtocolIdx { - cur := " " + tuiCursorStyle.Render(tuiCursor) + " " - s.WriteString(cur + tuiSelectedItemStyle.Render(proto) + "\n") - } else { - cur := " " - s.WriteString(cur + tuiItemStyle.Render(proto) + "\n") - } - } + writeOptionList(s, cpProtocols, m.cpProtocolIdx) if m.cpAmbientProtocol() { s.WriteString(tuiDimStyle.Render(" credentials come from the AWS chain; pin a region or profile with `ocr config set custom_providers."+m.cpNameInput.Value()+".aws_region `") + "\n") } @@ -2322,23 +2343,11 @@ func (m providerTUIModel) viewCustomProviderForm(s *strings.Builder) { s.WriteString(" " + m.cpAuthInput.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") - } + writeInactiveFormField(s, f) } } - if m.formError != "" { - s.WriteString("\n") - s.WriteString(tuiErrorStyle.Render(" " + m.formError)) - s.WriteString("\n") - } + writeFormError(s, m.formError) } func (m providerTUIModel) viewManualTab(s *strings.Builder) { @@ -2361,13 +2370,7 @@ func (m providerTUIModel) viewManualTab(s *strings.Builder) { s.WriteString(tuiTitleStyle.Render(" Manual Configuration")) s.WriteString("\n\n") - type field struct { - label string - value string - active bool - } - - fields := []field{ + fields := []formField{ {"URL", m.manualURLInput.Value(), m.manualStep == manualStepURL}, {"Protocol", manualProtocols[m.manualProtocolIdx], m.manualStep == manualStepProtocol}, {"Model", m.manualModelInput.Value(), m.manualStep == manualStepModel}, @@ -2382,15 +2385,7 @@ func (m providerTUIModel) viewManualTab(s *strings.Builder) { case manualStepURL: s.WriteString(" " + m.manualURLInput.View() + "\n") case manualStepProtocol: - for i, proto := range manualProtocols { - if i == m.manualProtocolIdx { - cur := " " + tuiCursorStyle.Render(tuiCursor) + " " - s.WriteString(cur + tuiSelectedItemStyle.Render(proto) + "\n") - } else { - cur := " " - s.WriteString(cur + tuiItemStyle.Render(proto) + "\n") - } - } + writeOptionList(s, manualProtocols, m.manualProtocolIdx) case manualStepModel: s.WriteString(" " + m.manualModelInput.View() + "\n") case manualStepAuthToken: @@ -2405,23 +2400,11 @@ func (m providerTUIModel) viewManualTab(s *strings.Builder) { 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") - } + writeInactiveFormField(s, f) } } - if m.formError != "" { - s.WriteString("\n") - s.WriteString(tuiErrorStyle.Render(" " + m.formError)) - s.WriteString("\n") - } + writeFormError(s, m.formError) } func (m providerTUIModel) viewModel(s *strings.Builder) { @@ -2525,11 +2508,7 @@ func (m providerTUIModel) viewAPIKey(s *strings.Builder) { } } - if m.formError != "" { - s.WriteString("\n") - s.WriteString(tuiErrorStyle.Render(" " + m.formError)) - s.WriteString("\n") - } + writeFormError(s, m.formError) s.WriteString("\n") s.WriteString(tuiHelpStyle.Render(" Enter Confirm Esc Back")) @@ -3010,70 +2989,22 @@ func (m *modelTUIModel) updateDeleteModelConfirm(key string) (tea.Model, tea.Cmd } func (m *modelTUIModel) confirmDeleteModel() (tea.Model, tea.Cmd) { - if m.isCustomProvider { - return m.confirmDeleteCustomProviderModel() - } - return m.confirmDeleteOfficialModel() -} - -func (m *modelTUIModel) confirmDeleteCustomProviderModel() (tea.Model, tea.Cmd) { - if !m.isUserAddedModel(m.deleteModelName) { + if !m.isUserAddedModel(m.deleteModelName) || m.existingCfg == nil || m.providerName == "" { m.confirmingDeleteModel = false return *m, nil } - if m.existingCfg == nil || m.providerName == "" { - m.confirmingDeleteModel = false - return *m, nil - } - if m.existingCfg.CustomProviders == nil { - m.existingCfg.CustomProviders = make(map[string]ProviderEntry) - } - prevEntry := cloneProviderEntry(m.existingCfg.CustomProviders[m.providerName]) - prevCfgModel := "" - if m.existingCfg.Provider == m.providerName { - prevCfgModel = m.existingCfg.Model - } - entry := applyModelDeleteToEntry(m.existingCfg.CustomProviders[m.providerName], m.deleteModelName) - clearCfgActiveModelIfDeleted(m.existingCfg, m.providerName, m.deleteModelName) - m.existingCfg.CustomProviders[m.providerName] = entry - if m.configPath != "" { - if err := saveConfig(m.configPath, m.existingCfg); err != nil { - if !m.reloadConfigAfterSaveFailure() { - m.rollbackModelDelete(prevEntry, prevCfgModel) - } - m.formError = fmt.Sprintf("failed to save: %v", err) - m.adjustModelIdxAfterDelete() - m.confirmingDeleteModel = false - return *m, nil - } - } - m.adjustModelIdxAfterDelete() - m.resetCustomModelInput() - m.savedInSession = true - m.confirmingDeleteModel = false - return *m, nil -} - -func (m *modelTUIModel) confirmDeleteOfficialModel() (tea.Model, tea.Cmd) { - if !m.isUserAddedModel(m.deleteModelName) { - m.confirmingDeleteModel = false - return *m, nil - } - if m.existingCfg == nil || m.providerName == "" { - m.confirmingDeleteModel = false - return *m, nil - } - if m.existingCfg.Providers == nil { - m.existingCfg.Providers = make(map[string]ProviderEntry) + field := &m.existingCfg.Providers + if m.isCustomProvider { + field = &m.existingCfg.CustomProviders } - prevEntry := cloneProviderEntry(m.existingCfg.Providers[m.providerName]) - prevCfgModel := "" - if m.existingCfg.Provider == m.providerName { - prevCfgModel = m.existingCfg.Model + if *field == nil { + *field = make(map[string]ProviderEntry) } - entry := applyModelDeleteToEntry(m.existingCfg.Providers[m.providerName], m.deleteModelName) + entries := *field + prevEntry := cloneProviderEntry(entries[m.providerName]) + prevCfgModel := cfgActiveModelFor(m.existingCfg, m.providerName) + entries[m.providerName] = applyModelDeleteToEntry(entries[m.providerName], m.deleteModelName) clearCfgActiveModelIfDeleted(m.existingCfg, m.providerName, m.deleteModelName) - m.existingCfg.Providers[m.providerName] = entry if m.configPath != "" { if err := saveConfig(m.configPath, m.existingCfg); err != nil { if !m.reloadConfigAfterSaveFailure() { @@ -3151,11 +3082,7 @@ func (m modelTUIModel) View() tea.View { s.WriteString("\n") } - if m.formError != "" { - s.WriteString("\n") - s.WriteString(tuiErrorStyle.Render(" " + m.formError)) - s.WriteString("\n") - } + writeFormError(&s, m.formError) s.WriteString("\n") diff --git a/cmd/opencodereview/provider_tui_funcs_test.go b/cmd/opencodereview/provider_tui_funcs_test.go index 094cd3db0..d6c2e5654 100644 --- a/cmd/opencodereview/provider_tui_funcs_test.go +++ b/cmd/opencodereview/provider_tui_funcs_test.go @@ -1230,7 +1230,7 @@ func TestModelTUI_DeleteSetsSavedInSession(t *testing.T) { m.deleteModelName = "aaa" m.confirmingDeleteModel = true - result, _ := m.confirmDeleteCustomProviderModel() + result, _ := m.confirmDeleteModel() m2 := asModelTUIModel(t, result) if !m2.savedInSession { t.Error("savedInSession should be true after delete") @@ -1389,7 +1389,7 @@ func TestModelTUI_CustomProvider_DeleteClearsCustomInput(t *testing.T) { m.deleteModelName = "aaa" m.confirmingDeleteModel = true - result, _ := m.confirmDeleteCustomProviderModel() + result, _ := m.confirmDeleteModel() m2 := asModelTUIModel(t, result) if m2.customModel { diff --git a/cmd/opencodereview/provider_tui_modeltui_test.go b/cmd/opencodereview/provider_tui_modeltui_test.go index 33441eb33..4543cb52c 100644 --- a/cmd/opencodereview/provider_tui_modeltui_test.go +++ b/cmd/opencodereview/provider_tui_modeltui_test.go @@ -195,7 +195,7 @@ func TestConfirmDeleteCustomProviderModel_Guards(t *testing.T) { m, _, _ := newCustomModelTUI(t, []string{"m1"}) m.confirmingDeleteModel = true m.deleteModelName = "not-in-list" - out, _ := m.confirmDeleteCustomProviderModel() + out, _ := m.confirmDeleteModel() got := out.(modelTUIModel) if got.confirmingDeleteModel { t.Error("unknown model should end confirm without saving") @@ -210,7 +210,7 @@ func TestConfirmDeleteCustomProviderModel_Guards(t *testing.T) { m.deleteModelName = "m1" m.existingCfg = nil m.confirmingDeleteModel = true - out, _ := m.confirmDeleteCustomProviderModel() + out, _ := m.confirmDeleteModel() if out.(modelTUIModel).savedInSession { t.Error("nil cfg must not persist") } @@ -243,7 +243,7 @@ func TestConfirmDeleteOfficialModel(t *testing.T) { m, cfg := newOfficial(t) m.confirmingDeleteModel = true m.deleteModelName = "user-added" - out, _ := m.confirmDeleteOfficialModel() + out, _ := m.confirmDeleteModel() got := out.(modelTUIModel) if !got.savedInSession { t.Errorf("should persist deletion; formError=%q", got.formError) @@ -257,7 +257,7 @@ func TestConfirmDeleteOfficialModel(t *testing.T) { m, _ := newOfficial(t) m.confirmingDeleteModel = true m.deleteModelName = "gpt-4" // in registry, not user-added - out, _ := m.confirmDeleteOfficialModel() + out, _ := m.confirmDeleteModel() if out.(modelTUIModel).savedInSession { t.Error("registry model must not be deletable") } diff --git a/cmd/opencodereview/provider_tui_persist_test.go b/cmd/opencodereview/provider_tui_persist_test.go index b9b38d1bb..cd37c139b 100644 --- a/cmd/opencodereview/provider_tui_persist_test.go +++ b/cmd/opencodereview/provider_tui_persist_test.go @@ -383,7 +383,7 @@ func TestConfirmDeleteOfficialModelActiveClear(t *testing.T) { m.confirmingDeleteModel = true m.deleteModelName = "user-added" - out, _ := m.confirmDeleteOfficialModel() + out, _ := m.confirmDeleteModel() got := out.(modelTUIModel) if !got.savedInSession { t.Fatalf("should persist deletion; formError=%q", got.formError) diff --git a/cmd/opencodereview/provider_tui_savefail_test.go b/cmd/opencodereview/provider_tui_savefail_test.go index cc9240100..b1cb4846b 100644 --- a/cmd/opencodereview/provider_tui_savefail_test.go +++ b/cmd/opencodereview/provider_tui_savefail_test.go @@ -8,6 +8,8 @@ import ( "path/filepath" "strings" "testing" + + "github.com/alibaba/open-code-review/internal/llm" ) // unwritableConfigPath returns a config path that is itself a directory, so any @@ -147,6 +149,57 @@ func TestConfirmDeleteOfficialModel_SaveFailureRollback(t *testing.T) { } } +// modelTUIModel.confirmDeleteModel serves official and custom providers from +// one body, so a failed save has to roll back whichever config map the +// provider lives in, along with the active model. +func TestModelTUIConfirmDeleteModel_SaveFailureRollback(t *testing.T) { + for _, custom := range []bool{false, true} { + name := "official" + if custom { + name = "custom" + } + t.Run(name, func(t *testing.T) { + const provider, userModel = "prov", "user-added" + entries := map[string]ProviderEntry{provider: {Models: []string{userModel}}} + cfg := &Config{Provider: provider, Model: userModel} + if custom { + cfg.CustomProviders = entries + } else { + cfg.Providers = entries + } + m := newModelTUIConfig(modelTUIConfig{ + Provider: llm.Provider{Name: provider, DisplayName: "Prov", Models: []string{userModel}}, + ProviderName: provider, + RegistryModels: []string{"registry-model"}, + ExistingCfg: cfg, + ConfigPath: unwritableConfigPath(t), + IsCustom: custom, + }) + m.deleteModelName = userModel + m.confirmingDeleteModel = true + if !m.isUserAddedModel(userModel) { + t.Fatalf("test setup: %q not recognized as user-added", userModel) + } + + out, _ := m.confirmDeleteModel() + got := out.(modelTUIModel) + + if !strings.Contains(got.formError, "failed to save") { + t.Errorf("formError = %q, want save-failure message", got.formError) + } + if got.savedInSession { + t.Error("savedInSession should be false after save failure") + } + if !containsStr(entries[provider].Models, userModel) { + t.Errorf("rolled-back models = %v, want to contain %q", entries[provider].Models, userModel) + } + if cfg.Model != userModel { + t.Errorf("active model = %q, want %q restored", cfg.Model, userModel) + } + }) + } +} + func containsStr(list []string, want string) bool { for _, s := range list { if s == want { From 758a7f5153b14ba773ec1f759bd19db1b44f0d90 Mon Sep 17 00:00:00 2001 From: sisantaChhatoi Date: Sat, 26 Sep 2026 01:24:45 +0530 Subject: [PATCH 2/2] test(provider-tui): share setup helpers and merge mirrored tests Extract repeated fixtures, lookups and assertions into helpers, and run the delete-flow and custom-model-input scenarios that were copied per model or per tab from one table-driven test each. --- .../provider_tui_customform_test.go | 3 +- .../provider_tui_deleteconfirm_test.go | 166 ++++ .../provider_tui_editsave_test.go | 6 +- cmd/opencodereview/provider_tui_funcs_test.go | 485 +++-------- .../provider_tui_persist_test.go | 54 +- .../provider_tui_savefail_test.go | 61 +- cmd/opencodereview/provider_tui_test.go | 818 +++++------------- 7 files changed, 545 insertions(+), 1048 deletions(-) diff --git a/cmd/opencodereview/provider_tui_customform_test.go b/cmd/opencodereview/provider_tui_customform_test.go index a05cd7b52..195b05afd 100644 --- a/cmd/opencodereview/provider_tui_customform_test.go +++ b/cmd/opencodereview/provider_tui_customform_test.go @@ -203,8 +203,7 @@ func TestCustomFormEndsAtProtocolForBedrock(t *testing.T) { setup := func(t *testing.T) providerTUIModel { t.Helper() cfg := &Config{} - m := newProviderTUI(cfg, filepath.Join(t.TempDir(), "c.json")) - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, filepath.Join(t.TempDir(), "c.json"), tabCustom) m.creatingCustom = true m.cpStep = cpStepProtocol m.cpProtocolIdx = cpProtocolIndex(llm.ProtocolAnthropicBedrock) diff --git a/cmd/opencodereview/provider_tui_deleteconfirm_test.go b/cmd/opencodereview/provider_tui_deleteconfirm_test.go index c44dc3eb3..1b2e35763 100644 --- a/cmd/opencodereview/provider_tui_deleteconfirm_test.go +++ b/cmd/opencodereview/provider_tui_deleteconfirm_test.go @@ -5,7 +5,10 @@ package main import ( "path/filepath" + "strings" "testing" + + tea "charm.land/bubbletea/v2" ) // TestUpdateDeleteModelConfirm covers the key branches of the delete-model @@ -63,3 +66,166 @@ func TestUpdateDeleteModelConfirm_ProviderTUI(t *testing.T) { } }) } + +// deleteFlowTUI adapts the two models that implement model deletion, +// providerTUIModel (ocr config provider) and modelTUIModel (ocr config model), +// so each delete scenario below is written once and run against both. +type deleteFlowTUI struct { + name string + // official returns a dashscope model whose extraModels are user-added. + official func(t *testing.T, configPath string, extraModels []string) tea.Model + // custom returns a custom provider whose models include "aaa". + custom func(t *testing.T) tea.Model + // selectModel moves the cursor to name, or to the custom-input row when + // name is empty. + selectModel func(t *testing.T, m tea.Model, name string) tea.Model + confirming func(m tea.Model) bool + cfg func(m tea.Model) *Config +} + +var deleteFlowTUIs = []deleteFlowTUI{ + { + name: "provider TUI", + official: func(t *testing.T, configPath string, extraModels []string) tea.Model { + return officialDashscopeModelTUI(t, configPath, extraModels) + }, + custom: func(t *testing.T) tea.Model { + return customStepfunModelTUI(t, "", []string{"step-3.5-flash", "aaa"}) + }, + selectModel: func(t *testing.T, m tea.Model, name string) tea.Model { + pm := m.(providerTUIModel) + if name == "" { + pm.modelIdx = len(pm.models()) + } else { + pm.modelIdx = modelIdxForName(t, pm, name) + } + return pm + }, + confirming: func(m tea.Model) bool { return m.(providerTUIModel).confirmingDeleteModel }, + cfg: func(m tea.Model) *Config { return m.(providerTUIModel).existingCfg }, + }, + { + name: "model TUI", + official: func(t *testing.T, configPath string, extraModels []string) tea.Model { + return officialConfigModelTUI(t, configPath, extraModels) + }, + custom: func(t *testing.T) tea.Model { + return customConfigModelTUI(t, "", []string{"m1", "aaa"}) + }, + selectModel: func(t *testing.T, m tea.Model, name string) tea.Model { + mm := m.(modelTUIModel) + if name == "" { + mm.modelIdx = len(mm.displayModels()) + } else { + mm.modelIdx = modelTUIIdxForName(t, mm, name) + } + return mm + }, + confirming: func(m tea.Model) bool { return m.(modelTUIModel).confirmingDeleteModel }, + cfg: func(m tea.Model) *Config { return m.(modelTUIModel).existingCfg }, + }, +} + +func viewText(m tea.Model) string { return stripANSI(m.View().Content) } + +func TestDeleteFlow_OfficialDeleteHintOnlyOnUserAddedModel(t *testing.T) { + for _, tui := range deleteFlowTUIs { + t.Run(tui.name, func(t *testing.T) { + m := tui.official(t, "", []string{"my-custom-model"}) + if got := viewText(tui.selectModel(t, m, "qwen3.7-max")); strings.Contains(got, "d Delete") { + t.Errorf("built-in model should not show d Delete hint; got:\n%s", got) + } + if got := viewText(tui.selectModel(t, m, "my-custom-model")); !strings.Contains(got, "d Delete") { + t.Errorf("user-added model should show d Delete hint; got:\n%s", got) + } + }) + } +} + +func TestDeleteFlow_CustomDeleteHintOnlyOnModelRows(t *testing.T) { + for _, tui := range deleteFlowTUIs { + t.Run(tui.name, func(t *testing.T) { + m := tui.custom(t) + if got := viewText(tui.selectModel(t, m, "")); strings.Contains(got, "d Delete") { + t.Errorf("custom input row should not show d Delete hint; got:\n%s", got) + } + if got := viewText(tui.selectModel(t, m, "aaa")); !strings.Contains(got, "d Delete") { + t.Errorf("custom model row should show d Delete hint; got:\n%s", got) + } + }) + } +} + +func TestDeleteFlow_OfficialDeleteIgnoredOnNonDeletableRows(t *testing.T) { + rows := []struct{ name, model string }{ + {"registry model", "qwen3.7-max"}, + {"custom input row", ""}, + } + for _, tui := range deleteFlowTUIs { + for _, row := range rows { + t.Run(tui.name+"/"+row.name, func(t *testing.T) { + m := tui.selectModel(t, tui.official(t, "", []string{"my-custom-model"}), row.model) + m, _ = m.Update(dKey()) + if tui.confirming(m) { + t.Error("d should not trigger delete confirmation") + } + }) + } + } +} + +func TestDeleteFlow_OfficialDeleteCancel(t *testing.T) { + cancelKeys := []struct { + name string + key tea.KeyPressMsg + }{ + {"n", nKey()}, + {"esc", escKey()}, + } + for _, tui := range deleteFlowTUIs { + for _, tc := range cancelKeys { + t.Run(tui.name+"/"+tc.name, func(t *testing.T) { + m := tui.selectModel(t, tui.official(t, "", []string{"my-custom-model"}), "my-custom-model") + m, _ = m.Update(dKey()) + if !tui.confirming(m) { + t.Fatal("expected confirmingDeleteModel after d") + } + m, _ = m.Update(tc.key) + if tui.confirming(m) { + t.Error("confirmingDeleteModel should be false after cancel") + } + got := tui.cfg(m).Providers["dashscope"].Models + if len(got) != 2 || got[1] != "my-custom-model" { + t.Errorf("Models = %v, want model unchanged", got) + } + }) + } + } +} + +func TestDeleteFlow_OfficialDeleteActiveUserModelClearsCfg(t *testing.T) { + for _, tui := range deleteFlowTUIs { + t.Run(tui.name, func(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + m := tui.official(t, configPath, []string{"my-custom-model"}) + cfg := tui.cfg(m) + cfg.Model = "my-custom-model" + cfg.Providers["dashscope"] = ProviderEntry{ + Model: "my-custom-model", + Models: []string{"qwen3.7-max", "my-custom-model"}, + } + m = tui.selectModel(t, m, "my-custom-model") + + m, _ = m.Update(dKey()) + m, _ = m.Update(yKey()) + + cfg = tui.cfg(m) + if cfg.Providers["dashscope"].Model != "" { + t.Errorf("entry.Model = %q, want empty", cfg.Providers["dashscope"].Model) + } + if cfg.Model != "" { + t.Errorf("cfg.Model = %q, want empty", cfg.Model) + } + }) + } +} diff --git a/cmd/opencodereview/provider_tui_editsave_test.go b/cmd/opencodereview/provider_tui_editsave_test.go index 4ad79a514..29660830c 100644 --- a/cmd/opencodereview/provider_tui_editsave_test.go +++ b/cmd/opencodereview/provider_tui_editsave_test.go @@ -47,8 +47,7 @@ func TestApplyEditCustomProviderSave_RenameReassignsActiveProvider(t *testing.T) "oldname": {URL: "https://example.com/v1", Protocol: "openai"}, }, } - m := newProviderTUI(cfg, configPath) - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, configPath, tabCustom) m.editingCustom = true m.editTargetName = "oldname" m.cpProtocolIdx = 1 // openai @@ -87,8 +86,7 @@ func TestApplyEditCustomProviderSave_SaveFailureRestoresBackup(t *testing.T) { "aaa": {URL: "https://example.com/v1", Protocol: "openai", Models: []string{"m1"}}, }, } - m := newProviderTUI(cfg, blockPath) - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, blockPath, tabCustom) m.editingCustom = true m.editTargetName = "aaa" m.cpProtocolIdx = 1 diff --git a/cmd/opencodereview/provider_tui_funcs_test.go b/cmd/opencodereview/provider_tui_funcs_test.go index d6c2e5654..15d465079 100644 --- a/cmd/opencodereview/provider_tui_funcs_test.go +++ b/cmd/opencodereview/provider_tui_funcs_test.go @@ -60,13 +60,7 @@ func TestOfficialProviderActiveModel_NilCfg(t *testing.T) { } func TestOfficialProviderActiveModel_DifferentProvider(t *testing.T) { - cfg := &Config{ - Provider: "deepseek", - Model: "deepseek-v4-flash", - Providers: map[string]ProviderEntry{ - "deepseek": {Model: "deepseek-v4-flash"}, - }, - } + cfg := activeDeepseekConfig() m := newProviderTUI(cfg, "") got := m.officialProviderActiveModel(llm.Provider{Name: "anthropic", DisplayName: "Anthropic Claude API"}) if got != "" { @@ -136,8 +130,7 @@ func TestModelProviderName_CustomTab(t *testing.T) { "my-llm": {URL: "http://localhost", Model: "m"}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, "", tabCustom) m.customIdx = 0 name := m.modelProviderName() if !strings.Contains(name, "(custom)") { @@ -146,8 +139,7 @@ func TestModelProviderName_CustomTab(t *testing.T) { } func TestModelProviderName_CustomTab_NoSelection(t *testing.T) { - m := newProviderTUI(&Config{}, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(&Config{}, "", tabCustom) m.customIdx = 999 name := m.modelProviderName() if name != "" { @@ -423,8 +415,7 @@ func TestHandleUp_CustomTab(t *testing.T) { "b": {URL: "http://b"}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, "", tabCustom) m.customIdx = 1 result, _ := m.Update(upKey()) m2 := result.(providerTUIModel) @@ -535,8 +526,7 @@ func TestCurrentProvider_OutOfBounds(t *testing.T) { } func TestCurrentProvider_WrongTab(t *testing.T) { - m := newProviderTUI(&Config{}, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(&Config{}, "", tabCustom) p := m.currentProvider() if p.Name != "" { t.Errorf("expected empty provider for non-official tab, got %q", p.Name) @@ -552,8 +542,7 @@ func TestSelectedCustomProvider_NotCustomTab(t *testing.T) { } func TestSelectedCustomProvider_OutOfBounds(t *testing.T) { - m := newProviderTUI(&Config{}, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(&Config{}, "", tabCustom) m.customIdx = 9999 _, ok := m.selectedCustomProvider() if ok { @@ -618,8 +607,7 @@ func TestHandleDown_CustomTab(t *testing.T) { "b": {URL: "http://b"}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, "", tabCustom) m.customIdx = 0 result, _ := m.Update(downKey()) m2 := result.(providerTUIModel) @@ -634,8 +622,7 @@ func TestHandleDown_CustomTab_Wraps(t *testing.T) { "a": {URL: "http://a"}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, "", tabCustom) m.customIdx = m.customListCount() - 1 result, _ := m.Update(downKey()) m2 := result.(providerTUIModel) @@ -727,11 +714,8 @@ func TestCustomProviderEntry_Fallback(t *testing.T) { } func TestNewModelTUI(t *testing.T) { - p := llm.ListProviders() - if len(p) == 0 { - t.Skip("no providers") - } - m := newModelTUI(p[0], "") + p := firstRegistryProvider(t, 0) + m := newModelTUI(p, "") if m.isCustomProvider { t.Error("preset provider test helper should set isCustomProvider=false") } @@ -748,12 +732,9 @@ func TestNewModelTUI(t *testing.T) { } func TestNewModelTUI_WithCurrentModel(t *testing.T) { - p := llm.ListProviders() - if len(p) == 0 || len(p[0].Models) == 0 { - t.Skip("need provider with models") - } - current := p[0].Models[0] - m := newModelTUI(p[0], current) + p := firstRegistryProvider(t, 1) + current := p.Models[0] + m := newModelTUI(p, current) if m.modelIdx != 0 { t.Errorf("modelIdx = %d, want 0 for first model", m.modelIdx) } @@ -763,23 +744,17 @@ func TestNewModelTUI_WithCurrentModel(t *testing.T) { } func TestNewModelTUI_CustomModel(t *testing.T) { - p := llm.ListProviders() - if len(p) == 0 { - t.Skip("no providers") - } - m := newModelTUI(p[0], "custom-model-xyz") - if m.modelIdx != len(p[0].Models) { - t.Errorf("modelIdx = %d, want %d for custom model", m.modelIdx, len(p[0].Models)) + p := firstRegistryProvider(t, 0) + m := newModelTUI(p, "custom-model-xyz") + if m.modelIdx != len(p.Models) { + t.Errorf("modelIdx = %d, want %d for custom model", m.modelIdx, len(p.Models)) } } func TestModelTUI_IsCustomItem(t *testing.T) { - p := llm.ListProviders() - if len(p) == 0 { - t.Skip("no providers") - } - m := newModelTUI(p[0], "") - if !m.isCustomItem(len(p[0].Models)) { + p := firstRegistryProvider(t, 0) + m := newModelTUI(p, "") + if !m.isCustomItem(len(p.Models)) { t.Error("expected true for custom item index") } if m.isCustomItem(0) { @@ -788,34 +763,25 @@ func TestModelTUI_IsCustomItem(t *testing.T) { } func TestModelTUI_ItemCount(t *testing.T) { - p := llm.ListProviders() - if len(p) == 0 { - t.Skip("no providers") - } - m := newModelTUI(p[0], "") - if m.itemCount() != len(p[0].Models)+1 { - t.Errorf("itemCount() = %d, want %d", m.itemCount(), len(p[0].Models)+1) + p := firstRegistryProvider(t, 0) + m := newModelTUI(p, "") + if m.itemCount() != len(p.Models)+1 { + t.Errorf("itemCount() = %d, want %d", m.itemCount(), len(p.Models)+1) } } func TestModelTUI_SelectedModel(t *testing.T) { - p := llm.ListProviders() - if len(p) == 0 || len(p[0].Models) == 0 { - t.Skip("need provider with models") - } - m := newModelTUI(p[0], "") + p := firstRegistryProvider(t, 1) + m := newModelTUI(p, "") got := m.selectedModel() - if got != p[0].Models[0] { - t.Errorf("selectedModel() = %q, want %q", got, p[0].Models[0]) + if got != p.Models[0] { + t.Errorf("selectedModel() = %q, want %q", got, p.Models[0]) } } func TestModelTUI_SelectedModel_OutOfBounds(t *testing.T) { - p := llm.ListProviders() - if len(p) == 0 { - t.Skip("no providers") - } - m := newModelTUI(p[0], "") + p := firstRegistryProvider(t, 0) + m := newModelTUI(p, "") m.modelIdx = 9999 got := m.selectedModel() if got != "" { @@ -824,11 +790,8 @@ func TestModelTUI_SelectedModel_OutOfBounds(t *testing.T) { } func TestModelTUI_Update_UpDown(t *testing.T) { - p := llm.ListProviders() - if len(p) == 0 || len(p[0].Models) < 2 { - t.Skip("need provider with at least 2 models") - } - m := newModelTUI(p[0], "") + p := firstRegistryProvider(t, 2) + m := newModelTUI(p, "") result, _ := m.Update(downKey()) m2 := result.(modelTUIModel) if m2.modelIdx != 1 { @@ -842,11 +805,8 @@ func TestModelTUI_Update_UpDown(t *testing.T) { } func TestModelTUI_Update_WindowSize(t *testing.T) { - p := llm.ListProviders() - if len(p) == 0 { - t.Skip("no providers") - } - m := newModelTUI(p[0], "") + p := firstRegistryProvider(t, 0) + m := newModelTUI(p, "") result, _ := m.Update(tea.WindowSizeMsg{Width: 100, Height: 50}) m2 := result.(modelTUIModel) if m2.width != 100 || m2.height != 50 { @@ -855,11 +815,8 @@ func TestModelTUI_Update_WindowSize(t *testing.T) { } func TestModelTUI_Update_EscCancels(t *testing.T) { - p := llm.ListProviders() - if len(p) == 0 { - t.Skip("no providers") - } - m := newModelTUI(p[0], "") + p := firstRegistryProvider(t, 0) + m := newModelTUI(p, "") result, _ := m.Update(escKey()) m2 := result.(modelTUIModel) if !m2.cancelled { @@ -883,8 +840,7 @@ func TestProviderTUIView_StepProvider_CustomTab(t *testing.T) { "my-llm": {URL: "http://localhost", Model: "m"}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, "", tabCustom) v := m.View() if !strings.Contains(v.Content, "my-llm") { t.Errorf("expected custom provider name in view, got %q", v.Content) @@ -892,8 +848,7 @@ func TestProviderTUIView_StepProvider_CustomTab(t *testing.T) { } func TestProviderTUIView_StepProvider_CustomTab_CreatingCustom(t *testing.T) { - m := newProviderTUI(&Config{}, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(&Config{}, "", tabCustom) m.creatingCustom = true v := m.View() if !strings.Contains(v.Content, "Add Custom Provider") { @@ -907,8 +862,7 @@ func TestProviderTUIView_StepProvider_CustomTab_EditingCustom(t *testing.T) { "ed": {URL: "http://ed"}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, "", tabCustom) m.editingCustom = true m.editTargetName = "ed" v := m.View() @@ -918,8 +872,7 @@ func TestProviderTUIView_StepProvider_CustomTab_EditingCustom(t *testing.T) { } func TestProviderTUIView_StepProvider_ManualTab(t *testing.T) { - m := newProviderTUI(&Config{}, "") - m.activeTab = tabManual + m := newProviderTUIOnTab(&Config{}, "", tabManual) v := m.View() if !strings.Contains(v.Content, "Manual") { t.Errorf("expected 'Manual' in view") @@ -927,8 +880,7 @@ func TestProviderTUIView_StepProvider_ManualTab(t *testing.T) { } func TestProviderTUIView_StepProvider_ManualTab_InForm(t *testing.T) { - m := newProviderTUI(&Config{}, "") - m.activeTab = tabManual + m := newProviderTUIOnTab(&Config{}, "", tabManual) m.inManualForm = true v := m.View() if !strings.Contains(v.Content, "Manual Configuration") { @@ -942,8 +894,7 @@ func TestProviderTUIView_StepProvider_ConfirmingDelete(t *testing.T) { "del": {URL: "http://del"}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, "", tabCustom) m.confirmingDelete = true m.deleteTargetName = "del" v := m.View() @@ -1028,11 +979,8 @@ func TestRenderTabBar_AllTabs(t *testing.T) { } func TestModelTUI_View(t *testing.T) { - p := llm.ListProviders() - if len(p) == 0 { - t.Skip("no providers") - } - m := newModelTUI(p[0], "") + p := firstRegistryProvider(t, 0) + m := newModelTUI(p, "") v := m.View() if !strings.Contains(v.Content, "Select a model") { t.Errorf("expected 'Select a model' in view, got %q", v.Content) @@ -1040,11 +988,8 @@ func TestModelTUI_View(t *testing.T) { } func TestModelTUI_View_CustomModel(t *testing.T) { - p := llm.ListProviders() - if len(p) == 0 { - t.Skip("no providers") - } - m := newModelTUI(p[0], "") + p := firstRegistryProvider(t, 0) + m := newModelTUI(p, "") m.customModel = true v := m.View() if v.Content == "" { @@ -1052,6 +997,53 @@ func TestModelTUI_View_CustomModel(t *testing.T) { } } +func newProviderTUIOnTab(cfg *Config, configPath string, tab providerTab) providerTUIModel { + m := newProviderTUI(cfg, configPath) + m.activeTab = tab + return m +} + +func firstRegistryProvider(t *testing.T, minModels int) llm.Provider { + t.Helper() + p := llm.ListProviders() + if len(p) == 0 || len(p[0].Models) < minModels { + t.Skipf("need a registry provider with at least %d models", minModels) + } + return p[0] +} + +func activeDeepseekConfig() *Config { + return &Config{ + Provider: "deepseek", + Model: "deepseek-v4-flash", + Providers: map[string]ProviderEntry{ + "deepseek": {Model: "deepseek-v4-flash"}, + }, + } +} + +func selectOfficialProvider(t *testing.T, m *providerTUIModel, name string) { + t.Helper() + for i, p := range m.providers { + if p.Name == name { + m.officialIdx = i + return + } + } + t.Fatalf("official provider %q not found", name) +} + +func selectCustomProvider(t *testing.T, m *providerTUIModel, name string) { + t.Helper() + for i, cp := range m.customProviders { + if cp.name == name { + m.customIdx = i + return + } + } + t.Fatalf("custom provider %q not found", name) +} + func officialConfigModelTUI(t *testing.T, configPath string, extraModels []string) modelTUIModel { t.Helper() preset, ok := llm.LookupProvider("dashscope") @@ -1167,12 +1159,8 @@ func TestModelTUI_Official_AddCustomModelStaysOnList(t *testing.T) { if m3.existingCfg.Model != "qwen3.7-max" { t.Errorf("cfg.Model = %q, want active model unchanged", m3.existingCfg.Model) } - diskCfg, err := loadOrCreateConfig(configPath) - if err != nil { - t.Fatalf("load disk config: %v", err) - } - if !llm.ModelListContains(diskCfg.Providers["dashscope"].Models, "new-model") { - t.Errorf("disk Models = %v, want new-model persisted", diskCfg.Providers["dashscope"].Models) + if disk := loadDiskConfig(t, configPath).Providers["dashscope"].Models; !llm.ModelListContains(disk, "new-model") { + t.Errorf("disk Models = %v, want new-model persisted", disk) } } @@ -1267,29 +1255,8 @@ func TestModelTUI_Official_DeleteUserAddedModel(t *testing.T) { result, _ = m2.Update(yKey()) m3 := result.(modelTUIModel) - got := m3.existingCfg.Providers["dashscope"].Models - if len(got) != 1 || got[0] != "qwen3.7-max" { - t.Errorf("Models = %v, want [qwen3.7-max]", got) - } - - diskCfg, err := loadOrCreateConfig(configPath) - if err != nil { - t.Fatalf("load disk config: %v", err) - } - if len(diskCfg.Providers["dashscope"].Models) != 1 { - t.Errorf("disk Models = %v, want [qwen3.7-max]", diskCfg.Providers["dashscope"].Models) - } -} - -func TestModelTUI_Official_DeleteBuiltInModelIgnored(t *testing.T) { - m := officialConfigModelTUI(t, "", []string{"my-custom-model"}) - m.modelIdx = modelTUIIdxForName(t, m, "qwen3.7-max") - - result, _ := m.Update(dKey()) - m2 := result.(modelTUIModel) - if m2.confirmingDeleteModel { - t.Error("pressing d on built-in model should not trigger delete confirmation") - } + assertModels(t, "Models", m3.existingCfg.Providers["dashscope"].Models, "qwen3.7-max") + assertModels(t, "disk Models", loadDiskConfig(t, configPath).Providers["dashscope"].Models, "qwen3.7-max") } func TestIsUserAddedOfficialModelName_RegistryDuplicateNotUserAdded(t *testing.T) { @@ -1310,32 +1277,6 @@ func TestIsUserAddedOfficialModelName_RegistryDuplicateNotUserAdded(t *testing.T } } -func TestModelTUI_Official_RegistryModelNotDeletable(t *testing.T) { - m := officialConfigModelTUI(t, "", []string{"my-custom-model"}) - m.modelIdx = modelTUIIdxForName(t, m, "qwen3.7-max") - - if m.isUserAddedModel("qwen3.7-max") { - t.Error("qwen3.7-max should not be user-added when it is in the registry") - } - - result, _ := m.Update(dKey()) - m2 := result.(modelTUIModel) - if m2.confirmingDeleteModel { - t.Error("pressing d on registry model should not trigger delete confirmation") - } -} - -func TestModelTUI_Official_DeleteOnCustomModelInputIgnored(t *testing.T) { - m := officialConfigModelTUI(t, "", []string{"my-custom-model"}) - m.modelIdx = len(m.displayModels()) - - result, _ := m.Update(dKey()) - m2 := result.(modelTUIModel) - if m2.confirmingDeleteModel { - t.Error("pressing d on Enter custom model name... should not trigger delete confirmation") - } -} - func TestModelTUI_CustomProvider_DeleteOnCustomModelInputIgnored(t *testing.T) { m := customConfigModelTUI(t, "", []string{"m1", "aaa"}) m.modelIdx = len(m.displayModels()) @@ -1347,38 +1288,6 @@ func TestModelTUI_CustomProvider_DeleteOnCustomModelInputIgnored(t *testing.T) { } } -func TestModelTUI_Official_UserAddedModelShowsDeleteHint(t *testing.T) { - m := officialConfigModelTUI(t, "", []string{"my-custom-model"}) - - m.modelIdx = modelTUIIdxForName(t, m, "qwen3.7-max") - got := stripANSI(m.View().Content) - if strings.Contains(got, "d Delete") { - t.Errorf("built-in model should not show d Delete hint; got:\n%s", got) - } - - m.modelIdx = modelTUIIdxForName(t, m, "my-custom-model") - got = stripANSI(m.View().Content) - if !strings.Contains(got, "d Delete") { - t.Errorf("user-added model should show d Delete hint; got:\n%s", got) - } -} - -func TestModelTUI_CustomProvider_ShowsDeleteHint(t *testing.T) { - m := customConfigModelTUI(t, "", []string{"m1", "aaa"}) - - m.modelIdx = len(m.displayModels()) - got := stripANSI(m.View().Content) - if strings.Contains(got, "d Delete") { - t.Errorf("custom input row should not show d Delete hint; got:\n%s", got) - } - - m.modelIdx = modelTUIIdxForName(t, m, "aaa") - got = stripANSI(m.View().Content) - if !strings.Contains(got, "d Delete") { - t.Errorf("custom model row should show d Delete hint; got:\n%s", got) - } -} - func TestModelTUI_CustomProvider_DeleteClearsCustomInput(t *testing.T) { dir := t.TempDir() configPath := filepath.Join(dir, "config.json") @@ -1414,10 +1323,7 @@ func TestModelTUI_CustomProvider_DeleteModelViaDKey(t *testing.T) { result, _ = m2.Update(yKey()) m3 := result.(modelTUIModel) - got := m3.existingCfg.CustomProviders["my-llm"].Models - if len(got) != 1 || got[0] != "m1" { - t.Errorf("Models = %v, want [m1]", got) - } + assertModels(t, "Models", m3.existingCfg.CustomProviders["my-llm"].Models, "m1") } func TestModelTUI_CustomProvider_DeleteCancel(t *testing.T) { @@ -1437,62 +1343,6 @@ func TestModelTUI_CustomProvider_DeleteCancel(t *testing.T) { } } -func TestModelTUI_Official_DeleteCancel(t *testing.T) { - cancelKeys := []struct { - name string - key tea.KeyPressMsg - }{ - {"n", nKey()}, - {"esc", escKey()}, - } - for _, tc := range cancelKeys { - t.Run(tc.name, func(t *testing.T) { - m := officialConfigModelTUI(t, "", []string{"my-custom-model"}) - m.modelIdx = modelTUIIdxForName(t, m, "my-custom-model") - - result, _ := m.Update(dKey()) - m2 := result.(modelTUIModel) - if !m2.confirmingDeleteModel { - t.Fatal("expected confirmingDeleteModel after d") - } - - result, _ = m2.Update(tc.key) - m3 := result.(modelTUIModel) - if m3.confirmingDeleteModel { - t.Error("confirmingDeleteModel should be false after cancel") - } - got := m3.existingCfg.Providers["dashscope"].Models - if len(got) != 2 || got[1] != "my-custom-model" { - t.Errorf("Models = %v, want model unchanged", got) - } - }) - } -} - -func TestModelTUI_Official_DeleteActiveUserModelClearsCfg(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, "config.json") - m := officialConfigModelTUI(t, configPath, []string{"my-custom-model"}) - m.existingCfg.Model = "my-custom-model" - m.existingCfg.Providers["dashscope"] = ProviderEntry{ - Model: "my-custom-model", - Models: []string{"qwen3.7-max", "my-custom-model"}, - } - m.modelIdx = modelTUIIdxForName(t, m, "my-custom-model") - - result, _ := m.Update(dKey()) - m2 := result.(modelTUIModel) - result, _ = m2.Update(yKey()) - m3 := result.(modelTUIModel) - - if m3.existingCfg.Providers["dashscope"].Model != "" { - t.Errorf("entry.Model = %q, want empty", m3.existingCfg.Providers["dashscope"].Model) - } - if m3.existingCfg.Model != "" { - t.Errorf("cfg.Model = %q, want empty", m3.existingCfg.Model) - } -} - // --- result() tests --- func TestResult_OfficialTab(t *testing.T) { @@ -1514,8 +1364,7 @@ func TestResult_OfficialTab_WithMaskedKey(t *testing.T) { } func TestResult_CustomTab_Creating(t *testing.T) { - m := newProviderTUI(&Config{}, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(&Config{}, "", tabCustom) m.creatingCustom = true m.cpNameInput.SetValue("new-prov") m.cpURLInput.SetValue("http://url") @@ -1534,8 +1383,7 @@ func TestResult_CustomTab_Editing(t *testing.T) { "ed": {URL: "http://ed", Model: "m1", Models: []string{"m1", "m2"}}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, "", tabCustom) m.editingCustom = true m.editTargetName = "ed" m.cpNameInput.SetValue("ed") @@ -1555,8 +1403,7 @@ func TestResult_CustomTab_Selected(t *testing.T) { "sel": {URL: "http://sel", Model: "gpt", Models: []string{"gpt"}}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, "", tabCustom) m.customIdx = 0 r := m.result() if !r.isCustom { @@ -1568,8 +1415,7 @@ func TestResult_CustomTab_Selected(t *testing.T) { } func TestResult_CustomTab_OutOfBounds(t *testing.T) { - m := newProviderTUI(&Config{}, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(&Config{}, "", tabCustom) m.customIdx = 999 r := m.result() if r.provider != "" { @@ -1578,8 +1424,7 @@ func TestResult_CustomTab_OutOfBounds(t *testing.T) { } func TestResult_ManualTab(t *testing.T) { - m := newProviderTUI(&Config{}, "") - m.activeTab = tabManual + m := newProviderTUIOnTab(&Config{}, "", tabManual) m.manualURLInput.SetValue("http://manual") m.manualModelInput.SetValue("model-x") r := m.result() @@ -1595,8 +1440,7 @@ func TestResult_ManualTab(t *testing.T) { } func TestResult_ManualTab_MaskedToken(t *testing.T) { - m := newProviderTUI(&Config{}, "") - m.activeTab = tabManual + m := newProviderTUIOnTab(&Config{}, "", tabManual) m.manualTokenMasked = true m.manualTokenOriginal = "tok-secret" r := m.result() @@ -1613,8 +1457,7 @@ func TestLoadExistingAPIKey_CustomTab_HasKey(t *testing.T) { "cp": {URL: "http://cp", APIKey: "sk-key"}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, "", tabCustom) m.customIdx = 0 m.loadExistingAPIKey() if !m.apiKeyMasked { @@ -1631,8 +1474,7 @@ func TestLoadExistingAPIKey_CustomTab_NoKey(t *testing.T) { "cp": {URL: "http://cp"}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, "", tabCustom) m.customIdx = 0 m.loadExistingAPIKey() if m.apiKeyMasked { @@ -1712,28 +1554,10 @@ func TestSyncSessionModelSelection_EmptyModel(t *testing.T) { func TestSyncSessionModelSelection_CrossOfficialProviderNoPersist(t *testing.T) { dir := t.TempDir() configPath := filepath.Join(dir, "config.json") - cfg := &Config{ - Provider: "deepseek", - Model: "deepseek-v4-flash", - Providers: map[string]ProviderEntry{ - "deepseek": {Model: "deepseek-v4-flash"}, - }, - } - m := newProviderTUI(cfg, configPath) - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "baidu-qianfan" { - m.officialIdx = i - break - } - } - m.modelIdx = 0 - for i, name := range m.models() { - if name == "glm-5" { - m.modelIdx = i - break - } - } + cfg := activeDeepseekConfig() + m := newProviderTUIOnTab(cfg, configPath, tabOfficial) + selectOfficialProvider(t, &m, "baidu-qianfan") + m.modelIdx = modelIdxForName(t, m, "glm-5") if err := m.syncSessionModelSelection(); err != nil { t.Fatalf("unexpected error: %v", err) @@ -1752,21 +1576,9 @@ func TestSyncSessionModelSelection_CrossOfficialProviderNoPersist(t *testing.T) func TestSyncSessionModelSelection_ActiveOfficialProviderDefersPersist(t *testing.T) { dir := t.TempDir() configPath := filepath.Join(dir, "config.json") - cfg := &Config{ - Provider: "deepseek", - Model: "deepseek-v4-flash", - Providers: map[string]ProviderEntry{ - "deepseek": {Model: "deepseek-v4-flash"}, - }, - } - m := newProviderTUI(cfg, configPath) - m.activeTab = tabOfficial - for i, name := range m.models() { - if name == "deepseek-v4-pro" { - m.modelIdx = i - break - } - } + cfg := activeDeepseekConfig() + m := newProviderTUIOnTab(cfg, configPath, tabOfficial) + m.modelIdx = modelIdxForName(t, m, "deepseek-v4-pro") if err := m.syncSessionModelSelection(); err != nil { t.Fatalf("unexpected error: %v", err) @@ -1804,14 +1616,8 @@ func TestSyncSessionModelSelection_CrossCustomProviderNoPersist(t *testing.T) { }, }, } - m := newProviderTUI(cfg, configPath) - m.activeTab = tabCustom - for i, cp := range m.customProviders { - if cp.name == "other" { - m.customIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, configPath, tabCustom) + selectCustomProvider(t, &m, "other") m.modelIdx = 0 if err := m.syncSessionModelSelection(); err != nil { @@ -1828,27 +1634,10 @@ func TestSyncSessionModelSelection_CrossCustomProviderNoPersist(t *testing.T) { func TestSyncSessionModelSelection_RecordsSessionPickForInactiveOfficialProvider(t *testing.T) { dir := t.TempDir() configPath := filepath.Join(dir, "config.json") - cfg := &Config{ - Provider: "deepseek", - Model: "deepseek-v4-flash", - Providers: map[string]ProviderEntry{ - "deepseek": {Model: "deepseek-v4-flash"}, - }, - } - m := newProviderTUI(cfg, configPath) - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "baidu-qianfan" { - m.officialIdx = i - break - } - } - for i, name := range m.models() { - if name == "glm-5" { - m.modelIdx = i - break - } - } + cfg := activeDeepseekConfig() + m := newProviderTUIOnTab(cfg, configPath, tabOfficial) + selectOfficialProvider(t, &m, "baidu-qianfan") + m.modelIdx = modelIdxForName(t, m, "glm-5") if err := m.syncSessionModelSelection(); err != nil { t.Fatalf("unexpected error: %v", err) @@ -1865,21 +1654,9 @@ func TestSyncSessionModelSelection_RecordsSessionPickForInactiveOfficialProvider } func TestProviderTUI_ResultUsesSessionModelPickWhenSelectionEmpty(t *testing.T) { - cfg := &Config{ - Provider: "deepseek", - Model: "deepseek-v4-flash", - Providers: map[string]ProviderEntry{ - "deepseek": {Model: "deepseek-v4-flash"}, - }, - } - m := newProviderTUI(cfg, "") - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "baidu-qianfan" { - m.officialIdx = i - break - } - } + cfg := activeDeepseekConfig() + m := newProviderTUIOnTab(cfg, "", tabOfficial) + selectOfficialProvider(t, &m, "baidu-qianfan") m.sessionModelPick = map[string]string{"baidu-qianfan": "glm-5"} m.modelIdx = 9999 // force selectedModelFromState() empty @@ -2082,8 +1859,7 @@ func TestHandleManualFormEnter_AuthTokenGate(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - m := newProviderTUI(&Config{Llm: tc.llmCfg}, "") - m.activeTab = tabManual + m := newProviderTUIOnTab(&Config{Llm: tc.llmCfg}, "", tabManual) m.inManualForm = true m.manualStep = manualStepAuthToken if tc.typedToken != "" { @@ -2123,8 +1899,7 @@ func TestHandleManualFormEnter_AuthTokenGate(t *testing.T) { // --- viewCustomProviderForm field steps --- func TestProviderTUIView_CustomForm_AllSteps(t *testing.T) { - m := newProviderTUI(&Config{}, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(&Config{}, "", tabCustom) m.creatingCustom = true for _, step := range []customProviderStep{cpStepName, cpStepBaseURL, cpStepAPIKey, cpStepAuthHeader, cpStepProtocol} { m.cpStep = step @@ -2136,8 +1911,7 @@ func TestProviderTUIView_CustomForm_AllSteps(t *testing.T) { } func TestProviderTUIView_CustomForm_WithError(t *testing.T) { - m := newProviderTUI(&Config{}, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(&Config{}, "", tabCustom) m.creatingCustom = true m.formError = "name is required" v := m.View() @@ -2149,8 +1923,7 @@ func TestProviderTUIView_CustomForm_WithError(t *testing.T) { // --- viewManualTab field steps --- func TestProviderTUIView_ManualForm_AllSteps(t *testing.T) { - m := newProviderTUI(&Config{}, "") - m.activeTab = tabManual + m := newProviderTUIOnTab(&Config{}, "", tabManual) m.inManualForm = true for _, step := range []manualStep{manualStepURL, manualStepProtocol, manualStepModel, manualStepAuthToken, manualStepAuthHeader} { m.manualStep = step @@ -2162,8 +1935,7 @@ func TestProviderTUIView_ManualForm_AllSteps(t *testing.T) { } func TestProviderTUIView_ManualForm_WithError(t *testing.T) { - m := newProviderTUI(&Config{}, "") - m.activeTab = tabManual + m := newProviderTUIOnTab(&Config{}, "", tabManual) m.inManualForm = true m.formError = "URL required" v := m.View() @@ -2176,8 +1948,7 @@ func TestProviderTUIView_ManualTab_WithExistingConfig(t *testing.T) { cfg := &Config{ Llm: LlmConfig{URL: "http://existing", Model: "old-model"}, } - m := newProviderTUI(cfg, "") - m.activeTab = tabManual + m := newProviderTUIOnTab(cfg, "", tabManual) v := m.View() if !strings.Contains(v.Content, "http://existing") { t.Errorf("expected existing URL in manual tab") diff --git a/cmd/opencodereview/provider_tui_persist_test.go b/cmd/opencodereview/provider_tui_persist_test.go index cd37c139b..9db168867 100644 --- a/cmd/opencodereview/provider_tui_persist_test.go +++ b/cmd/opencodereview/provider_tui_persist_test.go @@ -33,13 +33,8 @@ func TestPersistCustomModelName(t *testing.T) { t.Run("custom tab success", func(t *testing.T) { path := filepath.Join(t.TempDir(), "config.json") - cfg := &Config{ - CustomProviders: map[string]ProviderEntry{ - "cp": {URL: "https://x.example", Protocol: "openai", Models: []string{"m1"}}, - }, - } - m := newProviderTUI(cfg, path) - m.activeTab = tabCustom + cfg := cpConfig() + m := newProviderTUIOnTab(cfg, path, tabCustom) m.customIdx = 0 persisted, err := m.persistCustomModelName("m2") if err != nil || !persisted { @@ -53,12 +48,8 @@ func TestPersistCustomModelName(t *testing.T) { t.Run("official tab success", func(t *testing.T) { path := filepath.Join(t.TempDir(), "config.json") cfg := &Config{} - m := newProviderTUI(cfg, path) - m.activeTab = tabOfficial - provider := m.currentProvider() - if provider.Name == "" { - t.Skip("no official provider available") - } + m := newProviderTUIOnTab(cfg, path, tabOfficial) + provider := currentProviderOrSkip(t, m) persisted, err := m.persistCustomModelName("my-model") if err != nil || !persisted { t.Fatalf("got (%v, %v), want (true, nil)", persisted, err) @@ -69,13 +60,8 @@ func TestPersistCustomModelName(t *testing.T) { }) t.Run("custom tab save failure rolls back", func(t *testing.T) { - cfg := &Config{ - CustomProviders: map[string]ProviderEntry{ - "cp": {URL: "https://x.example", Protocol: "openai", Models: []string{"m1"}}, - }, - } - m := newProviderTUI(cfg, unwritableConfigPath(t)) - m.activeTab = tabCustom + cfg := cpConfig() + m := newProviderTUIOnTab(cfg, unwritableConfigPath(t), tabCustom) m.customIdx = 0 persisted, err := m.persistCustomModelName("m2") if err == nil || persisted { @@ -88,12 +74,8 @@ func TestPersistCustomModelName(t *testing.T) { t.Run("official tab save failure rolls back", func(t *testing.T) { cfg := &Config{} - m := newProviderTUI(cfg, unwritableConfigPath(t)) - m.activeTab = tabOfficial - provider := m.currentProvider() - if provider.Name == "" { - t.Skip("no official provider available") - } + m := newProviderTUIOnTab(cfg, unwritableConfigPath(t), tabOfficial) + provider := currentProviderOrSkip(t, m) persisted, err := m.persistCustomModelName("my-model") if err == nil || persisted { t.Fatalf("got (%v, %v), want (false, error)", persisted, err) @@ -111,13 +93,8 @@ func TestUpdateCustomModelInput(t *testing.T) { newModel := func(t *testing.T) providerTUIModel { t.Helper() path := filepath.Join(t.TempDir(), "config.json") - cfg := &Config{ - CustomProviders: map[string]ProviderEntry{ - "cp": {URL: "https://x.example", Protocol: "openai", Models: []string{"m1"}}, - }, - } - m := newProviderTUI(cfg, path) - m.activeTab = tabCustom + cfg := cpConfig() + m := newProviderTUIOnTab(cfg, path, tabCustom) m.customIdx = 0 m.customModel = true return m @@ -177,8 +154,7 @@ func TestUpdateCustomModelInput(t *testing.T) { func TestApplyCreateCustomProvider(t *testing.T) { setup := func(t *testing.T, cfg *Config, path string) providerTUIModel { t.Helper() - m := newProviderTUI(cfg, path) - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, path, tabCustom) m.creatingCustom = true m.cpProtocolIdx = 0 return m @@ -392,3 +368,11 @@ func TestConfirmDeleteOfficialModelActiveClear(t *testing.T) { t.Error("active Model should be cleared after deleting the active model") } } + +func cpConfig() *Config { + return &Config{ + CustomProviders: map[string]ProviderEntry{ + "cp": {URL: "https://x.example", Protocol: "openai", Models: []string{"m1"}}, + }, + } +} diff --git a/cmd/opencodereview/provider_tui_savefail_test.go b/cmd/opencodereview/provider_tui_savefail_test.go index b1cb4846b..6c2a7a350 100644 --- a/cmd/opencodereview/provider_tui_savefail_test.go +++ b/cmd/opencodereview/provider_tui_savefail_test.go @@ -43,8 +43,7 @@ func TestUpdateDeleteConfirm_SaveFailure(t *testing.T) { "cp": {URL: "https://x.example", Protocol: "openai", Models: []string{"m1"}}, }, } - m := newProviderTUI(cfg, unwritableConfigPath(t)) - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, unwritableConfigPath(t), tabCustom) m.confirmingDelete = true m.deleteTargetIdx = 0 m.deleteTargetName = "cp" @@ -52,12 +51,7 @@ func TestUpdateDeleteConfirm_SaveFailure(t *testing.T) { out, _ := m.updateDeleteConfirm("y") got := out.(providerTUIModel) - if !strings.Contains(got.formError, "failed to save") { - t.Errorf("formError = %q, want save-failure message", got.formError) - } - if got.savedInSession { - t.Error("savedInSession should be false after save failure") - } + assertSaveFailed(t, got.formError, got.savedInSession) if got.confirmingDelete { t.Error("confirmingDelete should be cleared after handling") } @@ -79,8 +73,7 @@ func TestConfirmDeleteCustomModel_SaveFailureRollback(t *testing.T) { }, }, } - m := newProviderTUI(cfg, unwritableConfigPath(t)) - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, unwritableConfigPath(t), tabCustom) m.customIdx = 0 m.step = stepModel m.modelIdx = 1 @@ -90,12 +83,7 @@ func TestConfirmDeleteCustomModel_SaveFailureRollback(t *testing.T) { out, _ := m.confirmDeleteCustomModel() got := out.(providerTUIModel) - if !strings.Contains(got.formError, "failed to save") { - t.Errorf("formError = %q, want save-failure message", got.formError) - } - if got.savedInSession { - t.Error("savedInSession should be false after save failure") - } + assertSaveFailed(t, got.formError, got.savedInSession) // Rollback restored the model list in the in-memory config. entry := got.existingCfg.CustomProviders["cp"] if len(entry.Models) != 2 { @@ -110,12 +98,8 @@ func TestConfirmDeleteCustomModel_SaveFailureRollback(t *testing.T) { // delete handler down its save-failure branch for a user-added model and asserts // the provider entry is rolled back. func TestConfirmDeleteOfficialModel_SaveFailureRollback(t *testing.T) { - m := newProviderTUI(&Config{}, unwritableConfigPath(t)) - m.activeTab = tabOfficial - provider := m.currentProvider() - if provider.Name == "" { - t.Skip("no official provider available") - } + m := newProviderTUIOnTab(&Config{}, unwritableConfigPath(t), tabOfficial) + provider := currentProviderOrSkip(t, m) userModel := "user-added-model-xyz" cfg := &Config{ Provider: provider.Name, @@ -136,12 +120,7 @@ func TestConfirmDeleteOfficialModel_SaveFailureRollback(t *testing.T) { out, _ := m.confirmDeleteOfficialModel() got := out.(providerTUIModel) - if !strings.Contains(got.formError, "failed to save") { - t.Errorf("formError = %q, want save-failure message", got.formError) - } - if got.savedInSession { - t.Error("savedInSession should be false after save failure") - } + assertSaveFailed(t, got.formError, got.savedInSession) // Rollback restored the user-added model. entry := got.existingCfg.Providers[provider.Name] if !containsStr(entry.Models, userModel) { @@ -184,12 +163,7 @@ func TestModelTUIConfirmDeleteModel_SaveFailureRollback(t *testing.T) { out, _ := m.confirmDeleteModel() got := out.(modelTUIModel) - if !strings.Contains(got.formError, "failed to save") { - t.Errorf("formError = %q, want save-failure message", got.formError) - } - if got.savedInSession { - t.Error("savedInSession should be false after save failure") - } + assertSaveFailed(t, got.formError, got.savedInSession) if !containsStr(entries[provider].Models, userModel) { t.Errorf("rolled-back models = %v, want to contain %q", entries[provider].Models, userModel) } @@ -200,6 +174,25 @@ func TestModelTUIConfirmDeleteModel_SaveFailureRollback(t *testing.T) { } } +func assertSaveFailed(t *testing.T, formError string, savedInSession bool) { + t.Helper() + if !strings.Contains(formError, "failed to save") { + t.Errorf("formError = %q, want save-failure message", formError) + } + if savedInSession { + t.Error("savedInSession should be false after save failure") + } +} + +func currentProviderOrSkip(t *testing.T, m providerTUIModel) llm.Provider { + t.Helper() + provider := m.currentProvider() + if provider.Name == "" { + t.Skip("no official provider available") + } + return provider +} + func containsStr(list []string, want string) bool { for _, s := range list { if s == want { diff --git a/cmd/opencodereview/provider_tui_test.go b/cmd/opencodereview/provider_tui_test.go index 5edf3e9b3..f08cfb08c 100644 --- a/cmd/opencodereview/provider_tui_test.go +++ b/cmd/opencodereview/provider_tui_test.go @@ -6,6 +6,7 @@ package main import ( "os" "path/filepath" + "slices" "sort" "strings" "testing" @@ -301,8 +302,7 @@ func TestProviderTUI_ManualFormEscRefocusesPreviousInput(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - m := newProviderTUI(&Config{}, "") - m.activeTab = tabManual + m := newProviderTUIOnTab(&Config{}, "", tabManual) m.inManualForm = true m.manualStep = tt.fromStep @@ -545,14 +545,8 @@ func TestProviderTUI_SessionModelPickSurvivesOfficialProviderSwitch(t *testing.T "deepseek": {Model: "deepseek-v4-flash"}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "baidu-qianfan" { - m.officialIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, "", tabOfficial) + selectOfficialProvider(t, &m, "baidu-qianfan") result, _ := m.Update(enterKey()) m2 := result.(providerTUIModel) @@ -613,24 +607,14 @@ func TestProviderTUI_CustomTabSelectAddStartsForm(t *testing.T) { } func TestProviderTUI_CustomFormEscFromNameExitsForm(t *testing.T) { - m := newProviderTUI(&Config{}, "") - - // Switch to custom tab and start form - result, _ := m.Update(rightKey()) - m2 := result.(providerTUIModel) - result, _ = m2.Update(enterKey()) - m3 := result.(providerTUIModel) - if !m3.creatingCustom { - t.Fatalf("should be creating custom") - } + m := openCustomProviderForm(t, &Config{}, "") // Esc from name step should exit form - result, _ = m3.Update(escKey()) - m4 := result.(providerTUIModel) - if m4.creatingCustom { + m = sendKey(m, escKey()) + if m.creatingCustom { t.Error("Esc from name step should exit custom form") } - if m4.cancelled { + if m.cancelled { t.Error("should not be cancelled") } } @@ -684,47 +668,24 @@ 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) - } + configPath := filepath.Join(t.TempDir(), "config.json") + m := fillCustomFormToAuthHeader(t, openCustomProviderForm(t, &Config{}, configPath), "") for _, c := range "bad-header" { - result, _ = m7.Update(charKey(c)) - m7 = result.(providerTUIModel) + m = sendKey(m, charKey(c)) } - result, _ = m7.Update(enterKey()) - m8 := result.(providerTUIModel) + m = sendKey(m, enterKey()) - if m8.cpStep != cpStepAuthHeader { - t.Errorf("cpStep = %d, want %d", m8.cpStep, cpStepAuthHeader) + if m.cpStep != cpStepAuthHeader { + t.Errorf("cpStep = %d, want %d", m.cpStep, cpStepAuthHeader) } - if m8.formError == "" { + if m.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 !strings.Contains(m.formError, "Unsupported Auth Header") { + t.Errorf("formError = %q, want unsupported auth header message", m.formError) } - if !m8.creatingCustom { + if !m.creatingCustom { t.Error("creatingCustom should remain true when validation fails") } if _, err := os.Stat(configPath); err == nil { @@ -744,8 +705,7 @@ func TestProviderTUI_CustomFormEditRejectsInvalidAuthHeader(t *testing.T) { }, }, } - m := newProviderTUI(cfg, configPath) - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, configPath, tabCustom) m.customIdx = 0 m.enterEditCustomProvider() m.cpStep = cpStepAuthHeader @@ -784,8 +744,7 @@ func TestProviderTUI_EditCustomProviderSaveRejectsDuplicateRename(t *testing.T) }, }, } - m := newProviderTUI(cfg, configPath) - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, configPath, tabCustom) m.editingCustom = true m.editTargetName = "other" m.cpProtocolIdx = 1 // openai @@ -821,8 +780,7 @@ func TestApplyEditCustomProviderSave_ClearsAPIKeyWhenEditedEmpty(t *testing.T) { }, }, } - m := newProviderTUI(cfg, configPath) - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, configPath, tabCustom) m.editingCustom = true m.editTargetName = "aaa" m.cpProtocolIdx = 0 @@ -836,10 +794,7 @@ func TestApplyEditCustomProviderSave_ClearsAPIKeyWhenEditedEmpty(t *testing.T) { if got := cfg.CustomProviders["aaa"].APIKey; got != "" { t.Errorf("APIKey = %q, want empty", got) } - diskCfg, err := loadOrCreateConfig(configPath) - if err != nil { - t.Fatalf("load config: %v", err) - } + diskCfg := loadDiskConfig(t, configPath) if got := diskCfg.CustomProviders["aaa"].APIKey; got != "" { t.Errorf("persisted APIKey = %q, want empty", got) } @@ -857,8 +812,7 @@ func TestApplyEditCustomProviderSave_PreservesAPIKeyWhenMasked(t *testing.T) { }, }, } - m := newProviderTUI(cfg, configPath) - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, configPath, tabCustom) m.customIdx = 0 m.enterEditCustomProvider() @@ -884,14 +838,8 @@ func TestProviderTUI_EditCustomClearKey_NoMaskedOnStepAPIKey(t *testing.T) { }, }, } - m := newProviderTUI(cfg, configPath) - m.activeTab = tabCustom - for i, cp := range m.customProviders { - if cp.name == "aaa" { - m.customIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, configPath, tabCustom) + selectCustomProvider(t, &m, "aaa") m.enterEditCustomProvider() m.cpStep = cpStepAPIKey m.beginAPIKeyReplace() @@ -935,8 +883,7 @@ func TestProviderTUI_ReenterEditAfterClearKey_ShowsEmpty(t *testing.T) { }, }, } - m := newProviderTUI(cfg, configPath) - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, configPath, tabCustom) m.customIdx = 0 m.editingCustom = true m.editTargetName = "aaa" @@ -982,51 +929,34 @@ func TestCustomAPIKeyForSave(t *testing.T) { } func TestProviderTUI_CustomFormCreateReturnsToModelList(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, "config.json") + configPath := filepath.Join(t.TempDir(), "config.json") cfg := &Config{} - m := newProviderTUI(cfg, configPath) + m := fillCustomFormToAuthHeader(t, openCustomProviderForm(t, cfg, configPath), "key-123") - 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) + result, cmd := m.Update(enterKey()) // auth header -> save + m = result.(providerTUIModel) if cmd != nil { t.Error("create should not quit TUI") } - if m8.creatingCustom { + if m.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 m.step != stepModel { + t.Errorf("step = %d, want stepModel", m.step) } - if len(m8.customProviders) != 1 { - t.Fatalf("expected 1 custom provider, got %d", len(m8.customProviders)) + if len(m.customProviders) != 1 { + t.Fatalf("expected 1 custom provider, got %d", len(m.customProviders)) } - if m8.customProviders[0].name != "my-new" { - t.Errorf("provider name = %q, want %q", m8.customProviders[0].name, "my-new") + if m.customProviders[0].name != "my-new" { + t.Errorf("provider name = %q, want %q", m.customProviders[0].name, "my-new") } if cfg.Provider != "" { t.Error("active provider should not be set when only creating") } - if !m8.savedInSession { + if !m.savedInSession { t.Error("savedInSession should be true after create") } if _, err := os.Stat(configPath); err != nil { @@ -1088,13 +1018,6 @@ func TestProviderTUI_SelectExistingCustomGoesToModel(t *testing.T) { // --- collectCustomProviders tests --- -func TestCollectCustomProviders_NilConfig(t *testing.T) { - result := collectCustomProviders(nil) - if result != nil { - t.Errorf("expected nil, got %v", result) - } -} - func TestCollectCustomProviders_ReadsCustomProviders(t *testing.T) { cfg := &Config{ Providers: map[string]ProviderEntry{ @@ -1315,224 +1238,151 @@ func TestActiveModelForProvider_FallsBackToCfgModel(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"}, - }, - }, - } - 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() +func TestProviderTUI_CustomModelInput_AddsModel(t *testing.T) { + for _, tab := range customModelInputTabs { + t.Run(tab.name, func(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + m := enterCustomModelName(tab.build(t, configPath), "newmodel") - 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) + if m.customModel { + t.Error("customModel should be cleared after Enter") + } + if m.formError != "" { + t.Errorf("formError = %q, want empty", m.formError) + } + if !m.savedInSession { + t.Error("savedInSession should be true after add") + } + assertModels(t, "Models", tab.models(m.existingCfg), tab.listed, "newmodel") + assertModels(t, "disk Models", tab.models(loadDiskConfig(t, configPath)), tab.listed, "newmodel") + }) } } 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) + for _, tab := range customModelInputTabs { + t.Run(tab.name, func(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + m := enterCustomModelName(tab.build(t, configPath), tab.listed) - 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") + if !m.customModel { + t.Error("customModel should stay true after duplicate reject") + } + if want := "Already in list: " + tab.listed; m.formError != want { + t.Errorf("formError = %q, want %q", m.formError, want) + } + if m.modelInput.Value() != tab.listed { + t.Errorf("input should be preserved on dup; got %q", m.modelInput.Value()) + } + assertModels(t, "Models", tab.models(m.existingCfg), tab.listed) + if _, err := os.Stat(configPath); err == nil { + t.Errorf("disk file should not exist; duplicate did not persist") + } + if m.savedInSession { + t.Error("savedInSession should be false after rejected duplicate") + } + }) } } -func TestProviderTUI_OfficialTab_CustomModelInput_PersistsName(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, "config.json") +func officialDashscopeModelTUI(t *testing.T, configPath string, extraModels []string) providerTUIModel { + t.Helper() + models := []string{"qwen3.7-max"} + models = append(models, extraModels...) cfg := &Config{ Provider: "dashscope", Model: "qwen3.7-max", Providers: map[string]ProviderEntry{ "dashscope": { Model: "qwen3.7-max", - Models: []string{"qwen3.7-max"}, + Models: models, }, }, } - m := newProviderTUI(cfg, configPath) - m.activeTab = tabOfficial - - // Land the cursor on the official provider we configured. - for i, p := range m.providers { - if p.Name == "dashscope" { - m.officialIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, configPath, tabOfficial) + selectOfficialProvider(t, &m, "dashscope") m.step = stepModel - m.modelIdx = len(m.models()) // "Enter custom model name..." - m.customModel = true - m.modelInput.SetValue("my-custom-model") - m.modelInput.Focus() + return m +} - result, _ := m.Update(enterKey()) - m2 := result.(providerTUIModel) +// Each tab starts with listed as its only model. +type customModelInputTab struct { + name string + build func(t *testing.T, configPath string) providerTUIModel + listed string + models func(cfg *Config) []string +} - if m2.customModel { - t.Error("customModel should be cleared after Enter") - } - if m2.formError != "" { - t.Errorf("formError = %q, want empty", m2.formError) - } - if !m2.savedInSession { - t.Error("savedInSession should be true after successful add") - } - got := m2.existingCfg.Providers["dashscope"].Models - want := []string{"qwen3.7-max", "my-custom-model"} - if len(got) != len(want) || got[0] != want[0] || got[1] != want[1] { - t.Errorf("official Models = %v, want %v", got, want) - } +var customModelInputTabs = []customModelInputTab{ + { + name: "custom tab", + build: func(t *testing.T, configPath string) providerTUIModel { + return customStepfunModelTUI(t, configPath, nil) + }, + listed: "step-3.5-flash", + models: func(cfg *Config) []string { return cfg.CustomProviders["stepfun"].Models }, + }, + { + name: "official tab", + build: func(t *testing.T, configPath string) providerTUIModel { + return officialDashscopeModelTUI(t, configPath, nil) + }, + listed: "qwen3.7-max", + models: func(cfg *Config) []string { return cfg.Providers["dashscope"].Models }, + }, +} - diskCfg, err := loadOrCreateConfig(configPath) +func enterCustomModelName(m providerTUIModel, name string) providerTUIModel { + m.modelIdx = len(m.models()) + m.customModel = true + m.modelInput.SetValue(name) + m.modelInput.Focus() + return sendKey(m, enterKey()) +} + +func loadDiskConfig(t *testing.T, configPath string) *Config { + t.Helper() + cfg, err := loadOrCreateConfig(configPath) if err != nil { t.Fatalf("load disk config: %v", err) } - diskModels := diskCfg.Providers["dashscope"].Models - if len(diskModels) != 2 || diskModels[1] != "my-custom-model" { - t.Errorf("disk Models = %v, want [qwen3.7-max my-custom-model]", diskModels) - } + return cfg } -func TestProviderTUI_OfficialTab_CustomModelInput_RejectsDuplicate(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, "config.json") - cfg := &Config{ - Provider: "dashscope", - Model: "qwen3.7-max", - Providers: map[string]ProviderEntry{ - "dashscope": { - Model: "qwen3.7-max", - Models: []string{"qwen3.7-max"}, - }, - }, - } - m := newProviderTUI(cfg, configPath) - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "dashscope" { - m.officialIdx = i - break - } +func assertModels(t *testing.T, what string, got []string, want ...string) { + t.Helper() + if !slices.Equal(got, want) { + t.Errorf("%s = %v, want %v", what, got, want) } - m.step = stepModel - m.modelIdx = len(m.models()) - m.customModel = true - m.modelInput.SetValue("qwen3.7-max") - m.modelInput.Focus() +} - result, _ := m.Update(enterKey()) - m2 := result.(providerTUIModel) +func sendKey(m providerTUIModel, key tea.KeyPressMsg) providerTUIModel { + result, _ := m.Update(key) + return result.(providerTUIModel) +} - if !m2.customModel { - t.Error("customModel should stay true after duplicate reject") - } - if m2.formError != "Already in list: qwen3.7-max" { - t.Errorf("formError = %q, want %q", m2.formError, "Already in list: qwen3.7-max") - } - if _, err := os.Stat(configPath); err == nil { - t.Errorf("disk file should not exist; duplicate did not persist") +func openCustomProviderForm(t *testing.T, cfg *Config, configPath string) providerTUIModel { + t.Helper() + m := sendKey(sendKey(newProviderTUI(cfg, configPath), rightKey()), enterKey()) + if !m.creatingCustom { + t.Fatal("should be creating custom") } + return m } -func officialDashscopeModelTUI(t *testing.T, configPath string, extraModels []string) providerTUIModel { +// The provider is always named "my-new". +func fillCustomFormToAuthHeader(t *testing.T, m providerTUIModel, apiKey string) providerTUIModel { t.Helper() - models := []string{"qwen3.7-max"} - models = append(models, extraModels...) - cfg := &Config{ - Provider: "dashscope", - Model: "qwen3.7-max", - Providers: map[string]ProviderEntry{ - "dashscope": { - Model: "qwen3.7-max", - Models: models, - }, - }, - } - m := newProviderTUI(cfg, configPath) - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "dashscope" { - m.officialIdx = i - break - } + m.cpNameInput.SetValue("my-new") + m = sendKey(m, enterKey()) // name -> protocol + m = sendKey(m, enterKey()) // protocol -> URL + m.cpURLInput.SetValue("https://api.example.com") + m = sendKey(m, enterKey()) // URL -> API key + m.apiKeyInput.SetValue(apiKey) + m = sendKey(m, enterKey()) // API key -> auth header + if m.cpStep != cpStepAuthHeader { + t.Fatalf("cpStep = %d, want %d", m.cpStep, cpStepAuthHeader) } - m.step = stepModel return m } @@ -1553,14 +1403,8 @@ func customStepfunModelTUI(t *testing.T, configPath string, models []string) pro }, }, } - m := newProviderTUI(cfg, configPath) - m.activeTab = tabCustom - for i, cp := range m.customProviders { - if cp.name == "stepfun" { - m.customIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, configPath, tabCustom) + selectCustomProvider(t, &m, "stepfun") m.step = stepModel return m } @@ -1596,58 +1440,11 @@ func TestProviderTUI_OfficialTab_DeleteUserAddedModel(t *testing.T) { if m3.confirmingDeleteModel { t.Error("confirmingDeleteModel should be false after y") } - got := m3.existingCfg.Providers["dashscope"].Models - if len(got) != 1 || got[0] != "qwen3.7-max" { - t.Errorf("Models = %v, want [qwen3.7-max]", got) - } + assertModels(t, "Models", m3.existingCfg.Providers["dashscope"].Models, "qwen3.7-max") if !m3.savedInSession { t.Error("savedInSession should be true after delete") } - - diskCfg, err := loadOrCreateConfig(configPath) - if err != nil { - t.Fatalf("load disk config: %v", err) - } - if len(diskCfg.Providers["dashscope"].Models) != 1 { - t.Errorf("disk Models = %v, want [qwen3.7-max]", diskCfg.Providers["dashscope"].Models) - } -} - -func TestProviderTUI_OfficialTab_DeleteBuiltInModelIgnored(t *testing.T) { - m := officialDashscopeModelTUI(t, "", []string{"my-custom-model"}) - m.modelIdx = modelIdxForName(t, m, "qwen3.7-max") - - result, _ := m.Update(dKey()) - m2 := result.(providerTUIModel) - if m2.confirmingDeleteModel { - t.Error("pressing d on built-in model should not trigger delete confirmation") - } -} - -func TestProviderTUI_OfficialTab_RegistryModelNotDeletable(t *testing.T) { - m := officialDashscopeModelTUI(t, "", []string{"my-custom-model"}) - m.modelIdx = modelIdxForName(t, m, "qwen3.7-max") - - if m.isUserAddedOfficialModel("qwen3.7-max") { - t.Error("qwen3.7-max should not be user-added when it is in the registry") - } - - result, _ := m.Update(dKey()) - m2 := result.(providerTUIModel) - if m2.confirmingDeleteModel { - t.Error("pressing d on registry model should not trigger delete confirmation") - } -} - -func TestProviderTUI_OfficialTab_DeleteOnCustomModelInputIgnored(t *testing.T) { - m := officialDashscopeModelTUI(t, "", []string{"my-custom-model"}) - m.modelIdx = len(m.models()) - - result, _ := m.Update(dKey()) - m2 := result.(providerTUIModel) - if m2.confirmingDeleteModel { - t.Error("pressing d on Enter custom model name... should not trigger delete confirmation") - } + assertModels(t, "disk Models", loadDiskConfig(t, configPath).Providers["dashscope"].Models, "qwen3.7-max") } func TestProviderTUI_CustomTab_DeleteOnCustomModelInputIgnored(t *testing.T) { @@ -1661,38 +1458,6 @@ func TestProviderTUI_CustomTab_DeleteOnCustomModelInputIgnored(t *testing.T) { } } -func TestProviderTUI_CustomTab_ModelShowsDeleteHint(t *testing.T) { - m := customStepfunModelTUI(t, "", []string{"step-3.5-flash", "aaa"}) - - m.modelIdx = len(m.models()) - got := stripANSI(m.View().Content) - if strings.Contains(got, "d Delete") { - t.Errorf("custom input row should not show d Delete hint; got:\n%s", got) - } - - m.modelIdx = modelIdxForName(t, m, "aaa") - got = stripANSI(m.View().Content) - if !strings.Contains(got, "d Delete") { - t.Errorf("custom model row should show d Delete hint; got:\n%s", got) - } -} - -func TestProviderTUI_OfficialTab_UserAddedModelShowsDeleteHint(t *testing.T) { - m := officialDashscopeModelTUI(t, "", []string{"my-custom-model"}) - - m.modelIdx = modelIdxForName(t, m, "qwen3.7-max") - got := stripANSI(m.View().Content) - if strings.Contains(got, "d Delete") { - t.Errorf("built-in model should not show d Delete hint; got:\n%s", got) - } - - m.modelIdx = modelIdxForName(t, m, "my-custom-model") - got = stripANSI(m.View().Content) - if !strings.Contains(got, "d Delete") { - t.Errorf("user-added model should show d Delete hint; got:\n%s", got) - } -} - func TestProviderTUI_OfficialTab_DeleteModelPreservesActiveModel(t *testing.T) { dir := t.TempDir() configPath := filepath.Join(dir, "config.json") @@ -1717,62 +1482,6 @@ func TestProviderTUI_OfficialTab_DeleteModelPreservesActiveModel(t *testing.T) { } } -func TestProviderTUI_OfficialTab_DeleteUserAddedModelCancel(t *testing.T) { - cancelKeys := []struct { - name string - key tea.KeyPressMsg - }{ - {"n", nKey()}, - {"esc", escKey()}, - } - for _, tc := range cancelKeys { - t.Run(tc.name, func(t *testing.T) { - m := officialDashscopeModelTUI(t, "", []string{"my-custom-model"}) - m.modelIdx = modelIdxForName(t, m, "my-custom-model") - - result, _ := m.Update(dKey()) - m2 := result.(providerTUIModel) - if !m2.confirmingDeleteModel { - t.Fatal("expected confirmingDeleteModel after d") - } - - result, _ = m2.Update(tc.key) - m3 := result.(providerTUIModel) - if m3.confirmingDeleteModel { - t.Error("confirmingDeleteModel should be false after cancel") - } - got := m3.existingCfg.Providers["dashscope"].Models - if len(got) != 2 || got[1] != "my-custom-model" { - t.Errorf("Models = %v, want model unchanged", got) - } - }) - } -} - -func TestProviderTUI_OfficialTab_DeleteActiveUserModelClearsCfg(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, "config.json") - m := officialDashscopeModelTUI(t, configPath, []string{"my-custom-model"}) - m.existingCfg.Model = "my-custom-model" - m.existingCfg.Providers["dashscope"] = ProviderEntry{ - Model: "my-custom-model", - Models: []string{"qwen3.7-max", "my-custom-model"}, - } - m.modelIdx = modelIdxForName(t, m, "my-custom-model") - - result, _ := m.Update(dKey()) - m2 := result.(providerTUIModel) - result, _ = m2.Update(yKey()) - m3 := result.(providerTUIModel) - - if m3.existingCfg.Providers["dashscope"].Model != "" { - t.Errorf("entry.Model = %q, want empty", m3.existingCfg.Providers["dashscope"].Model) - } - if m3.existingCfg.Model != "" { - t.Errorf("cfg.Model = %q, want empty", m3.existingCfg.Model) - } -} - func TestOfficialSelectedModelUsesRegistryNotStaleMerge(t *testing.T) { registry := []string{"built-in"} deletedCustom := "my-custom" @@ -1822,14 +1531,8 @@ func TestProviderTUI_CustomTab_DeleteModelViaDKey(t *testing.T) { }, }, } - m := newProviderTUI(cfg, configPath) - m.activeTab = tabCustom - for i, cp := range m.customProviders { - if cp.name == "stepfun" { - m.customIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, configPath, tabCustom) + selectCustomProvider(t, &m, "stepfun") m.step = stepModel m.modelIdx = modelIdxForName(t, m, "aaa") @@ -1841,18 +1544,8 @@ func TestProviderTUI_CustomTab_DeleteModelViaDKey(t *testing.T) { result, _ = m2.Update(yKey()) m3 := result.(providerTUIModel) - got := m3.existingCfg.CustomProviders["stepfun"].Models - if len(got) != 1 || got[0] != "step-3.5-flash" { - t.Errorf("Models = %v, want [step-3.5-flash]", got) - } - - diskCfg, err := loadOrCreateConfig(configPath) - if err != nil { - t.Fatalf("load disk config: %v", err) - } - if len(diskCfg.CustomProviders["stepfun"].Models) != 1 { - t.Errorf("disk Models = %v, want [step-3.5-flash]", diskCfg.CustomProviders["stepfun"].Models) - } + assertModels(t, "Models", m3.existingCfg.CustomProviders["stepfun"].Models, "step-3.5-flash") + assertModels(t, "disk Models", loadDiskConfig(t, configPath).CustomProviders["stepfun"].Models, "step-3.5-flash") } func TestProviderTUI_PersistCustomModelName_SaveFailureRollsBack(t *testing.T) { @@ -1888,8 +1581,7 @@ 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 := newProviderTUIOnTab(cfg, configPath, tabManual) m.inManualForm = true m.manualStep = manualStepAuthHeader m.manualAuthHeaderInput.Focus() @@ -1927,14 +1619,11 @@ func TestProviderTUI_CustomFormPassesKToAuthHeaderInput(t *testing.T) { 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) + for _, c := range "key" { + m = sendKey(m, charKey(c)) + } - if got := m4.cpAuthInput.Value(); got != "key" { + if got := m.cpAuthInput.Value(); got != "key" { t.Errorf("cpAuthInput.Value() = %q, want %q", got, "key") } } @@ -1947,14 +1636,8 @@ func TestProviderTUI_ViewAPIKey_MaskedShowsReplaceHintAndLastFour(t *testing.T) "dashscope": {APIKey: "sk-secret-1234567890abcd"}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "dashscope" { - m.officialIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, "", tabOfficial) + selectOfficialProvider(t, &m, "dashscope") m.step = stepAPIKey m.loadExistingAPIKey() m.apiKeyInput.Focus() @@ -1980,14 +1663,8 @@ func TestProviderTUI_ViewAPIKey_ShortKeyOmitsFingerprint(t *testing.T) { "dashscope": {APIKey: "12345678901234"}, // 14 runes — below min length 15 }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "dashscope" { - m.officialIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, "", tabOfficial) + selectOfficialProvider(t, &m, "dashscope") m.step = stepAPIKey m.loadExistingAPIKey() m.apiKeyInput.Focus() @@ -2009,14 +1686,8 @@ func TestProviderTUI_ViewAPIKey_MinLenKeyShowsFingerprint(t *testing.T) { "dashscope": {APIKey: "123456789012345"}, // 15 runes — at min length }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "dashscope" { - m.officialIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, "", tabOfficial) + selectOfficialProvider(t, &m, "dashscope") m.step = stepAPIKey m.loadExistingAPIKey() m.apiKeyInput.Focus() @@ -2044,14 +1715,8 @@ func TestProviderTUI_ViewAPIKey_FreshHidesReplaceHint(t *testing.T) { "dashscope": {}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "dashscope" { - m.officialIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, "", tabOfficial) + selectOfficialProvider(t, &m, "dashscope") m.step = stepAPIKey m.loadExistingAPIKey() @@ -2084,14 +1749,8 @@ func TestProviderTUI_ViewAPIKey_EnvSetNoSavedKey(t *testing.T) { "deepseek": {Model: "deepseek-v4-flash"}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "deepseek" { - m.officialIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, "", tabOfficial) + selectOfficialProvider(t, &m, "deepseek") m.step = stepAPIKey m.loadExistingAPIKey() m.apiKeyInput.Focus() @@ -2112,14 +1771,8 @@ func TestProviderTUI_ViewAPIKey_EnvSetWithSavedKey(t *testing.T) { "deepseek": {APIKey: "sk-secret-1234567890abcd", Model: "deepseek-v4-flash"}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "deepseek" { - m.officialIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, "", tabOfficial) + selectOfficialProvider(t, &m, "deepseek") m.step = stepAPIKey m.loadExistingAPIKey() m.apiKeyInput.Focus() @@ -2150,8 +1803,7 @@ func TestProviderTUI_ApiKeyPasteReplacesMaskedKey(t *testing.T) { }, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, "", tabCustom) m.customIdx = 0 m.step = stepAPIKey m.loadExistingAPIKey() @@ -2186,8 +1838,7 @@ func TestProviderTUI_ManualTokenPasteReplacesMaskedToken(t *testing.T) { AuthToken: "old-token-secret", }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabManual + m := newProviderTUIOnTab(cfg, "", tabManual) m.inManualForm = true m.manualStep = manualStepAuthToken m.manualTokenMasked = true @@ -2221,8 +1872,7 @@ func TestProviderTUI_ApiKeyTypingShowsOneStarPerChar(t *testing.T) { "stepfun": {APIKey: "old-key-ssss", Model: "step-3.5-flash"}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, "", tabCustom) m.customIdx = 0 m.step = stepAPIKey m.loadExistingAPIKey() @@ -2250,8 +1900,7 @@ func TestProviderTUI_ApiKeyEnterWithoutEditKeepsOriginal(t *testing.T) { "stepfun": {APIKey: "keep-me"}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, "", tabCustom) m.customIdx = 0 m.step = stepAPIKey m.loadExistingAPIKey() @@ -2282,14 +1931,8 @@ func TestProviderTUI_ApiKeyClearSavedKeyReturnsEmpty(t *testing.T) { "deepseek": {APIKey: "old-saved-key", Model: "deepseek-v4-flash"}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "deepseek" { - m.officialIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, "", tabOfficial) + selectOfficialProvider(t, &m, "deepseek") m.step = stepAPIKey m.loadExistingAPIKey() m.beginAPIKeyReplace() @@ -2307,14 +1950,8 @@ func TestProviderTUI_ApiKeyResultTrimSpace(t *testing.T) { "deepseek": {Model: "deepseek-v4-flash"}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "deepseek" { - m.officialIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, "", tabOfficial) + selectOfficialProvider(t, &m, "deepseek") m.step = stepAPIKey m.apiKeyInput.SetValue(" ") @@ -2331,14 +1968,8 @@ func TestProviderTUI_OfficialApiKeyEmptyWithoutEnvBlocksEnter(t *testing.T) { "dashscope": {}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "dashscope" { - m.officialIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, "", tabOfficial) + selectOfficialProvider(t, &m, "dashscope") m.step = stepAPIKey m.loadExistingAPIKey() m.apiKeyInput.Focus() @@ -2368,14 +1999,8 @@ func TestProviderTUI_OfficialApiKeyEmptyWithEnvAllowsEnter(t *testing.T) { "dashscope": {}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "dashscope" { - m.officialIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, "", tabOfficial) + selectOfficialProvider(t, &m, "dashscope") m.step = stepAPIKey m.loadExistingAPIKey() m.apiKeyInput.Focus() @@ -2402,8 +2027,7 @@ func TestProviderTUI_CustomExistingApiKeyEmptyBlocksEnter(t *testing.T) { "stepfun": {APIKey: "old-key"}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, "", tabCustom) m.customIdx = 0 m.step = stepAPIKey m.loadExistingAPIKey() @@ -2426,8 +2050,7 @@ func TestProviderTUI_CustomExistingApiKeyEmptyBlocksEnter(t *testing.T) { } func TestProviderTUI_CustomCreateApiKeyOptional(t *testing.T) { - m := newProviderTUI(&Config{}, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(&Config{}, "", tabCustom) m.creatingCustom = true m.cpStep = cpStepAPIKey m.apiKeyInput.SetValue("") @@ -2451,14 +2074,8 @@ func TestProviderTUI_ViewAPIKey_ShowsFormError(t *testing.T) { "dashscope": {}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "dashscope" { - m.officialIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, "", tabOfficial) + selectOfficialProvider(t, &m, "dashscope") m.step = stepAPIKey m.loadExistingAPIKey() m.formError = "API key is required (or set $DASHSCOPE_API_KEY)" @@ -2480,14 +2097,8 @@ func TestProviderTUI_CancelIncompleteOfficialProviderSwitch_NoPersistedChanges(t "deepseek": {Model: "deepseek-v4-flash"}, }, } - m := newProviderTUI(cfg, configPath) - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "baidu-qianfan" { - m.officialIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, configPath, tabOfficial) + selectOfficialProvider(t, &m, "baidu-qianfan") m.step = stepModel m.modelIdx = modelIdxForName(t, m, "glm-5") @@ -2516,10 +2127,7 @@ func TestProviderTUI_CancelIncompleteOfficialProviderSwitch_NoPersistedChanges(t t.Error("expected tea.Quit on final Esc") } if _, err := os.Stat(configPath); err == nil { - diskCfg, err := loadOrCreateConfig(configPath) - if err != nil { - t.Fatalf("load config: %v", err) - } + diskCfg := loadDiskConfig(t, configPath) if diskCfg.Provider != "deepseek" { t.Errorf("Provider = %q, want deepseek", diskCfg.Provider) } @@ -2539,8 +2147,7 @@ func TestProviderTUI_SameOfficialProviderModelChange_DefersPersistUntilConfirm(t "deepseek": {Model: "deepseek-v4-flash"}, }, } - m := newProviderTUI(cfg, configPath) - m.activeTab = tabOfficial + m := newProviderTUIOnTab(cfg, configPath, tabOfficial) m.step = stepModel m.modelIdx = modelIdxForName(t, m, "deepseek-v4-pro") @@ -2574,14 +2181,8 @@ func TestProviderTUI_OfficialModelChangeBlockedAtAPIKey_KeepsGlobalModel(t *test }, }, } - m := newProviderTUI(cfg, configPath) - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "anthropic" { - m.officialIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, configPath, tabOfficial) + selectOfficialProvider(t, &m, "anthropic") m.step = stepModel m.modelIdx = modelIdxForName(t, m, "claude-opus-4-7") @@ -2640,8 +2241,7 @@ func TestProviderTUI_DeleteModelPreservesActiveModel(t *testing.T) { }, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, "", tabCustom) m.customIdx = 0 m.step = stepModel m.modelIdx = 1 // aaa @@ -2884,8 +2484,7 @@ func TestEnterEditCustomProvider_ProtocolIndex(t *testing.T) { "cp": {URL: "https://x.example.com", Protocol: tt.protocol}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabCustom + m := newProviderTUIOnTab(cfg, "", tabCustom) m.customIdx = 0 m.enterEditCustomProvider() if m.cpProtocolIdx != tt.wantIdx { @@ -2990,8 +2589,7 @@ func TestApplyManualConfig_DoubleWritesProtocolAndUseAnthropic(t *testing.T) { // Manual tab returns the canonical protocol name picked from manualProtocols. func TestProviderTUIResult_ManualProtocolIsCanonical(t *testing.T) { cfg := &Config{} - m := newProviderTUI(cfg, "") - m.activeTab = tabManual + m := newProviderTUIOnTab(cfg, "", tabManual) m.inManualForm = true m.manualURLInput.SetValue("https://example.com/v1") m.manualModelInput.SetValue("m") @@ -3025,14 +2623,8 @@ func TestProviderTUI_ViewAPIKey_ShowsAPIKeyCmdHint(t *testing.T) { "deepseek": {APIKeyCmd: "op read op://dev/deepseek/key", Model: "deepseek-v4-flash"}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "deepseek" { - m.officialIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, "", tabOfficial) + selectOfficialProvider(t, &m, "deepseek") m.step = stepAPIKey m.loadExistingAPIKey() m.apiKeyInput.Focus() @@ -3056,14 +2648,8 @@ func TestProviderTUI_ViewAPIKey_NoCmdHintWhenUnset(t *testing.T) { "deepseek": {Model: "deepseek-v4-flash"}, }, } - m := newProviderTUI(cfg, "") - m.activeTab = tabOfficial - for i, p := range m.providers { - if p.Name == "deepseek" { - m.officialIdx = i - break - } - } + m := newProviderTUIOnTab(cfg, "", tabOfficial) + selectOfficialProvider(t, &m, "deepseek") m.step = stepAPIKey m.loadExistingAPIKey()