diff --git a/.gitignore b/.gitignore index c8421fc..f392948 100644 --- a/.gitignore +++ b/.gitignore @@ -62,3 +62,4 @@ completions/phosphor.*sh # Local planning/scratch artifacts (temporary working docs, never committed) /_scratch/ .gocache/ +/.tmp/ diff --git a/docs/INDEX.md b/docs/INDEX.md index 04fbf4d..2582c8d 100644 --- a/docs/INDEX.md +++ b/docs/INDEX.md @@ -102,6 +102,7 @@ searchable through the `phosphor_docs` tool or by viewing any | --- | --- | | [architecture/OVERVIEW.md](architecture/OVERVIEW.md) | Module layout and the transport/backend split. | | [architecture/SESSIONS.md](architecture/SESSIONS.md) | The two-tier session model backed by SQLite. | +| [architecture/PROVIDER_ONBOARDING.md](architecture/PROVIDER_ONBOARDING.md) | The first-run provider onboarding flow and the custom (local) provider path. | ### Architecture Decision Records diff --git a/docs/architecture/PROVIDER_ONBOARDING.md b/docs/architecture/PROVIDER_ONBOARDING.md new file mode 100644 index 0000000..58d70cf --- /dev/null +++ b/docs/architecture/PROVIDER_ONBOARDING.md @@ -0,0 +1,154 @@ +# Provider Onboarding Flow + +This document describes what happens the first time Phosphor is started +in a workspace on a machine with no usable provider configuration, and +how the custom (local/self-hosted) provider path fits into it. + +## The gate: `Config.IsConfigured()` + +Everything hinges on one check: + +```go +// pkg/config/config.go +func (c *Config) IsConfigured() bool { + return len(c.EnabledProviders()) > 0 +} +``` + +If no provider is enabled (no global config, no data config, no project +`phosphor.json` with providers), the app refuses to initialize the agent +(`internal/app/app.go`) and the TUI routes straight into onboarding +(`internal/ui/model/ui.go`, `desiredState = uiOnboarding`). + +## The onboarding screen + +Onboarding is **the `Models` dialog rendered in onboarding mode** +(`internal/ui/dialog/models.go`, `NewModels(com, isOnboarding)`). +`UI.Init()` force-opens it whenever the state is `uiOnboarding`. + +Input is captured at two layers: + +1. `handleKeyPressMsg` routes *every* message to the dialog overlay + while `m.dialog.HasDialogs()` is true, so global keys (including + `ctrl+p`, the main command menu) never fire. +2. The dialog itself is inescapable: `esc` returns `ActionClose`, but + `handleDialogMsg` swallows it while the `Models` dialog is the front + dialog in onboarding and re-opens the dialog otherwise. + +Selecting a provider/model chains into the credential dialogs +(`openAuthenticationDialog` → `NewOAuthHyper` / `NewOAuthCopilot` / +`NewAPIKeyInput`). Completing the flow calls `handleSelectModel`, which +flips the state to `uiLanding`, and initializes the system agent (the +config store's `autoReload` runs `SetupAgents()` as soon as the first +provider makes the config usable). + +## The chicken-and-egg problem + +The custom provider wizard (`internal/ui/dialog/provider_wizard.go`, +opened via `Ctrl+P` → *Add Custom Provider*) is only reachable from the +main command menu. But the command menu cannot be opened during +onboarding because the onboarding dialog intercepts all input. So a +user running local models (Ollama, vLLM, llama.cpp, LM Studio, …) had +no way to configure their endpoint inside the app — they had to +first authenticate a cloud provider, or hand-edit +`$XDG_DATA_HOME/phosphor/phosphor.json` outside the TUI. + +## The fix: custom provider entry in the onboarding list + +The onboarding provider list now starts with a synthetic +*Set up a custom provider* entry. Selecting it emits +`ActionOpenDialog{DialogID: ProviderWizardID}`, which opens the +existing 6-step wizard (name → base URL → API key → model IDs → +context window → attachments) directly on top of the onboarding screen: + +- The dialog is a framed, centered box with fixed dimensions: the + summary panel lists every field (provider, url, api key, models, + context, attachments) on every step, so collected values appear as + the user progresses and the box never resizes. +- The context-window step takes a positive token count and is skippable + (empty input). +- The attachments step is a yes/no radio selector (arrows move, enter + confirms), so the stored value is always a valid boolean. +- The same fields are editable later via *Manage Custom Providers* + (`context_window` and `supports_attachments` rows on each model). + `pkg/config/custom_provider_capabilities_test.go` pins that both + fields survive the config load pipeline. +- An unset `context_window` (0) silently disables context compaction — + `shouldSummarize` in `pkg/agent/agent.go` skips summarization when + the window is zero — so the value is worth filling in. + +- The wizard always renders as a framed, centered dialog — even during + onboarding — so it stays visually distinct from the provider picker + behind it. +- While the wizard is the front dialog, `esc` closes *it* and returns + to the provider list (it remains swallowed only for the provider + picker itself, so onboarding stays inescapable). This also applies to + the API-key/OAuth dialogs opened on top of the picker — `esc` there + steps back to the provider list instead of doing nothing. +- Finishing the wizard persists the provider to the global data config + (`setupProvider` → `SetConfigField(ScopeGlobal, …)`), then closes the + wizard and refreshes the provider list so the new provider's models + appear immediately. +- Finishing the wizard auto-selects the first configured model as the + large model (mirroring the API-key dialog, which selects the model + the user was on). `handleSelectModel` then records `models.large` in + the workspace config, defaults the small model to the same one, + initializes the system agent, and leaves onboarding — so image paste + (`currentModelSupportsImages`, which resolves through the agent's + large model) works immediately after setup. +- The user then picks one of those models; `handleSelectModel` sees the + provider is configured, skips authentication, sets large/small model + preferences (custom providers fall back to the large model as the + small model, `internal/app/app.go`, `GetDefaultSmallModel`), and + leaves onboarding normally. + +## Flow diagram + +```mermaid +flowchart TD + A[phosphor TUI starts] --> B{Config.IsConfigured?} + B -- no --> C[uiOnboarding:\nModels dialog force-opened] + B -- yes --> Z[uiLanding / uiInitialize] + C --> D{User choice} + D -- built-in provider --> E[Credential dialog\nAPI key / OAuth] + D -- custom provider entry --> F[Provider wizard\nname, URL, key, models] + F -- esc --> C + F -- finish --> G[Persist to global data config\nproviders.id.* ] + G --> C + E --> H[handleSelectModel] + C -- select configured model --> H + H --> I[SetupAgents + InitSystemAgent\nstate = uiLanding] +``` + +## Why not open the whole command menu during onboarding? + +Items like Sessions, Skills, Prune, or Codebase Index require the +workspace system agent, which is only initialized after a provider is +selected (`InitSystemAgent` runs at the end of `handleSelectModel`). +Exposing the full menu there would offer actions that cannot run yet. +The single, targeted *Set up a custom provider* entry gives local-model +users the escape hatch without pretending the rest of the app works +before a provider exists. + +## Files involved + +| File | Role | +| --- | --- | +| `internal/ui/model/ui.go` | state machine, dialog routing, `setupProvider`, `openProviderWizardDialog` | +| `internal/ui/dialog/models.go` | provider/model picker (the onboarding screen) | +| `internal/ui/dialog/provider_wizard.go` | custom provider setup wizard | +| `internal/ui/dialog/provider_manager.go` | custom provider management (post-onboarding) | +| `internal/ui/dialog/api_key_input.go`, `oauth.go` | credential capture | +| `pkg/config/config.go`, `pkg/config/load.go`, `pkg/config/store.go` | provider schema, validation, persistence | +| `internal/app/app.go` | `IsConfigured` gate, `GetDefaultSmallModel` fallback | + +## Testing + +`internal/ui/dialog/models_onboarding_test.go` covers the synthetic +entry: presence only during onboarding, that selecting it returns +`ActionOpenDialog{ProviderWizardID}`, and that built-in providers remain +listed alongside it. + +Note: in sandboxes where `~/.local/share/phosphor` is unwritable, set +`PHOSPHOR_DISABLE_PROVIDER_AUTO_UPDATE=1` when running the suite — +otherwise `config.Providers` fails trying to refresh the provider cache. \ No newline at end of file diff --git a/internal/ui/dialog/models.go b/internal/ui/dialog/models.go index b552973..0099fca 100644 --- a/internal/ui/dialog/models.go +++ b/internal/ui/dialog/models.go @@ -67,6 +67,15 @@ const ( smallModelInputPlaceholder = "Choose a model for small, simple tasks" ) +// customProviderItemID is the sentinel provider id used by the synthetic +// "set up a custom provider" entry shown during onboarding. Colons are +// excluded from user-authored provider ids (see providerIDPattern in +// provider_wizard.go), so this id can never collide with a real provider. +const customProviderItemID = catwalk.InferenceProvider("phosphor:custom-provider") + +// customProviderItemName is the label of the synthetic onboarding entry. +const customProviderItemName = "Set up a custom provider (local or self-hosted endpoint)" + // ModelsID is the identifier for the model selection dialog. const ModelsID = "models" @@ -195,6 +204,10 @@ func (m *Models) HandleMsg(msg tea.Msg) Action { break } + if modelItem.prov.ID == customProviderItemID { + return ActionOpenDialog{DialogID: ProviderWizardID} + } + isEdit := key.Matches(msg, m.keyMap.Edit) return ActionSelectModel{ @@ -378,6 +391,24 @@ func (m *Models) setProviderItems() error { // itemsMap contains the keys of added model items. itemsMap := make(map[string]*ModelItem) groups := []ModelGroup{} + + // During onboarding, offer the custom provider wizard at the top of + // the list so local-model users can point the app at their own + // endpoint without first authenticating a cloud provider. + if m.isOnboarding { + customProvider := catwalk.Provider{ + ID: customProviderItemID, + Name: "Custom Provider", + } + customItem := NewModelItem( + t, + customProvider, + catwalk.Model{ID: customProviderItemName, Name: customProviderItemName}, + m.modelType, + false, + ) + groups = append(groups, NewModelGroup(t, "Custom Provider", false, customItem)) + } for id, p := range cfg.Providers.Seq2() { if p.Disable { continue @@ -489,7 +520,13 @@ func (m *Models) setProviderItems() error { } if len(recentGroup.Items) > 0 { - groups = append([]ModelGroup{recentGroup}, groups...) + customIdx := 0 + if m.isOnboarding { + // Keep the custom provider entry above recently used + // models, which belong to previously configured providers. + customIdx = 1 + } + groups = append([]ModelGroup{recentGroup}, groups[customIdx:]...) } } diff --git a/internal/ui/dialog/models_onboarding_test.go b/internal/ui/dialog/models_onboarding_test.go new file mode 100644 index 0000000..5dfa84a --- /dev/null +++ b/internal/ui/dialog/models_onboarding_test.go @@ -0,0 +1,107 @@ +package dialog + +import ( + "testing" + + tea "charm.land/bubbletea/v2" + "github.com/hackafterdark/phosphor/internal/ui/common" + "github.com/hackafterdark/phosphor/internal/ui/list" + "github.com/hackafterdark/phosphor/internal/ui/styles" + "github.com/hackafterdark/phosphor/pkg/config" + "github.com/hackafterdark/phosphor/pkg/csync" + "github.com/stretchr/testify/require" +) + +func TestOnboardingCustomProviderEntry(t *testing.T) { + t.Parallel() + + t.Run("shown at top of list during onboarding", func(t *testing.T) { + t.Parallel() + + m := newTestModels(t, true) + items := visibleSelectable(m) + require.NotEmpty(t, items) + first, ok := items[0].(*ModelItem) + require.True(t, ok) + require.Equal(t, customProviderItemID, first.prov.ID) + }) + + t.Run("selecting it opens the provider wizard", func(t *testing.T) { + t.Parallel() + + m := newTestModels(t, true) + m.list.SelectFirst() + action := m.HandleMsg(selectKeyMsg()) + open, ok := action.(ActionOpenDialog) + require.True(t, ok, "expected ActionOpenDialog, got %T", action) + require.Equal(t, ProviderWizardID, open.DialogID) + }) + + t.Run("not shown outside onboarding", func(t *testing.T) { + t.Parallel() + + m := newTestModels(t, false) + for _, item := range visibleSelectable(m) { + mi, ok := item.(*ModelItem) + if !ok { + continue + } + require.NotEqual(t, customProviderItemID, mi.prov.ID) + } + }) + + t.Run("real models are unaffected", func(t *testing.T) { + t.Parallel() + + m := newTestModels(t, true) + var found bool + for _, item := range visibleSelectable(m) { + mi, ok := item.(*ModelItem) + if ok && mi.prov.ID != customProviderItemID { + found = true + break + } + } + require.True(t, found, "built-in providers must still be listed") + }) +} + +func newTestModels(t *testing.T, isOnboarding bool) *Models { + t.Helper() + + originalUseMock := config.UseMockProviders + config.UseMockProviders = true + t.Cleanup(func() { + config.UseMockProviders = originalUseMock + config.ResetProviders() + }) + config.ResetProviders() + + s := styles.CharmtonePantera() + cfg := &config.Config{ + Options: &config.Options{DisableProviderAutoUpdate: true}, + Providers: csync.NewMap[string, config.ProviderConfig](), + } + com := &common.Common{ + Styles: &s, + Workspace: &mockWorkspace{cfg: cfg}, + } + m, err := NewModels(com, isOnboarding) + require.NoError(t, err) + return m +} + +func visibleSelectable(m *Models) []list.Item { + items := m.list.VisibleItems() + var out []list.Item + for _, item := range items { + if _, ok := item.(*ModelItem); ok { + out = append(out, item) + } + } + return out +} + +func selectKeyMsg() tea.KeyPressMsg { + return tea.KeyPressMsg{Code: tea.KeyEnter} +} diff --git a/internal/ui/dialog/provider_wizard.go b/internal/ui/dialog/provider_wizard.go index 8492ccf..6537c13 100644 --- a/internal/ui/dialog/provider_wizard.go +++ b/internal/ui/dialog/provider_wizard.go @@ -1,10 +1,12 @@ package dialog import ( + "cmp" "fmt" "net/url" "regexp" "slices" + "strconv" "strings" "charm.land/bubbles/v2/help" @@ -27,8 +29,19 @@ const ( providerWizardStepURL providerWizardStepKey providerWizardStepModels + providerWizardStepContext + providerWizardStepAttach ) +// providerWizardStepCount is the total number of wizard screens, used in +// the "Step X of N" footer. +const providerWizardStepCount = int(providerWizardStepAttach) + 1 + +// providerWizardHeight is the fixed content height of the dialog. +// Padding every step to this height keeps the dialog from resizing as +// the collected values appear. +const providerWizardHeight = 10 + // providerIDPattern is the accepted character set for a provider id. Dots // are deliberately excluded because config writes use dotted key paths // (providers..base_url). @@ -38,12 +51,22 @@ var providerIDPattern = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`) // the flow. The caller is responsible for persisting the provider to the // global config and closing the dialog. type ActionSetupProvider struct { - ProviderID string - BaseURL string - APIKey string - Models []string + ProviderID string + BaseURL string + APIKey string + Models []string + ContextW int64 + Attachments *bool } +// boolOption identifies one choice on the boolean capabilities screen. +type boolOption int + +const ( + boolOptionYes boolOption = iota + boolOptionNo +) + // ProviderWizard is a small linear wizard that collects an // OpenAI-compatible endpoint (e.g. llama.cpp, vLLM, Ollama, LM Studio), // an optional API key, and a list of model ids, then hands everything to @@ -57,29 +80,37 @@ type ProviderWizard struct { baseURL string apiKey string models []string + contextW int64 + attach *bool + attachSel boolOption errText string - nameInput textinput.Model - urlInput textinput.Model - keyInput textinput.Model - modelInput textinput.Model + nameInput textinput.Model + urlInput textinput.Model + keyInput textinput.Model + modelInput textinput.Model + contextWInput textinput.Model help help.Model keyMap struct { Submit key.Binding Remove key.Binding + Prev key.Binding + Next key.Binding Close key.Binding } } var _ Dialog = (*ProviderWizard)(nil) -// NewProviderWizard creates a new provider setup wizard dialog. +// NewProviderWizard creates a new provider setup wizard dialog. It always +// renders as a framed, centered dialog so it stands apart from whatever +// dialog (e.g. the onboarding provider picker) sits behind it. func NewProviderWizard(com *common.Common) *ProviderWizard { m := &ProviderWizard{} m.com = com - m.width = 60 + m.width = 70 innerWidth := m.width - com.Styles.Dialog.View.GetHorizontalFrameSize() - 2 // (1) cursor padding @@ -87,6 +118,7 @@ func NewProviderWizard(com *common.Common) *ProviderWizard { m.urlInput = newWizardInput(com, innerWidth, "http://localhost:11434/v1") m.keyInput = newWizardInput(com, innerWidth, "leave empty if the server doesn't need one") m.modelInput = newWizardInput(com, innerWidth, "e.g. qwen3-32b") + m.contextWInput = newWizardInput(com, innerWidth, "e.g. 32768 (leave empty to skip)") m.nameInput.Focus() m.help = help.New() @@ -100,6 +132,14 @@ func NewProviderWizard(com *common.Common) *ProviderWizard { key.WithKeys("ctrl+d"), key.WithHelp("ctrl+d", "remove last model"), ) + m.keyMap.Prev = key.NewBinding( + key.WithKeys("left", "h"), + key.WithHelp("←", "previous"), + ) + m.keyMap.Next = key.NewBinding( + key.WithKeys("right", "l"), + key.WithHelp("→", "next"), + ) m.keyMap.Close = CloseKey return m @@ -133,9 +173,39 @@ func (m *ProviderWizard) HandleMsg(msg tea.Msg) Action { } return nil case key.Matches(msg, m.keyMap.Submit): + if m.step == providerWizardStepAttach { + return m.advance() + } + // On the model-ID screen an empty input means "done adding + // models": move to the context-window screen. With no models + // collected yet, fall through to advance() so the required + // list error surfaces instead of silently skipping. + if m.step == providerWizardStepModels && strings.TrimSpace(m.modelInput.Value()) == "" && len(m.models) > 0 { + m.step = providerWizardStepContext + m.errText = "" + m.focusStep() + return nil + } return m.advance() + case m.step == providerWizardStepAttach && + (key.Matches(msg, m.keyMap.Prev) || key.Matches(msg, m.keyMap.Next)): + if key.Matches(msg, m.keyMap.Prev) { + m.attachSel = boolOptionYes + } else { + m.attachSel = boolOptionNo + } + return nil default: // Backspace on an empty input steps back a screen. + if m.step == providerWizardStepAttach { + if msg.String() == "backspace" && m.step > providerWizardStepName { + m.step-- + m.errText = "" + m.focusStep() + return nil + } + return nil + } input := m.currentInput() if msg.String() == "backspace" && input.Value() == "" && m.step > providerWizardStepName { m.step-- @@ -151,6 +221,9 @@ func (m *ProviderWizard) HandleMsg(msg tea.Msg) Action { } } case tea.PasteMsg: + if m.step == providerWizardStepAttach { + return nil + } input := m.currentInput() var cmd tea.Cmd *input, cmd = input.Update(msg) @@ -169,8 +242,12 @@ func (m *ProviderWizard) currentInput() *textinput.Model { return &m.urlInput case providerWizardStepKey: return &m.keyInput - default: + case providerWizardStepModels: return &m.modelInput + case providerWizardStepContext: + return &m.contextWInput + default: + return nil } } @@ -179,7 +256,10 @@ func (m *ProviderWizard) focusStep() { m.urlInput.Blur() m.keyInput.Blur() m.modelInput.Blur() - m.currentInput().Focus() + m.contextWInput.Blur() + if input := m.currentInput(); input != nil { + input.Focus() + } } // advance validates the current step and moves to the next, or emits the @@ -229,11 +309,34 @@ func (m *ProviderWizard) advance() Action { m.errText = "Add at least one model id (press enter after typing it)." return nil } + m.step = providerWizardStepContext + m.focusStep() + case providerWizardStepContext: + raw := strings.TrimSpace(m.contextWInput.Value()) + if raw != "" { + contextW, err := strconv.ParseInt(raw, 10, 64) + if err != nil || contextW <= 0 { + m.errText = "Context window must be a positive number of tokens (e.g. 32768)." + return nil + } + m.contextW = contextW + } else { + // Navigating back and clearing the field must clear the + // stored value too, otherwise setup keeps the old window. + m.contextW = 0 + } + m.step = providerWizardStepAttach + m.focusStep() + case providerWizardStepAttach: + value := m.attachSel == boolOptionYes + m.attach = &value return ActionSetupProvider{ - ProviderID: m.providerID, - BaseURL: m.baseURL, - APIKey: m.apiKey, - Models: slices.Clone(m.models), + ProviderID: m.providerID, + BaseURL: m.baseURL, + APIKey: m.apiKey, + Models: slices.Clone(m.models), + ContextW: m.contextW, + Attachments: m.attach, } } return nil @@ -293,10 +396,14 @@ func (m *ProviderWizard) Draw(scr uv.Screen, area uv.Rectangle) *tea.Cursor { } lines = append(lines, m.stepLines()...) lines = append(lines, - textStyle.Render(fmt.Sprintf("Step %d of 4 · written to %s", m.step+1, config.GlobalConfigData())), + textStyle.Render(fmt.Sprintf("Step %d of %d · written to %s", m.step+1, providerWizardStepCount, config.GlobalConfigData())), "", helpStyle.Render(m.help.View(m)), ) + // Pad short steps so the dialog never changes size mid-flow. + for len(lines) < providerWizardHeight { + lines = append(lines, "") + } view := dialogStyle.Render(strings.Join(lines, "\n")) cur := m.Cursor() @@ -304,25 +411,32 @@ func (m *ProviderWizard) Draw(scr uv.Screen, area uv.Rectangle) *tea.Cursor { return cur } -// stepLines renders the running summary of everything collected so far so -// the user always sees the full picture, including the model list. +// stepLines renders the running summary of everything collected so far +// so the user always sees the full picture, including the model list. +// Fields whose value is not known yet hold a placeholder so the dialog +// keeps the same height on every step. func (m *ProviderWizard) stepLines() []string { t := m.com.Styles - var lines []string - if m.providerID != "" { - lines = append(lines, t.Dialog.SecondaryText.Render("provider: "+m.providerID)) + models := "(pending)" + if len(m.models) > 0 { + models = strings.Join(m.models, ", ") } - if m.baseURL != "" { - lines = append(lines, t.Dialog.SecondaryText.Render("url: "+m.baseURL)) + contextW := "(skipped)" + if m.contextW > 0 { + contextW = strconv.FormatInt(m.contextW, 10) } - if m.step == providerWizardStepModels { - if len(m.models) == 0 { - lines = append(lines, t.Dialog.SecondaryText.Render("models: (none yet)")) - } else { - lines = append(lines, t.Dialog.SecondaryText.Render("models: "+strings.Join(m.models, ", "))) - } + attach := "(pending)" + if m.attach != nil { + attach = fmt.Sprintf("%t", *m.attach) + } + return []string{ + t.Dialog.SecondaryText.Render("provider: " + cmp.Or(m.providerID, "(pending)")), + t.Dialog.SecondaryText.Render("url: " + cmp.Or(m.baseURL, "(pending)")), + t.Dialog.SecondaryText.Render("api key: " + cmp.Or(m.apiKey, "(none)")), + t.Dialog.SecondaryText.Render("models: " + models), + t.Dialog.SecondaryText.Render("context: " + contextW), + t.Dialog.SecondaryText.Render("attachments: " + attach), } - return lines } func (m *ProviderWizard) headerView() string { @@ -342,19 +456,49 @@ func (m *ProviderWizard) headerView() string { title = textStyle.Render("Enter the ") + accentStyle.Render("API Key") + textStyle.Render(" (optional).") case providerWizardStepModels: title = textStyle.Render("Add ") + accentStyle.Render("Model IDs") + textStyle.Render(".") + case providerWizardStepContext: + title = textStyle.Render("Set the ") + accentStyle.Render("Context Window") + textStyle.Render(" (optional).") + case providerWizardStepAttach: + title = textStyle.Render("Does it support ") + accentStyle.Render("Attachments") + textStyle.Render("?") } headerOffset := titleStyle.GetHorizontalFrameSize() + dialogStyle.GetHorizontalFrameSize() return common.DialogTitle(t, titleStyle.Render(title), m.width-headerOffset, t.Dialog.TitleGradFromColor, t.Dialog.TitleGradToColor) } +// inputView renders the current input, or the yes/no selector on the +// boolean capabilities screen. func (m *ProviderWizard) inputView() string { + if m.step == providerWizardStepAttach { + return m.attachView() + } return m.currentInput().View() } -// Cursor returns the cursor position relative to the dialog. +// attachView renders the yes/no selector with radio markers so the +// answer is always a valid boolean. +func (m *ProviderWizard) attachView() string { + t := m.com.Styles + yesRadio, noRadio := t.Radio.Off, t.Radio.Off + if m.attachSel == boolOptionYes { + yesRadio = t.Radio.On + } else { + noRadio = t.Radio.On + } + return fmt.Sprintf("%s %s %s %s", + yesRadio.Render(), t.Radio.Label.Render("Yes"), + noRadio.Render(), t.Radio.Label.Render("No"), + ) +} + +// Cursor returns the cursor position relative to the dialog. The +// boolean screen has no text input, so it shows no cursor. func (m *ProviderWizard) Cursor() *tea.Cursor { - return InputCursor(m.com.Styles, m.currentInput().Cursor()) + input := m.currentInput() + if input == nil { + return nil + } + return InputCursor(m.com.Styles, input.Cursor()) } // FullHelp returns the full help view. @@ -369,9 +513,11 @@ func (m *ProviderWizard) ShortHelp() []key.Binding { case providerWizardStepModels: submit := key.NewBinding( key.WithKeys("enter"), - key.WithHelp("enter", "add model / finish"), + key.WithHelp("enter", "add model / continue"), ) bindings = append(bindings, submit, m.keyMap.Remove) + case providerWizardStepAttach: + bindings = append(bindings, m.keyMap.Prev, m.keyMap.Next, m.keyMap.Submit) default: bindings = append(bindings, m.keyMap.Submit) } diff --git a/internal/ui/dialog/provider_wizard_test.go b/internal/ui/dialog/provider_wizard_test.go index 54e7156..5cb1bda 100644 --- a/internal/ui/dialog/provider_wizard_test.go +++ b/internal/ui/dialog/provider_wizard_test.go @@ -97,7 +97,7 @@ func TestProviderWizardAdvanceFlow(t *testing.T) { assert.Equal(t, providerWizardStepModels, w.step) // Step 4: enter with no models errors; typing + enter accumulates; - // enter on empty input emits the save action. + // enter on empty input moves to the context-window screen. w.modelInput.SetValue("") require.Nil(t, w.advance()) assert.NotEmpty(t, w.errText) @@ -110,6 +110,24 @@ func TestProviderWizardAdvanceFlow(t *testing.T) { require.Nil(t, w.advance()) assert.Equal(t, []string{"qwen3-32b", "llama-3.1-70b"}, w.models) + // HandleMsg routes the empty-input enter to the context-window screen. + w.errText = "" + require.Nil(t, w.HandleMsg(tea.KeyPressMsg{Code: tea.KeyEnter})) + assert.Equal(t, providerWizardStepContext, w.step) + + // Invalid context windows stay on the step. + w.contextWInput.SetValue("abc") + require.Nil(t, w.advance()) + assert.Equal(t, providerWizardStepContext, w.step) + assert.NotEmpty(t, w.errText) + + // Empty context window skips the value and moves to attachments. + w.contextWInput.SetValue("") + w.errText = "" + require.Nil(t, w.advance()) + assert.Equal(t, providerWizardStepAttach, w.step) + + // Enter on the boolean screen finishes with an explicit answer. action := w.advance() save, ok := action.(ActionSetupProvider) require.True(t, ok, "expected ActionSetupProvider, got %T", action) @@ -117,6 +135,69 @@ func TestProviderWizardAdvanceFlow(t *testing.T) { assert.Equal(t, "http://192.168.1.10:8000/v1", save.BaseURL) assert.Empty(t, save.APIKey) assert.Equal(t, []string{"qwen3-32b", "llama-3.1-70b"}, save.Models) + assert.Zero(t, save.ContextW) + require.NotNil(t, save.Attachments) + assert.True(t, *save.Attachments) +} + +func TestProviderWizardCapabilities(t *testing.T) { + t.Parallel() + + w := newTestWizard(t) + w.step = providerWizardStepContext + w.models = []string{"qwen3-32b"} + + w.contextWInput.SetValue("32768") + require.Nil(t, w.advance()) + assert.Equal(t, providerWizardStepAttach, w.step) + assert.Equal(t, int64(32768), w.contextW) + + // Invalid numbers stay on the step. + w.step = providerWizardStepContext + w.contextWInput.SetValue("32768 maybe") + require.Nil(t, w.advance()) + assert.Equal(t, providerWizardStepContext, w.step) + assert.NotEmpty(t, w.errText) + + w.contextWInput.SetValue("0") + require.Nil(t, w.advance()) + assert.Equal(t, providerWizardStepContext, w.step) + assert.NotEmpty(t, w.errText) + + // Empty input skips the value entirely, and clears any value + // entered on a previous pass through the step. + w.errText = "" + w.contextW = 32768 + w.contextWInput.SetValue("") + require.Nil(t, w.advance()) + assert.Equal(t, providerWizardStepAttach, w.step) + assert.Zero(t, w.contextW) + assert.Zero(t, w.attach) +} + +func TestProviderWizardAttachmentToggle(t *testing.T) { + t.Parallel() + + w := newTestWizard(t) + w.step = providerWizardStepAttach + w.models = []string{"qwen3-32b"} + + // The selector defaults to yes; arrows set an explicit boolean. + require.Nil(t, w.HandleMsg(tea.KeyPressMsg{Code: tea.KeyRight})) + assert.Equal(t, boolOptionNo, w.attachSel) + require.Nil(t, w.HandleMsg(tea.KeyPressMsg{Code: tea.KeyLeft})) + assert.Equal(t, boolOptionYes, w.attachSel) + + action := w.advance() + save, ok := action.(ActionSetupProvider) + require.True(t, ok, "expected ActionSetupProvider, got %T", action) + require.NotNil(t, save.Attachments) + assert.True(t, *save.Attachments) + + // Free-form typing on the boolean screen is ignored entirely. + w.HandleMsg(tea.KeyPressMsg{Code: 'x', Text: "x"}) + assert.Equal(t, boolOptionYes, w.attachSel) + assert.Equal(t, providerWizardStepAttach, w.step) } func TestProviderWizardKeys(t *testing.T) { @@ -139,16 +220,21 @@ func TestProviderWizardKeys(t *testing.T) { require.Nil(t, w.HandleMsg(tea.KeyPressMsg{Code: tea.KeyEnter})) assert.NotEmpty(t, w.errText) - // Re-adding a model and pressing enter on empty input finishes. + // Re-adding a model and pressing enter on empty input moves to the + // capabilities screens; enter on the context-window screen (empty) + // skips it, and enter on the boolean screen finishes the wizard. w.modelInput.SetValue("qwen3-32b") require.Nil(t, w.HandleMsg(tea.KeyPressMsg{Code: tea.KeyEnter})) + require.Nil(t, w.HandleMsg(tea.KeyPressMsg{Code: tea.KeyEnter})) + assert.Equal(t, providerWizardStepContext, w.step) + require.Nil(t, w.HandleMsg(tea.KeyPressMsg{Code: tea.KeyEnter})) + assert.Equal(t, providerWizardStepAttach, w.step) _, ok := w.HandleMsg(tea.KeyPressMsg{Code: tea.KeyEnter}).(ActionSetupProvider) require.True(t, ok, "expected ActionSetupProvider on finish") - // Backspace on the empty model input steps back to the key step. - w.modelInput.SetValue("") + // Backspace on the boolean screen steps back to the context step. require.Nil(t, w.HandleMsg(tea.KeyPressMsg{Code: tea.KeyBackspace})) - assert.Equal(t, providerWizardStepKey, w.step) + assert.Equal(t, providerWizardStepContext, w.step) // Escape closes the wizard. assert.Equal(t, ActionClose{}, w.HandleMsg(tea.KeyPressMsg{Code: tea.KeyEscape})) diff --git a/internal/ui/model/ui.go b/internal/ui/model/ui.go index 34407c3..a9876a0 100644 --- a/internal/ui/model/ui.go +++ b/internal/ui/model/ui.go @@ -1858,7 +1858,12 @@ func (m *UI) handleDialogMsg(msg tea.Msg) tea.Cmd { // Generic dialog messages case dialog.ActionClose: if isOnboarding && m.dialog.ContainsDialog(dialog.ModelsID) { - break + // The provider picker itself is inescapable during + // onboarding, but esc on any dialog opened on top of it + // (e.g. the custom provider wizard) returns to the picker. + if front := m.dialog.DialogLast(); front != nil && front.ID() == dialog.ModelsID { + break + } } if m.dialog.ContainsDialog(dialog.FilePickerID) { @@ -2604,7 +2609,9 @@ func (m *UI) handleSelectModel(msg dialog.ActionSelectModel) tea.Cmd { if isOnboarding { m.setState(uiLanding, uiFocusEditor) - m.com.Config().SetupAgents() + // SetupAgents is run by the config store's auto-reload once the + // first provider makes the config usable, so it is not needed + // here. if err := m.com.Workspace.InitSystemAgent(context.TODO()); err != nil { cmds = append(cmds, util.ReportError(err)) } @@ -4873,7 +4880,14 @@ func (m *UI) setupProvider(msg dialog.ActionSetupProvider) tea.Cmd { models := make([]map[string]any, 0, len(msg.Models)) for _, id := range msg.Models { - models = append(models, map[string]any{"id": id, "name": id}) + model := map[string]any{"id": id, "name": id} + if msg.ContextW > 0 { + model["context_window"] = msg.ContextW + } + if msg.Attachments != nil { + model["supports_attachments"] = *msg.Attachments + } + models = append(models, model) } type configField struct { @@ -4899,6 +4913,57 @@ func (m *UI) setupProvider(msg dialog.ActionSetupProvider) tea.Cmd { } m.dialog.CloseDialog(dialog.ProviderWizardID) + if m.state == uiOnboarding { + // handleSelectModel bails out (and closes nothing) when the + // agent is busy — another client can start the shared agent + // during onboarding in client/server mode. Check before closing + // the wizard and picker so the user can retry after the agent + // finishes instead of landing in a dialog-less dead end. The + // OAuth carve-out mirrors handleSelectModel. + if m.isAgentBusy() && !m.dialog.ContainsDialog(dialog.OAuthID) { + m.dialog.CloseDialog(dialog.ProviderWizardID) + return util.ReportWarn("Agent is busy, please wait...") + } + // The user just added this provider to set up their endpoint; + // pick its first model directly so onboarding completes instead + // of bouncing back to the provider list. This mirrors the + // API-key dialog, which selects the model the user was on. The + // small model defaults to the same one via GetDefaultSmallModel. + // handleSelectModel is called directly rather than emitted as an + // action: by the time an action would round-trip through + // handleDialogMsg, setupProvider has already flipped the state + // to landing and the onboarding branch would no longer run. + if len(msg.Models) > 0 { + m.dialog.CloseDialog(dialog.ModelsID) + if cmd := m.handleSelectModel(dialog.ActionSelectModel{ + Provider: catwalk.Provider{ + ID: catwalk.InferenceProvider(msg.ProviderID), + Name: msg.ProviderID, + }, + Model: config.SelectedModel{ + Provider: msg.ProviderID, + Model: msg.Models[0], + }, + ModelType: config.SelectedModelTypeLarge, + }); cmd != nil { + return cmd + } + return util.ReportInfo(fmt.Sprintf( + "Provider %q added. %s is ready — Ctrl+L to switch models.", + msg.ProviderID, msg.Models[0], + )) + } + // No models collected (older/invalid flow): refresh the list so + // the provider appears for manual selection. + m.dialog.CloseDialog(dialog.ModelsID) + if cmd := m.openModelsDialog(); cmd != nil { + return cmd + } + return util.ReportInfo(fmt.Sprintf( + "Provider %q added. Pick one of its models to continue.", + msg.ProviderID, + )) + } return util.ReportInfo(fmt.Sprintf( "Provider %q (%s) added to the global config. Press Ctrl+L to switch models.", msg.ProviderID, msg.BaseURL, diff --git a/pkg/config/custom_provider_capabilities_test.go b/pkg/config/custom_provider_capabilities_test.go new file mode 100644 index 0000000..ef44a75 --- /dev/null +++ b/pkg/config/custom_provider_capabilities_test.go @@ -0,0 +1,47 @@ +package config + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestCustomProviderCapabilitiesRoundTrip pins that capabilities fields +// written by the provider wizard survive the full load pipeline, so +// compaction and attachment handling see the user's choices. +func TestCustomProviderCapabilitiesRoundTrip(t *testing.T) { + dir := t.TempDir() + t.Setenv("PHOSPHOR_GLOBAL_CONFIG", dir) + t.Setenv("PHOSPHOR_GLOBAL_DATA", dir) + + configJSON := `{ + "providers": { + "ollama": { + "name": "ollama", + "type": "openai-compat", + "base_url": "http://localhost:11434/v1", + "models": [ + { + "id": "qwen3-32b", + "name": "qwen3-32b", + "context_window": 32768, + "supports_attachments": true + } + ] + } + } + }` + require.NoError(t, os.WriteFile(filepath.Join(dir, "phosphor.json"), []byte(configJSON), 0o600)) + + store, err := Load(dir, dir, false) + require.NoError(t, err) + + cfg := store.Config() + provider, ok := cfg.Providers.Get("ollama") + require.True(t, ok, "custom provider must be present") + require.Len(t, provider.Models, 1) + require.Equal(t, int64(32768), provider.Models[0].ContextWindow) + require.True(t, provider.Models[0].SupportsImages) +}