diff --git a/internal/tui/config_test.go b/internal/tui/config_test.go index d8808f4..2ab1ae2 100644 --- a/internal/tui/config_test.go +++ b/internal/tui/config_test.go @@ -422,12 +422,23 @@ func TestConfigureToolsWritesEveryEnabledTool(t *testing.T) { } } - if msg := configureTools(testKey, nil); !strings.Contains(msg, "5 added") { + msg, written := configureTools(testKey, nil) + if !strings.Contains(msg, "5 added") { t.Fatalf("configureTools said %q, want the five tools written", msg) } if len(*hermesCalls) == 0 { t.Error("Hermes was counted but never configured") } + // The names come back so the tab can say how to use each one; a tool + // written but not named leaves a member with a config and no next step. + if len(written) != 5 { + t.Errorf("configureTools named %v, want all five it wrote", written) + } + for _, name := range written { + if _, ok := nextStepFor[name]; !ok { + t.Errorf("%s is configured and the tab has nothing to tell anyone about using it", name) + } + } for name, p := range paths { data, err := os.ReadFile(p) @@ -447,7 +458,8 @@ func TestConfigureToolsWritesEveryEnabledTool(t *testing.T) { } // And unticking a tool takes only that one out. - if msg := configureTools(testKey, map[string]bool{"Pi": false}); !strings.Contains(msg, "1 removed") { + msg, _ = configureTools(testKey, map[string]bool{"Pi": false}) + if !strings.Contains(msg, "1 removed") { t.Errorf("configureTools said %q, want Pi removed", msg) } if isNaNConfigured("Pi", paths["Pi"]) { @@ -1227,3 +1239,54 @@ func TestTheKeyForTheKeyWorksFromWhereItIsAdvertised(t *testing.T) { t.Error("e opened the key editor without moving to the tab that shows it") } } + +// Pressing c used to call configureTools straight out of the key handler, so +// the panel sat frozen for as long as it took - and with Hermes in the list +// that is four processes and several seconds, with no repaint and no key +// accepted. Reported as the panel being hung, which is the only thing it +// could look like. +func TestConfiguringDoesNotBlockTheEventLoop(t *testing.T) { + m := setupModel(t, &session.Session{Token: "t", APIKey: testKey}) + m.lay = newLayout(90, 30) + m.active = tabIndex(tabSetup) + recordHermes(t) + + updated, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'c'}}) + after := updated.(model) + + if !after.configuring { + t.Error("c does not put the tab into a state it can draw") + } + if cmd == nil { + t.Fatal("c did the work inline instead of handing back a command") + } + // And the tab says so rather than looking hung. + if out := after.renderSetup(after.lay); !strings.Contains(out, "writing the configs") { + t.Errorf("nothing on screen says it is working:\n%s", out) + } +} + +// "5 added" answers what it did, not the question a member is left holding. +func TestTheTabSaysHowToUseWhatItJustConfigured(t *testing.T) { + m := setupModel(t, &session.Session{Token: "t", APIKey: testKey}) + m.lay = newLayout(90, 40) + m.configured = []string{"OpenCode", "Codex"} + + out := m.renderSetup(m.lay) + for _, want := range []string{"Now open them", "opencode", "/models", "codex"} { + if !strings.Contains(out, want) { + t.Errorf("the tab does not mention %q after configuring", want) + } + } +} + +// Every tool the Setup tab can configure has to have something to say about +// using it, or the list it prints has a hole in it exactly where a member +// looks. +func TestEveryConfigurableToolHasANextStep(t *testing.T) { + for _, tool := range detectTools() { + if _, ok := nextStepFor[tool.name]; !ok { + t.Errorf("%s can be configured and has no next step", tool.name) + } + } +} diff --git a/internal/tui/tui.go b/internal/tui/tui.go index db9dfc0..8602525 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -128,6 +128,11 @@ type keyCheckedMsg struct { // someone to quit and run something is the detour this replaced. var errNotSignedIn = errors.New("not signed in — press s") +type configuredMsg struct { + msg string + written []string +} + type linkSentMsg struct{ err error } type signedInMsg struct{ err error } @@ -153,6 +158,9 @@ type model struct { keyCheck string // Signing in, without leaving the panel. See startLogin. + configuring bool + configured []string + loginStage loginStage loginInput textinput.Model loginEmail string @@ -224,7 +232,7 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.lay = newLayout(msg.Width, msg.Height) case spinner.TickMsg: - if m.loading { + if m.loading || m.configuring { var cmd tea.Cmd m.spin, cmd = m.spin.Update(msg) return m, cmd @@ -239,6 +247,11 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.loading = false m.err = msg.err + case configuredMsg: + m.configuring = false + m.setupMsg = msg.msg + m.configured = msg.written + case linkSentMsg: m.loginBusy = false if msg.err != nil { @@ -404,7 +417,7 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } } case " ": - if !m.showHelp && m.activeID() == tabSetup { + if !m.showHelp && m.activeID() == tabSetup && !m.configuring { tools := detectTools() if m.setupCursor < len(tools) && tools[m.setupCursor].installed { name := tools[m.setupCursor].name @@ -449,15 +462,22 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.setupMsg = "" } case "c": - if !m.showHelp && m.activeID() == tabSetup { + if !m.showHelp && m.activeID() == tabSetup && !m.configuring { // Pressing the key that configures everything and having // nothing happen, with nothing said, is the worst of the // three possible answers. if m.sess.APIKey == "" { m.setupMsg = "error: set your API key first — press e" - } else { - m.setupMsg = configureTools(m.sess.APIKey, m.sess.EnabledTools) + return m, nil } + m.configuring = true + m.configured = nil + m.setupMsg = "" + key, tools := m.sess.APIKey, m.sess.EnabledTools + return m, tea.Batch(m.spin.Tick, func() tea.Msg { + msg, written := configureTools(key, tools) + return configuredMsg{msg, written} + }) } } } @@ -1551,7 +1571,15 @@ func (m model) toolEnabled(name string) bool { return true } -func configureTools(apiKey string, enabledTools map[string]bool) string { +// Writes the configs, and says which tools it wrote so the tab can tell a +// member how to use each one. +// +// This runs off the event loop. It used to be called straight out of the key +// handler, which meant the panel sat frozen for as long as it took - and with +// Hermes in the list that is four processes, several seconds, with no repaint +// and no key accepted. Reported as "se ha quedado paralizado y no sabia que +// pasaba", which is the only thing it could look like. +func configureTools(apiKey string, enabledTools map[string]bool) (string, []string) { isEnabled := func(name string) bool { if enabledTools == nil { return true @@ -1563,6 +1591,7 @@ func configureTools(apiKey string, enabledTools map[string]bool) string { } tools := detectTools() var lastErr error + var written []string added, removed := 0, 0 for _, t := range tools { if !t.installed { @@ -1586,6 +1615,7 @@ func configureTools(apiKey string, enabledTools map[string]bool) string { lastErr = err } else { added++ + written = append(written, t.name) } } else if t.configured { var err error @@ -1609,7 +1639,7 @@ func configureTools(apiKey string, enabledTools map[string]bool) string { } } if lastErr != nil { - return "error: " + lastErr.Error() + return "error: " + lastErr.Error(), written } parts := []string{} if added > 0 { @@ -1619,9 +1649,9 @@ func configureTools(apiKey string, enabledTools map[string]bool) string { parts = append(parts, fmt.Sprintf("%d removed", removed)) } if len(parts) == 0 { - return "nothing to sync" + return "nothing to sync", written } - return strings.Join(parts, " · ") + return strings.Join(parts, " · "), written } func factoryCustomID(displayName string, index int) string { @@ -2246,6 +2276,27 @@ func (m model) renderLogin(l layout) string { return b.String() } +// How to actually use each tool once its config is written. The panel said +// "4 added" and stopped there, which answers what it did and not the question +// a member is left holding: and now what. These are the steps each tool page +// on nan.builders publishes under "check that it works". +var nextStepFor = map[string][2]string{ + "OpenCode": {"opencode", "then /models inside it, and pick a NaN one"}, + "Codex": {"codex", "already aimed at the cluster; codex --model to switch"}, + "Pi": {"pi", "NaN is already its default provider"}, + "Factory AI": {"droid", "pick a model with (NaN) in its name"}, + "Hermes": {"hermes doctor", "says whether the provider answers, then talk to it"}, +} + +// Pads to a column width. renderCosts keeps its own; this is package-level +// because a second renderer now needs the same thing. +func lpadTo(v string, w int) string { + if len(v) >= w { + return v + } + return v + strings.Repeat(" ", w-len(v)) +} + func (m model) renderSetup(l layout) string { var b strings.Builder @@ -2301,6 +2352,12 @@ func (m model) renderSetup(l layout) string { // ── Tools ──────────────────────────────────────────────────────────────── b.WriteString("\n" + l.indent + titleStyle.Render("Tools") + "\n") + if m.configuring { + // Several seconds, because Hermes is configured by running Hermes. + // Saying so beats a panel that looks hung. + b.WriteString(l.indent + m.spin.View() + + dimStyle.Render(" writing the configs - a few seconds, Hermes is asked rather than written") + "\n") + } if m.setupMsg != "" { style := okStyle if strings.HasPrefix(m.setupMsg, "error") { @@ -2308,6 +2365,25 @@ func (m model) renderSetup(l layout) string { } b.WriteString(l.indent + style.Render(m.setupMsg) + "\n") } + + // And now what. The answer used to be nowhere in the panel. + if len(m.configured) > 0 { + b.WriteString("\n" + l.indent + titleStyle.Render("Now open them") + "\n\n") + width := 0 + for _, name := range m.configured { + if step, ok := nextStepFor[name]; ok && len(step[0]) > width { + width = len(step[0]) + } + } + for _, name := range m.configured { + step, ok := nextStepFor[name] + if !ok { + continue + } + b.WriteString(l.indent + accentStyle.Render(lpadTo(step[0], width+2)) + + dimStyle.Render(step[1]) + "\n") + } + } b.WriteString("\n") tools := detectTools() @@ -2373,7 +2449,7 @@ func (m model) renderSetup(l layout) string { // ── about renderer ─────────────────────────────────────────────────────────── -const Version = "0.1.11" +const Version = "0.1.12" func renderAbout(l layout) string { var b strings.Builder diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 2d7186f..1d50a49 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -118,7 +118,7 @@ function Get-LatestVersion { could not work out the latest version from the GitHub API it rate limits unauthenticated requests, so this is usually temporary wait a few minutes, or pick a version yourself: - & ([scriptblock]::Create((irm https://nan.builders/install.ps1))) -Version v0.1.11 + & ([scriptblock]::Create((irm https://nan.builders/install.ps1))) -Version v0.1.12 the releases are at https://github.com/$Repo/releases "@ } diff --git a/scripts/install.sh b/scripts/install.sh index 461aab2..2230aa6 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -62,7 +62,7 @@ require_version() { err "could not work out the latest version from the GitHub API" err "it rate limits unauthenticated requests, so this is usually temporary" err "wait a few minutes, or pick a version yourself:" - printf " VERSION=v0.1.11 curl -fsSL https://nan.builders/install | bash + printf " VERSION=v0.1.12 curl -fsSL https://nan.builders/install | bash " >&2 err "the releases are at https://github.com/$REPO/releases" exit 1