From ed451f3ef166b3bdcf7f556cb60edbc5b02adc63 Mon Sep 17 00:00:00 2001 From: Ruslan Strazhnyk Date: Wed, 2 Sep 2026 18:44:54 +0200 Subject: [PATCH 1/3] feat(tui): interactive /settings picker + /set hygiene fixes /settings (and bare /set) opens a keyboard-driven picker: toggles flip on Enter, enum settings (theme, cerebras model/effort) cycle, numeric settings edit inline; s saves the changed rows through the same apply path as /set, Esc discards. /set fixes from an audit: - strict numeric parsing: strconv.Atoi replaces fmt.Sscanf, which silently truncated inputs ('1e3' -> 1, '0x1f' -> 0, '12abc' -> 12) for project and budget - '/set project 0' now explicitly clears the default project - usage lists anthropic_key, which the switch already accepted - '/set apikey' / '/set anthropic-key' with no value prompt hidden via ReadSecret instead of requiring the secret on the visible input line - secret values are redacted from the in-session input history (both the interactive and queued-prompt append sites) - applySettingValue extracted as the single shared validation/apply path for /set, the picker, and future callers --- internal/repl/repl.go | 288 ++++++++++++++++++++++----- internal/repl/settings_test.go | 108 ++++++++++ internal/tui/input.go | 3 +- internal/tui/input_test.go | 2 +- internal/tui/settings_picker.go | 244 +++++++++++++++++++++++ internal/tui/settings_picker_test.go | 127 ++++++++++++ 6 files changed, 716 insertions(+), 56 deletions(-) create mode 100644 internal/repl/settings_test.go create mode 100644 internal/tui/settings_picker.go create mode 100644 internal/tui/settings_picker_test.go diff --git a/internal/repl/repl.go b/internal/repl/repl.go index 0cb012a..9f2a179 100644 --- a/internal/repl/repl.go +++ b/internal/repl/repl.go @@ -11,6 +11,8 @@ import ( "os" "os/signal" "path/filepath" + "sort" + "strconv" "strings" "sync" "syscall" @@ -253,7 +255,7 @@ func Run(ag *agent.Agent, cliAgent agent.CLIAgent, quietMode bool, version strin term.PrintSystem("processing queued prompt") } fmt.Println() - inputHistory = append(inputHistory, input) + inputHistory = append(inputHistory, redactSecretInput(input)) } else { result := tui.ReadInput(term.Prompt(), inputHistory, ag.Cfg.OutputVerbose, inputStatus()) @@ -279,7 +281,7 @@ func Run(ag *agent.Agent, cliAgent agent.CLIAgent, quietMode bool, version strin continue } inputWasPasted = result.Pasted - inputHistory = append(inputHistory, input) + inputHistory = append(inputHistory, redactSecretInput(input)) } // Built-in commands @@ -457,6 +459,10 @@ func Run(ag *agent.Agent, cliAgent agent.CLIAgent, quietMode bool, version strin handleSetCommand(input, ag, term) startCloudSession() continue + case input == "/settings" || strings.HasPrefix(input, "/settings "): + runSettingsPicker(ag, term) + startCloudSession() + continue case input == "/queue": items := pq.Peek() @@ -1806,15 +1812,71 @@ func printConfigInfo(cfg *api.Config, ctx *api.SessionContext, term *tui.Termina fmt.Println() } +// settingResult tells the caller what to do after applySettingValue runs. +type settingResult int + +const ( + settingInvalid settingResult = iota // validation failed; error already printed + settingApplied // applied; caller should persist via cfg.Save() + settingAppliedNoSave // applied; persistence handled elsewhere (keychain/auth.json/runtime) +) + +const setUsageKeys = "Keys: model, project, local_only, professional, autosave, cloud_sync, live_feed, output_verbose, budget, apikey, anthropic_key, ollama, backend, cerebras_model, cerebras_reasoning_effort, theme" + +// handleSetCommand implements /set. Bare `/set` opens the interactive +// settings picker (the keyboard-friendly form of this command); `/set ` +// for the secret keys prompts hidden instead of echoing the value into the +// terminal; `/set ` applies one setting directly. func handleSetCommand(input string, ag *agent.Agent, term *tui.Terminal) { parts := strings.Fields(input) - if len(parts) < 3 { - term.PrintError("Usage: /set ") - term.PrintSystem("Keys: model, project, local_only, professional, autosave, cloud_sync, live_feed, output_verbose, budget, apikey, ollama, backend, cerebras_model, cerebras_reasoning_effort, theme") + if len(parts) <= 1 { + runSettingsPicker(ag, term) return } key := strings.ToLower(parts[1]) - value := parts[2] + + if len(parts) == 2 { + switch key { + case "apikey", "anthropic-key", "anthropic_key": + // Prompt hidden rather than making the user paste a secret into the + // visible input line (and its history). + value := setup.ReadSecret(" Paste the value (blank to cancel): ") + if value == "" { + term.PrintSystem("No value entered — nothing changed.") + return + } + applyAndSaveSetting(key, value, ag, term) + return + } + term.PrintError("Usage: /set ") + term.PrintSystem(setUsageKeys) + return + } + + applyAndSaveSetting(key, parts[2], ag, term) +} + +// applyAndSaveSetting applies one key/value pair and persists when the apply +// path asks for it. +func applyAndSaveSetting(key, value string, ag *agent.Agent, term *tui.Terminal) { + cfg := ag.AppConfig + if cfg == nil { + cfg = api.DefaultConfig() + ag.AppConfig = cfg + } + if applySettingValue(key, value, ag, term) == settingApplied { + if err := cfg.Save(); err != nil { + term.PrintError(fmt.Sprintf("api.Config updated in memory but failed to save: %v", err)) + } else { + term.PrintSystem("api.Config saved to ~/.qmax-code/config.json") + } + } +} + +// applySettingValue validates and applies a single setting. It is the one +// shared path for /set, the /settings picker, and future callers, so +// validation and messaging can never drift between them. +func applySettingValue(key, value string, ag *agent.Agent, term *tui.Terminal) settingResult { cfg := ag.AppConfig if cfg == nil { cfg = api.DefaultConfig() @@ -1832,29 +1894,39 @@ func handleSetCommand(input string, ag *agent.Agent, term *tui.Terminal) { term.PrintSystem("Standalone local-only mode will be disabled after restart.") default: term.PrintError("Value must be true or false.") - return + return settingInvalid } case "model": if !api.IsValidClaudeModelName(value) { term.PrintError("Valid models: " + api.ValidClaudeModelsHelp()) - return + return settingInvalid } cfg.DefaultModel = api.ResolveClaudeModel(value) term.PrintSystem(fmt.Sprintf("Default model set to: %s", cfg.DefaultModel)) case "project": - if ag.Cfg.Context.LocalOnly { + if ag.Cfg.Context != nil && ag.Cfg.Context.LocalOnly { printStandaloneCloudUnavailable(term, "/set project") - return + return settingInvalid } - var pid int - if _, err := fmt.Sscanf(value, "%d", &pid); err != nil || pid < 0 { - term.PrintError("Project ID must be a non-negative integer.") - return + pid, err := strconv.Atoi(value) + if err != nil || pid < 0 { + term.PrintError("Project ID must be a positive integer (0 clears it).") + return settingInvalid + } + if pid == 0 { + cfg.DefaultProject = 0 + if ag.Cfg.Context != nil { + ag.Cfg.Context.ProjectID = 0 + } + term.PrintSystem("Default project cleared.") + return settingApplied } cfg.DefaultProject = pid - ag.Cfg.Context.ProjectID = pid + if ag.Cfg.Context != nil { + ag.Cfg.Context.ProjectID = pid + } term.PrintSystem(fmt.Sprintf("Default project set to: #%d", pid)) case "professional": @@ -1869,7 +1941,7 @@ func handleSetCommand(input string, ag *agent.Agent, term *tui.Terminal) { term.PrintSystem("Professional mode disabled. Cat personality restored.") default: term.PrintError("Value must be true or false.") - return + return settingInvalid } case "autosave": @@ -1882,7 +1954,7 @@ func handleSetCommand(input string, ag *agent.Agent, term *tui.Terminal) { term.PrintSystem("Auto-save disabled.") default: term.PrintError("Value must be true or false.") - return + return settingInvalid } case "output_verbose", "output-verbose", "output": @@ -1897,13 +1969,13 @@ func handleSetCommand(input string, ag *agent.Agent, term *tui.Terminal) { term.PrintSystem("Output mode set to compact.") default: term.PrintError("Value must be compact/verbose or true/false.") - return + return settingInvalid } case "cloud_sync", "cloudsync": - if ag.Cfg.Context.LocalOnly { + if ag.Cfg.Context != nil && ag.Cfg.Context.LocalOnly { printStandaloneCloudUnavailable(term, "/set cloud_sync") - return + return settingInvalid } switch strings.ToLower(value) { case "true", "1", "yes", "on": @@ -1916,53 +1988,57 @@ func handleSetCommand(input string, ag *agent.Agent, term *tui.Terminal) { term.PrintSystem("Cloud session sync disabled.") default: term.PrintError("Value must be true or false.") - return + return settingInvalid } case "live_feed", "live-feed", "livefeed": - if ag.Cfg.Context.LocalOnly { + if ag.Cfg.Context != nil && ag.Cfg.Context.LocalOnly { printStandaloneCloudUnavailable(term, "/set live_feed") - return + return settingInvalid } switch strings.ToLower(value) { case "true", "1", "yes", "on": cfg.LiveFeed = true - ag.Cfg.Context.LiveFeed = true + if ag.Cfg.Context != nil { + ag.Cfg.Context.LiveFeed = true + } term.PrintSystem("Live feed enabled — test runs and AI crawls will stream in QM Cloud Sandbox.") case "false", "0", "no", "off": cfg.LiveFeed = false - ag.Cfg.Context.LiveFeed = false + if ag.Cfg.Context != nil { + ag.Cfg.Context.LiveFeed = false + } term.PrintSystem("Live feed disabled.") default: term.PrintError("Value must be true or false.") - return + return settingInvalid } case "budget": - var budget int - if _, err := fmt.Sscanf(value, "%d", &budget); err != nil || budget < 0 { + budget, err := strconv.Atoi(value) + if err != nil || budget < 0 { term.PrintError("Budget must be a non-negative integer (token count).") - return + return settingInvalid } cfg.MaxTokenBudget = budget term.PrintSystem(fmt.Sprintf("Token budget set to: %d", budget)) case "apikey": - if ag.Cfg.Context.LocalOnly { + if ag.Cfg.Context != nil && ag.Cfg.Context.LocalOnly { printStandaloneCloudUnavailable(term, "/set apikey") - return + return settingInvalid } // Allow pasting API key directly: /set apikey qm-... auth, err := api.LoginWithAPIKey(value) if err != nil { term.PrintError(fmt.Sprintf("Invalid API key: %v", err)) - return + return settingInvalid } ag.Cfg.Context.Auth = auth ag.Cfg.Context.API = api.NewAPIClient(auth) tui.AnimateMax(tui.MoodHappy, fmt.Sprintf("Connected as %s", auth.Email)) fmt.Println() - return // auth.json handles persistence + return settingAppliedNoSave // auth.json handles persistence case "ollama": switch strings.ToLower(value) { @@ -1971,44 +2047,44 @@ func handleSetCommand(input string, ag *agent.Agent, term *tui.Terminal) { term.PrintError("No Ollama URL configured. Set it first:") term.PrintSystem(" qmax-code config set ollama_url https://user:pass@llm.example.com") term.PrintSystem(" qmax-code config set ollama_model gemma3:4b-it-q4_K_M") - return + return settingInvalid } if err := agent.ValidateOllamaURL(cfg.OllamaURL); err != nil { term.PrintError(fmt.Sprintf("Ollama URL rejected: %v", err)) - return + return settingInvalid } ag.Ollama = agent.NewOllamaClient(cfg) term.PrintSystem(fmt.Sprintf("Ollama enabled: %s (%s)", sysutil.MaskURL(cfg.OllamaURL), cfg.OllamaModel)) case "false", "0", "no", "off", "disabled": - if ag.Cfg.Context.LocalOnly && ag.Cfg.Context.Backend == "" && ag.Cfg.AnthropicKey == "" { + if ag.Cfg.Context != nil && ag.Cfg.Context.LocalOnly && ag.Cfg.Context.Backend == "" && ag.Cfg.AnthropicKey == "" { term.PrintError("Cannot disable Ollama: no Anthropic API key or CLI backend is active.") - return + return settingInvalid } ag.Ollama = nil term.PrintSystem("Ollama disabled. Using Claude for all calls.") default: term.PrintError("Value must be true/false (or enabled/disabled).") - return + return settingInvalid } - return // no config persistence needed — runtime toggle + return settingAppliedNoSave // runtime toggle — no config persistence needed case "backend": // /set backend cc|codex|api — persist backend choice. // For live switching use /cc, /codex, or /api instead. switch strings.ToLower(value) { case "cc": - if bin := agent.FindClaudeCode(); bin == "" { + if agent.FindClaudeCode() == "" { term.PrintError("'claude' CLI not found. Install Claude Code first.") term.PrintSystem(" https://claude.ai/download") - return + return settingInvalid } cfg.Backend = "cc" term.PrintSystem("Backend set to CC. Use /cc to switch live, or restart to apply.") case "codex": - if bin := agent.FindCodex(); bin == "" { + if agent.FindCodex() == "" { term.PrintError("'codex' CLI not found.") term.PrintSystem(" npm install -g @openai/codex") - return + return settingInvalid } cfg.Backend = "codex" term.PrintSystem("Backend set to Codex. Use /codex to switch live, or restart to apply.") @@ -2017,7 +2093,7 @@ func handleSetCommand(input string, ag *agent.Agent, term *tui.Terminal) { term.PrintSystem("Backend set to Anthropic API. Restart or use /api to switch live.") default: term.PrintError("Valid backends: cc, codex, api (use /gemma for cerebras)") - return + return settingInvalid } case "cerebras_model", "cerebras-model": @@ -2034,7 +2110,7 @@ func handleSetCommand(input string, ag *agent.Agent, term *tui.Terminal) { } else { if !api.ValidCerebrasReasoningEffort(value) { term.PrintError(fmt.Sprintf("Invalid value %q; allowed: none, low, medium, high", value)) - return + return settingInvalid } cfg.CerebrasReasoningEffort = api.NormalizeCerebrasReasoningEffort(value) } @@ -2058,7 +2134,7 @@ func handleSetCommand(input string, ag *agent.Agent, term *tui.Terminal) { } if !found { term.PrintError(fmt.Sprintf("Unknown theme %q. Available: %s", value, strings.Join(valid, ", "))) - return + return settingInvalid } cfg.Theme = strings.ToLower(value) tui.ApplyTheme(tui.ThemeByName(cfg.Theme)) @@ -2073,19 +2149,123 @@ func handleSetCommand(input string, ag *agent.Agent, term *tui.Terminal) { } else { term.PrintSystem("Anthropic API key saved to OS keychain.") } - return // don't save to config.json — keychain handles it + return settingAppliedNoSave // keychain handles persistence default: term.PrintError(fmt.Sprintf("Unknown config key: %s", key)) - term.PrintSystem("Keys: model, project, local_only, professional, autosave, cloud_sync, live_feed, output_verbose, budget, apikey, ollama, backend, cerebras_model, cerebras_reasoning_effort, theme") - return + term.PrintSystem(setUsageKeys) + return settingInvalid } - // Persist to disk - if err := cfg.Save(); err != nil { - term.PrintError(fmt.Sprintf("api.Config updated in memory but failed to save: %v", err)) - } else { - term.PrintSystem("api.Config saved to ~/.qmax-code/config.json") + return settingApplied +} + +// secretSetPrefixes are the /set forms whose value must never be stored in +// the in-session input history (up-arrow would otherwise recall the secret). +var secretSetPrefixes = []string{"/set apikey ", "/set anthropic-key ", "/set anthropic_key "} + +// redactSecretInput rewrites secret-carrying inputs to a redacted form before +// they enter the recallable history; anything else passes through unchanged. +func redactSecretInput(input string) string { + lower := strings.ToLower(input) + for _, p := range secretSetPrefixes { + if strings.HasPrefix(lower, p) { + return p + "" + } + } + return input +} + +// buildSettingsRows snapshots the config into picker rows. +func buildSettingsRows(cfg *api.Config) []tui.SettingsRow { + cloudSync := cfg.CloudSync != nil && *cfg.CloudSync + boolStr := func(b bool) string { + if b { + return "true" + } + return "false" + } + return []tui.SettingsRow{ + {Key: "project", Label: "Default project", Kind: tui.SettingsText, + Value: strconv.Itoa(cfg.DefaultProject), Hint: "0 = unset"}, + {Key: "budget", Label: "Token budget", Kind: tui.SettingsText, + Value: strconv.Itoa(cfg.MaxTokenBudget), Hint: "0 = unlimited"}, + {Key: "output_verbose", Label: "Output mode", Kind: tui.SettingsCycle, + Value: boolStr(cfg.OutputVerbose), Options: []string{"compact", "verbose"}, + Display: func(v string) string { + if v == "true" || v == "verbose" { + return "verbose" + } + return "compact" + }}, + {Key: "professional", Label: "Professional mode", Kind: tui.SettingsToggle, + Value: boolStr(cfg.Professional)}, + {Key: "autosave", Label: "Auto-save sessions", Kind: tui.SettingsToggle, + Value: boolStr(cfg.AutoSave)}, + {Key: "cloud_sync", Label: "Cloud session sync", Kind: tui.SettingsToggle, + Value: boolStr(cloudSync)}, + {Key: "live_feed", Label: "Live browser feed", Kind: tui.SettingsToggle, + Value: boolStr(cfg.LiveFeed)}, + {Key: "local_only", Label: "Local-only mode", Kind: tui.SettingsToggle, + Value: boolStr(cfg.LocalOnly), Hint: "restart applies"}, + {Key: "theme", Label: "Theme", Kind: tui.SettingsCycle, + Value: cfg.Theme, Options: tui.ThemeNames(), + Display: func(v string) string { + if v == "" { + return "(default)" + } + return v + }}, + {Key: "cerebras_model", Label: "Cerebras model", Kind: tui.SettingsCycle, + Value: cfg.CerebrasModel, Options: []string{api.CerebrasDefaultModel, api.CerebrasGemma4Model}, + Display: func(v string) string { + if v == "" { + return api.CerebrasDefaultModel + } + return v + }}, + {Key: "cerebras_reasoning_effort", Label: "Cerebras reasoning", Kind: tui.SettingsCycle, + Value: cfg.CerebrasReasoningEffort, Options: []string{"", "low", "medium", "high"}, + Display: func(v string) string { + if v == "" { + return "none (off)" + } + return v + }}, + } +} + +// runSettingsPicker opens the interactive settings editor and applies every +// changed row through applySettingValue — the same validation and messaging +// path /set uses. +func runSettingsPicker(ag *agent.Agent, term *tui.Terminal) { + cfg := ag.AppConfig + if cfg == nil { + cfg = api.DefaultConfig() + ag.AppConfig = cfg + } + res := tui.ShowSettingsPicker(buildSettingsRows(cfg)) + if !res.Confirmed { + term.PrintSystem("Settings unchanged.") + return + } + keys := make([]string, 0, len(res.Changes)) + for k := range res.Changes { + keys = append(keys, k) + } + sort.Strings(keys) + needSave := false + for _, k := range keys { + if applySettingValue(k, res.Changes[k], ag, term) == settingApplied { + needSave = true + } + } + if needSave { + if err := cfg.Save(); err != nil { + term.PrintError(fmt.Sprintf("api.Config updated in memory but failed to save: %v", err)) + } else { + term.PrintSystem("api.Config saved to ~/.qmax-code/config.json") + } } } diff --git a/internal/repl/settings_test.go b/internal/repl/settings_test.go new file mode 100644 index 0000000..92f5f37 --- /dev/null +++ b/internal/repl/settings_test.go @@ -0,0 +1,108 @@ +package repl + +import ( + "strings" + "testing" + + "github.com/qualitymax/qmax-code/internal/agent" + "github.com/qualitymax/qmax-code/internal/api" + "github.com/qualitymax/qmax-code/internal/tui" +) + +// TestApplySettingStrictNumericParsing pins the Sscanf-to-Atoi fix: partial +// numbers used to be silently truncated ("1e3" → 1, "0x1f" → 0, "12abc" → 12). +func TestApplySettingStrictNumericParsing(t *testing.T) { + cases := []struct{ key, value string }{ + {"project", "1e3"}, + {"project", "0x1f"}, + {"project", "12abc"}, + {"budget", "0x1f"}, + {"budget", "10k"}, + } + for _, tc := range cases { + ag := &agent.Agent{AppConfig: api.DefaultConfig()} + before := *ag.AppConfig + if got := applySettingValue(tc.key, tc.value, ag, &tui.Terminal{}); got != settingInvalid { + t.Errorf("applySettingValue(%q, %q) = %v, want settingInvalid", tc.key, tc.value, got) + } + if ag.AppConfig.DefaultProject != before.DefaultProject || ag.AppConfig.MaxTokenBudget != before.MaxTokenBudget { + t.Errorf("applySettingValue(%q, %q) mutated config despite rejection", tc.key, tc.value) + } + } +} + +func TestApplySettingProjectValues(t *testing.T) { + ag := &agent.Agent{} + if got := applySettingValue("project", "149", ag, &tui.Terminal{}); got != settingApplied { + t.Fatalf("project 149 = %v, want settingApplied", got) + } + if ag.AppConfig.DefaultProject != 149 { + t.Fatalf("DefaultProject = %d, want 149", ag.AppConfig.DefaultProject) + } + + if got := applySettingValue("project", "0", ag, &tui.Terminal{}); got != settingApplied { + t.Fatalf("project 0 = %v, want settingApplied", got) + } + if ag.AppConfig.DefaultProject != 0 { + t.Fatalf("project 0 should clear DefaultProject, got %d", ag.AppConfig.DefaultProject) + } + + if got := applySettingValue("project", "-3", ag, &tui.Terminal{}); got != settingInvalid { + t.Fatalf("project -3 = %v, want settingInvalid", got) + } +} + +func TestApplySettingUnknownKey(t *testing.T) { + ag := &agent.Agent{} + if got := applySettingValue("definitely_not_a_key", "1", ag, &tui.Terminal{}); got != settingInvalid { + t.Fatalf("unknown key = %v, want settingInvalid", got) + } +} + +// TestRedactSecretInput pins the history hygiene fix: /set values for the +// secret keys must never be recallable via up-arrow. +func TestRedactSecretInput(t *testing.T) { + cases := map[string]string{ + "/set apikey qm-live-secret123": "/set apikey ", + "/set anthropic-key sk-ant-abc123": "/set anthropic-key ", + "/SET ANTHROPIC_KEY sk-ant-abc123": "/set anthropic_key ", + "/set apikey": "/set apikey", + "/set theme dark": "/set theme dark", + "what is the anthropic-key for this": "what is the anthropic-key for this", + } + for in, want := range cases { + if got := redactSecretInput(in); got != want { + t.Errorf("redactSecretInput(%q) = %q, want %q", in, got, want) + } + } +} + +// TestBuildSettingsRowsSnapshot checks the picker rows reflect the config and +// that boolean defaults render as "false" rather than empty strings. +func TestBuildSettingsRowsSnapshot(t *testing.T) { + on := true + cfg := &api.Config{DefaultProject: 149, MaxTokenBudget: 5000, CloudSync: &on, Theme: "ocean"} + rows := buildSettingsRows(cfg) + + byKey := map[string]tui.SettingsRow{} + for _, r := range rows { + byKey[r.Key] = r + } + for _, key := range []string{"project", "budget", "cloud_sync", "theme", "cerebras_model", "cerebras_reasoning_effort"} { + if _, ok := byKey[key]; !ok { + t.Errorf("settings rows missing %q", key) + } + } + if byKey["project"].Value != "149" || byKey["budget"].Value != "5000" { + t.Errorf("project/budget snapshot wrong: %q / %q", byKey["project"].Value, byKey["budget"].Value) + } + if byKey["cloud_sync"].Value != "true" { + t.Errorf("cloud_sync with pointer true should snapshot as true, got %q", byKey["cloud_sync"].Value) + } + if byKey["theme"].Value != "ocean" { + t.Errorf("theme snapshot = %q, want ocean", byKey["theme"].Value) + } + if !strings.Contains(byKey["project"].Hint, "unset") { + t.Errorf("project row should hint that 0 unsets it") + } +} diff --git a/internal/tui/input.go b/internal/tui/input.go index 07bd72f..d9090e6 100644 --- a/internal/tui/input.go +++ b/internal/tui/input.go @@ -76,7 +76,8 @@ var slashMenuItems = []SlashMenuItem{ {"/browserfeed", "Live ASCII browser feed from a QM Cloud Sandbox noVNC URL"}, {"/paste", "Paste from clipboard (image or text)"}, {"/queue", "Show or add to prompt queue"}, - {"/set", "Update config"}, + {"/set", "Update one setting (bare /set opens the picker)"}, + {"/settings", "Interactive settings picker (toggles, cycles, values)"}, {"/gemma", "Gemma 4 31B on Cerebras (none|low|medium|high, off)"}, {"/ollama", "Toggle Ollama on/off"}, {"/clear", "Clear history"}, diff --git a/internal/tui/input_test.go b/internal/tui/input_test.go index 4fb3e43..0aad0c1 100644 --- a/internal/tui/input_test.go +++ b/internal/tui/input_test.go @@ -192,7 +192,7 @@ func TestSlashMenuCoversCriticalCommands(t *testing.T) { } critical := []string{ "/update", "/context", "/gemma", "/plan", // were missing - "/orch", "/help", "/set", "/clear", "/quit", "/gate", + "/orch", "/help", "/set", "/settings", "/clear", "/quit", "/gate", } for _, cmd := range critical { if !have[cmd] { diff --git a/internal/tui/settings_picker.go b/internal/tui/settings_picker.go new file mode 100644 index 0000000..a1cf6c0 --- /dev/null +++ b/internal/tui/settings_picker.go @@ -0,0 +1,244 @@ +package tui + +import ( + "fmt" + "strings" + + tea "github.com/charmbracelet/bubbletea" +) + +// SettingsPicker is a keyboard-driven editor for the qmax-code config that +// /set exposes as raw "key value" text. Rows are one of: +// +// SettingsToggle — boolean, Enter flips it +// SettingsCycle — fixed option list, Enter advances to the next option +// SettingsText — free text, Enter opens an inline editor, Enter commits +// +// `s` saves every changed row and exits; Esc (or q) discards and exits. The +// picker itself never touches config — the caller receives the per-key +// changes and applies them through the same code path as /set. + +type SettingsRowKind int + +const ( + SettingsToggle SettingsRowKind = iota + SettingsCycle + SettingsText +) + +// SettingsRow describes one editable setting. Value is the CURRENT value: +// "true"/"false" for toggles, one of Options for cycles, raw text otherwise. +// Display may override how Value renders (e.g. "" → "none") without changing +// what is committed. +type SettingsRow struct { + Key string + Label string + Kind SettingsRowKind + Value string + Options []string + Hint string + Display func(value string) string +} + +// SettingsPickerResult reports what the user changed. Changes holds only rows +// whose value differs from the initial one, keyed by SettingsRow.Key. +type SettingsPickerResult struct { + Confirmed bool + Changes map[string]string +} + +type settingsPickerModel struct { + rows []SettingsRow + initial map[string]string + cursor int + editing bool + editBuf string + done bool + saved bool +} + +func newSettingsPickerModel(rows []SettingsRow) settingsPickerModel { + initial := make(map[string]string, len(rows)) + for _, r := range rows { + initial[r.Key] = r.Value + } + return settingsPickerModel{rows: rows, initial: initial} +} + +func (m settingsPickerModel) Init() tea.Cmd { return nil } + +func (m settingsPickerModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + switch msg := msg.(type) { + case tea.KeyMsg: + if m.editing { + return m.updateEditing(msg) + } + return m.updateBrowsing(msg) + } + return m, nil +} + +func (m settingsPickerModel) updateBrowsing(msg tea.KeyMsg) (tea.Model, tea.Cmd) { + switch msg.Type { + case tea.KeyUp: + if m.cursor > 0 { + m.cursor-- + } + case tea.KeyDown: + if m.cursor < len(m.rows)-1 { + m.cursor++ + } + case tea.KeyEnter: + if m.cursor >= 0 && m.cursor < len(m.rows) { + row := &m.rows[m.cursor] + switch row.Kind { + case SettingsToggle: + if row.Value == "true" { + row.Value = "false" + } else { + row.Value = "true" + } + case SettingsCycle: + row.Value = nextCycleOption(row.Options, row.Value) + case SettingsText: + m.editing = true + m.editBuf = row.Value + } + } + case tea.KeyEsc, tea.KeyCtrlC: + m.done, m.saved = true, false + return m, tea.Quit + default: + // Single-letter shortcuts outside edit mode: s = save, q = quit. + if msg.Type == tea.KeyRunes { + switch strings.ToLower(string(msg.Runes)) { + case "s": + m.done, m.saved = true, true + return m, tea.Quit + case "q": + m.done, m.saved = true, false + return m, tea.Quit + } + } + } + return m, nil +} + +func (m settingsPickerModel) updateEditing(msg tea.KeyMsg) (tea.Model, tea.Cmd) { + row := &m.rows[m.cursor] + switch msg.Type { + case tea.KeyEnter: + row.Value = m.editBuf + m.editing = false + case tea.KeyEsc, tea.KeyCtrlC: + m.editing = false + case tea.KeyBackspace: + if n := len(m.editBuf); n > 0 { + runes := []rune(m.editBuf) + m.editBuf = string(runes[:len(runes)-1]) + } + case tea.KeyRunes: + m.editBuf += string(msg.Runes) + } + return m, nil +} + +func nextCycleOption(options []string, current string) string { + if len(options) == 0 { + return current + } + for i, o := range options { + if o == current { + return options[(i+1)%len(options)] + } + } + return options[0] +} + +func (m settingsPickerModel) changes() map[string]string { + changes := map[string]string{} + for _, r := range m.rows { + if r.Value != m.initial[r.Key] { + changes[r.Key] = r.Value + } + } + return changes +} + +func (m settingsPickerModel) rowDisplay(r SettingsRow) string { + if r.Display != nil { + return r.Display(r.Value) + } + return r.Value +} + +func (m settingsPickerModel) View() string { + var b strings.Builder + b.WriteString(pickerLabel.Render(" qmax-code settings")) + b.WriteByte('\n') + b.WriteString(pickerFooter.Render(" Backend & model: /orch · API keys: /keys")) + b.WriteByte('\n') + b.WriteString(pickerDivider.Render(strings.Repeat("─", 52))) + b.WriteByte('\n') + + for i, r := range m.rows { + isCursor := i == m.cursor + arrow := " " + if isCursor { + arrow = pickerBadgeStar.Render("▶ ") + } + + label := r.Label + if isCursor { + label = pickerLabelSel.Render(label) + } else { + label = pickerLabel.Render(label) + } + + value := m.rowDisplay(r) + if isCursor && m.editing { + value = pickerBadgeCurrent.Render(m.editBuf + "▌") + } else if r.Value != m.initial[r.Key] { + value = pickerBadgeCurrent.Render(value + " *") + } else { + value = pickerFooter.Render(value) + } + if r.Hint != "" { + value += pickerFooter.Render(" " + r.Hint) + } + + row := fmt.Sprintf("%s%-24s %s", arrow, label, value) + if isCursor { + b.WriteString(pickerRowSelected.Render(row)) + } else { + b.WriteString(pickerRowNormal.Render(row)) + } + b.WriteByte('\n') + } + + b.WriteString(pickerDivider.Render(strings.Repeat("─", 52))) + b.WriteByte('\n') + if m.editing { + b.WriteString(pickerFooter.Render(" Enter commit · Esc cancel edit")) + } else { + b.WriteString(pickerFooter.Render(" ↑↓ navigate · Enter change · s save · Esc/q discard")) + } + b.WriteByte('\n') + return pickerBox.Render(b.String()) +} + +// ShowSettingsPicker opens the settings editor. It never fails: a program +// error is reported as a plain cancellation. +func ShowSettingsPicker(rows []SettingsRow) SettingsPickerResult { + m := newSettingsPickerModel(rows) + p := tea.NewProgram(m) + result, err := p.Run() + if err != nil { + return SettingsPickerResult{} + } + final, ok := result.(settingsPickerModel) + if !ok || !final.done || !final.saved { + return SettingsPickerResult{} + } + return SettingsPickerResult{Confirmed: true, Changes: final.changes()} +} diff --git a/internal/tui/settings_picker_test.go b/internal/tui/settings_picker_test.go new file mode 100644 index 0000000..227911e --- /dev/null +++ b/internal/tui/settings_picker_test.go @@ -0,0 +1,127 @@ +package tui + +import ( + "testing" + + tea "github.com/charmbracelet/bubbletea" +) + +func pickerRows() []SettingsRow { + return []SettingsRow{ + {Key: "autosave", Label: "Auto-save", Kind: SettingsToggle, Value: "false"}, + {Key: "theme", Label: "Theme", Kind: SettingsCycle, Value: "ocean", Options: []string{"ocean", "forest", "sunset"}}, + {Key: "budget", Label: "Budget", Kind: SettingsText, Value: "1000"}, + } +} + +func updateSettings(t *testing.T, m settingsPickerModel, msg tea.KeyMsg) settingsPickerModel { + t.Helper() + updated, _ := m.Update(msg) + next, ok := updated.(settingsPickerModel) + if !ok { + t.Fatalf("Update returned %T, want settingsPickerModel", updated) + } + return next +} + +func TestSettingsPickerToggleFlipsValue(t *testing.T) { + m := newSettingsPickerModel(pickerRows()) + m = updateSettings(t, m, tea.KeyMsg{Type: tea.KeyEnter}) + if m.rows[0].Value != "true" { + t.Fatalf("Enter on toggle: value = %q, want true", m.rows[0].Value) + } + m = updateSettings(t, m, tea.KeyMsg{Type: tea.KeyEnter}) + if m.rows[0].Value != "false" { + t.Fatalf("second Enter on toggle: value = %q, want false", m.rows[0].Value) + } +} + +func TestSettingsPickerCycleAdvancesAndWraps(t *testing.T) { + m := newSettingsPickerModel(pickerRows()) + m.cursor = 1 + + m = updateSettings(t, m, tea.KeyMsg{Type: tea.KeyEnter}) + if m.rows[1].Value != "forest" { + t.Fatalf("cycle 1: value = %q, want forest", m.rows[1].Value) + } + m = updateSettings(t, m, tea.KeyMsg{Type: tea.KeyEnter}) + if m.rows[1].Value != "sunset" { + t.Fatalf("cycle 2: value = %q, want sunset", m.rows[1].Value) + } + m = updateSettings(t, m, tea.KeyMsg{Type: tea.KeyEnter}) + if m.rows[1].Value != "ocean" { + t.Fatalf("cycle 3 should wrap: value = %q, want ocean", m.rows[1].Value) + } +} + +func TestSettingsPickerTextEditCommitsAndCancels(t *testing.T) { + m := newSettingsPickerModel(pickerRows()) + m.cursor = 2 + + m = updateSettings(t, m, tea.KeyMsg{Type: tea.KeyEnter}) + if !m.editing { + t.Fatal("Enter on text row should open the inline editor") + } + // The editor pre-fills the current value ("1000") — clear it before typing. + for range "1000" { + m = updateSettings(t, m, tea.KeyMsg{Type: tea.KeyBackspace}) + } + for _, r := range "5000" { + m = updateSettings(t, m, tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{r}}) + } + m = updateSettings(t, m, tea.KeyMsg{Type: tea.KeyEnter}) + if m.editing { + t.Fatal("Enter in the editor should close it") + } + if m.rows[2].Value != "5000" { + t.Fatalf("committed value = %q, want 5000", m.rows[2].Value) + } + + // Esc during editing discards the buffer, keeping the row value. + m = updateSettings(t, m, tea.KeyMsg{Type: tea.KeyEnter}) + m = updateSettings(t, m, tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'9'}}) + m = updateSettings(t, m, tea.KeyMsg{Type: tea.KeyEsc}) + if m.editing { + t.Fatal("Esc should close the editor") + } + if m.rows[2].Value != "5000" { + t.Fatalf("Esc-cancelled edit leaked into value: %q", m.rows[2].Value) + } +} + +func TestSettingsPickerChangesOnlyReportsChangedRows(t *testing.T) { + m := newSettingsPickerModel(pickerRows()) + m = updateSettings(t, m, tea.KeyMsg{Type: tea.KeyEnter}) // flip autosave + + changes := m.changes() + if len(changes) != 1 { + t.Fatalf("changes = %v, want exactly the flipped autosave row", changes) + } + if changes["autosave"] != "true" { + t.Fatalf("changes[autosave] = %q, want true", changes["autosave"]) + } +} + +func TestSettingsPickerSaveAndDiscardKeys(t *testing.T) { + m := newSettingsPickerModel(pickerRows()) + + saved := updateSettings(t, m, tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'s'}}) + if !saved.done || !saved.saved { + t.Fatal("'s' should save and exit") + } + + discarded := updateSettings(t, m, tea.KeyMsg{Type: tea.KeyEsc}) + if !discarded.done || discarded.saved { + t.Fatal("Esc should exit without saving") + } +} + +func TestNextCycleOptionFallsBackToFirst(t *testing.T) { + got := nextCycleOption([]string{"a", "b"}, "not-in-list") + if got != "a" { + t.Fatalf("nextCycleOption fallback = %q, want a", got) + } + if got := nextCycleOption(nil, "x"); got != "x" { + t.Fatalf("empty options should keep current, got %q", got) + } +} From 60b62e6c32b79ef15c4e3b1bfefb7bcdc786bc7f Mon Sep 17 00:00:00 2001 From: Ruslan Strazhnyk Date: Wed, 2 Sep 2026 18:59:06 +0200 Subject: [PATCH 2/3] fix(repl): make secret-history redaction whitespace-tolerant Review follow-up on #184: strings.Fields accepts multi-space and tab separators, so '/set apikey ' executed as an apikey set but slipped past the fixed-prefix redaction check. A single whitespace-tolerant regex now matches every accepted form; tests cover the double-space and tab variants. --- internal/repl/repl.go | 15 +++++++-------- internal/repl/settings_test.go | 7 +++++-- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/internal/repl/repl.go b/internal/repl/repl.go index 9f2a179..63f1e82 100644 --- a/internal/repl/repl.go +++ b/internal/repl/repl.go @@ -11,6 +11,7 @@ import ( "os" "os/signal" "path/filepath" + "regexp" "sort" "strconv" "strings" @@ -2160,18 +2161,16 @@ func applySettingValue(key, value string, ag *agent.Agent, term *tui.Terminal) s return settingApplied } -// secretSetPrefixes are the /set forms whose value must never be stored in -// the in-session input history (up-arrow would otherwise recall the secret). -var secretSetPrefixes = []string{"/set apikey ", "/set anthropic-key ", "/set anthropic_key "} +// secretSetRe matches any /set form that carries a secret value, tolerating +// the whitespace variations strings.Fields accepts ("/set apikey k", +// tabs, ...) so none of them can slip past the redaction into history. +var secretSetRe = regexp.MustCompile(`(?i)^/set\s+(apikey|anthropic[-_]key)\s+\S`) // redactSecretInput rewrites secret-carrying inputs to a redacted form before // they enter the recallable history; anything else passes through unchanged. func redactSecretInput(input string) string { - lower := strings.ToLower(input) - for _, p := range secretSetPrefixes { - if strings.HasPrefix(lower, p) { - return p + "" - } + if m := secretSetRe.FindStringSubmatch(input); m != nil { + return "/set " + m[1] + " " } return input } diff --git a/internal/repl/settings_test.go b/internal/repl/settings_test.go index 92f5f37..f01bd20 100644 --- a/internal/repl/settings_test.go +++ b/internal/repl/settings_test.go @@ -60,12 +60,15 @@ func TestApplySettingUnknownKey(t *testing.T) { } // TestRedactSecretInput pins the history hygiene fix: /set values for the -// secret keys must never be recallable via up-arrow. +// secret keys must never be recallable via up-arrow — including the +// whitespace variations strings.Fields accepts as valid commands. func TestRedactSecretInput(t *testing.T) { cases := map[string]string{ "/set apikey qm-live-secret123": "/set apikey ", "/set anthropic-key sk-ant-abc123": "/set anthropic-key ", - "/SET ANTHROPIC_KEY sk-ant-abc123": "/set anthropic_key ", + "/SET ANTHROPIC_KEY sk-ant-abc123": "/set ANTHROPIC_KEY ", + "/set apikey qm-live-secret123": "/set apikey ", + "/set\tanthropic-key\tsk-ant-x": "/set anthropic-key ", "/set apikey": "/set apikey", "/set theme dark": "/set theme dark", "what is the anthropic-key for this": "what is the anthropic-key for this", From fed3069b9873d91b29ed72c6cd94ae8aeb7487c1 Mon Sep 17 00:00:00 2001 From: Ruslan Strazhnyk Date: Wed, 2 Sep 2026 20:22:45 +0200 Subject: [PATCH 3/3] fix(repl): guard nil Context in apikey set; NBSP-tolerant secret redaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 2 on #184: - /set apikey dereferenced ag.Cfg.Context unguarded (latent in the original code too) — a zero-value session context is now created before the auth write - Go regexp \s is ASCII-only but strings.Fields splits on unicode spaces (NBSP U+00A0), so a NBSP-separated '/set apikey ' executed yet escaped redaction; the matcher now covers \p{Zs} separators --- internal/repl/repl.go | 7 ++++++- internal/repl/settings_test.go | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/repl/repl.go b/internal/repl/repl.go index 63f1e82..e176a2a 100644 --- a/internal/repl/repl.go +++ b/internal/repl/repl.go @@ -2029,6 +2029,11 @@ func applySettingValue(key, value string, ag *agent.Agent, term *tui.Terminal) s printStandaloneCloudUnavailable(term, "/set apikey") return settingInvalid } + // The auth update below writes through Context; guarantee it exists + // (a zero-value Agent in tests or early startup has it nil). + if ag.Cfg.Context == nil { + ag.Cfg.Context = &api.SessionContext{} + } // Allow pasting API key directly: /set apikey qm-... auth, err := api.LoginWithAPIKey(value) if err != nil { @@ -2164,7 +2169,7 @@ func applySettingValue(key, value string, ag *agent.Agent, term *tui.Terminal) s // secretSetRe matches any /set form that carries a secret value, tolerating // the whitespace variations strings.Fields accepts ("/set apikey k", // tabs, ...) so none of them can slip past the redaction into history. -var secretSetRe = regexp.MustCompile(`(?i)^/set\s+(apikey|anthropic[-_]key)\s+\S`) +var secretSetRe = regexp.MustCompile(`(?i)^/set[\s\p{Zs}]+(apikey|anthropic[-_]key)[\s\p{Zs}]+\S`) // redactSecretInput rewrites secret-carrying inputs to a redacted form before // they enter the recallable history; anything else passes through unchanged. diff --git a/internal/repl/settings_test.go b/internal/repl/settings_test.go index f01bd20..9f7f1f1 100644 --- a/internal/repl/settings_test.go +++ b/internal/repl/settings_test.go @@ -69,6 +69,7 @@ func TestRedactSecretInput(t *testing.T) { "/SET ANTHROPIC_KEY sk-ant-abc123": "/set ANTHROPIC_KEY ", "/set apikey qm-live-secret123": "/set apikey ", "/set\tanthropic-key\tsk-ant-x": "/set anthropic-key ", + "/set\u00A0apikey\u00A0qm-live-x": "/set apikey ", // NBSP is a unicode space strings.Fields splits on "/set apikey": "/set apikey", "/set theme dark": "/set theme dark", "what is the anthropic-key for this": "what is the anthropic-key for this",