diff --git a/cmd/late/bootstrap_test.go b/cmd/late/bootstrap_test.go new file mode 100644 index 0000000..6f45d70 --- /dev/null +++ b/cmd/late/bootstrap_test.go @@ -0,0 +1,54 @@ +package main + +import ( + "errors" + "fmt" + "strings" + "testing" +) + +// TestInitialBootstrapStatus guards the pre-program status-bar decision: +// a failed app-config load surfaces as a "config error: ..." warning (Step 1 +// wraps the error with the exact config path, so the path must survive +// verbatim), while a clean load returns "" so main() falls back to the +// plain "Starting..." text. +func TestInitialBootstrapStatus(t *testing.T) { + tests := []struct { + name string + loadErr error + want string + }{ + { + name: "clean load defers to the plain startup status", + loadErr: nil, + want: "", + }, + { + name: "load error becomes a config error warning", + loadErr: errors.New("/Users/u/Library/Application Support/late/config.json: trailing comma at line 3"), + want: "config error: /Users/u/Library/Application Support/late/config.json: trailing comma at line 3", + }, + { + name: "wrapped error keeps its full cause text", + loadErr: fmt.Errorf("reading config %s: %w", "/Users/u/Library/Application Support/late/config.json", errors.New("invalid character '}' looking for beginning of value")), + want: "config error: reading config /Users/u/Library/Application Support/late/config.json: invalid character '}' looking for beginning of value", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := initialBootstrapStatus(tt.loadErr) + if got != tt.want { + t.Fatalf("initialBootstrapStatus(%v) = %q, want %q", tt.loadErr, got, tt.want) + } + if tt.loadErr != nil { + if !strings.HasPrefix(got, "config error: ") { + t.Fatalf("warning must be prefixed with %q, got %q", "config error: ", got) + } + if !strings.Contains(got, "config.json") { + t.Fatalf("warning must retain the config path, got %q", got) + } + } + }) + } +} diff --git a/cmd/late/main.go b/cmd/late/main.go index bb16d27..45cc796 100644 --- a/cmd/late/main.go +++ b/cmd/late/main.go @@ -362,8 +362,12 @@ func main() { } } } - // Load App configuration + // Load App configuration. A load error means the run proceeds with + // degraded defaults (LoadConfig already wraps the error with the exact + // config path); keep the message so the TUI status bar can surface it + // before backend discovery reports. appConfig, err := appconfig.LoadConfig() + configLoadWarning := initialBootstrapStatus(err) if err != nil { fmt.Fprintf(os.Stderr, "Warning: Failed to load app config: %v\n", err) } @@ -676,7 +680,15 @@ func main() { pOpts = append(pOpts, tea.WithWindowSize(w, h)) } - model.BootstrapStatus = "Starting..." + // A degraded app config surfaces as the initial status-bar text so the + // user sees it on the first paint; the async BootstrapStatusMsg traffic + // below replaces it as soon as backend discovery reports. "Starting..." + // only applies to a clean config load. + if configLoadWarning != "" { + model.BootstrapStatus = configLoadWarning + } else { + model.BootstrapStatus = "Starting..." + } p := tea.NewProgram(model, pOpts...) // toolSync serializes plugin/MCP tool-registry refreshes triggered by @@ -788,6 +800,18 @@ func deriveEffectiveSessionID(historyPath string) string { } return id } + +// initialBootstrapStatus decides what the TUI status bar shows before the +// async backend-discovery messages arrive. A failed app-config load returns +// the "config error: ..." warning (the error already names the exact config +// path); a clean load returns "" so the caller falls back to "Starting...". +func initialBootstrapStatus(loadErr error) string { + if loadErr != nil { + return fmt.Sprintf("config error: %v", loadErr) + } + return "" +} + func newModelClient(ctx context.Context, setting appconfig.ModelSetting, enableImages bool, logitBias map[string]int) *client.Client { c := client.NewClient(client.Config{ BaseURL: setting.URL, diff --git a/internal/config/config.go b/internal/config/config.go index 4b0463f..936089b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -86,6 +86,15 @@ type Config struct { Theme string `json:"theme,omitempty"` Models []ModelSetting `json:"models,omitempty"` AgentModels map[string]string `json:"agent_models,omitempty"` + + // Degraded is set by LoadConfig when config.json exists but could not + // be read or parsed: the returned config is a fallback default, not + // the user's real settings. SaveConfig refuses to persist a degraded + // config so that runtime toggles (/infobar, /timestamps, /model) + // cannot overwrite the user's hand-edited config.json with defaults. + // It is never serialized (json:"-") and is NOT set when the file is + // merely missing — that is a normal fresh install. + Degraded bool `json:"-"` } func defaultConfig() Config { @@ -136,7 +145,8 @@ func LoadConfig() (*Config, error) { } fallback := defaultConfig() - return &fallback, err + fallback.Degraded = true + return &fallback, fmt.Errorf("failed to read %s: %w", configPath, err) } permErr := ensureSecureConfigPermissions(lateConfigDir, configPath) @@ -144,7 +154,8 @@ func LoadConfig() (*Config, error) { var cfg Config if err := json.Unmarshal(content, &cfg); err != nil { fallback := defaultConfig() - return &fallback, err + fallback.Degraded = true + return &fallback, fmt.Errorf("failed to parse %s: %w", configPath, err) } if cfg.EnabledTools == nil { @@ -361,7 +372,13 @@ func (cfg *Config) GetModelForAgent(agentType string) (ModelSetting, bool) { } // SaveConfig atomically writes the configuration back to config.json. +// A degraded config (loaded from an invalid config.json) is never saved: +// the caller must fix or remove the file first, so a fallback default can +// never clobber the user's hand-edited config. func SaveConfig(cfg *Config) error { + if cfg != nil && cfg.Degraded { + return fmt.Errorf("refusing to save config: it was loaded from an invalid config.json; fix or remove the file first") + } lateConfigDir, err := pathutil.LateConfigDir() if err != nil { return err diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 5d4f8c1..1550adb 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -838,3 +838,140 @@ func TestConfig_PermissionModeJSONRoundTrip(t *testing.T) { t.Fatalf("empty config should not marshal a permission-mode key, got %s", emptyData) } } + +// TestLoadConfig_DegradationGuard covers the config degradation guard: a +// config.json that cannot be parsed or read yields the fallback default +// together with an error naming the exact file path and a Degraded flag +// that makes SaveConfig refuse to overwrite the user's file. A valid file +// (including unknown extra keys, which must keep parsing permissively) +// loads non-degraded and saves normally. A missing file (fresh install) is +// covered by TestLoadConfig_MissingFileCreatesDefault and stays +// non-degraded. +func TestLoadConfig_DegradationGuard(t *testing.T) { + cases := []struct { + name string + configContent string + wantErr bool + wantPathInErr bool + wantDegraded bool + // roundTripModel: non-empty for savable configs — SaveConfig must + // succeed and the model must survive a save/reload round trip. + roundTripModel string + }{ + { + name: "valid config parses without degradation", + configContent: `{"enabled_tools":{"bash":true},"openai_model":"gpt-test"}`, + wantErr: false, + wantDegraded: false, + roundTripModel: "gpt-test", + }, + { + name: "trailing comma is a parse error naming the path", + configContent: `{"enabled_tools":{"bash":true},}`, + wantErr: true, + wantPathInErr: true, + wantDegraded: true, + }, + { + name: "wrong-typed field is a parse error naming the path", + configContent: `{"theme":123}`, + wantErr: true, + wantPathInErr: true, + wantDegraded: true, + }, + { + name: "unknown extra field parses permissively", + configContent: `{"totally-new-option":123}`, + wantErr: false, + wantDegraded: false, + roundTripModel: "", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + configRoot := t.TempDir() + setUserConfigEnv(t, configRoot) + configPath := lateConfigPath(t) + + if err := os.MkdirAll(filepath.Dir(configPath), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(configPath, []byte(tc.configContent), 0o644); err != nil { + t.Fatal(err) + } + + cfg, err := LoadConfig() + if tc.wantErr { + if err == nil { + t.Fatal("LoadConfig() expected an error, got nil") + } + if tc.wantPathInErr && !strings.Contains(err.Error(), configPath) { + t.Fatalf("LoadConfig() error = %q, want it to contain the config path %q", err.Error(), configPath) + } + } else if err != nil { + t.Fatalf("LoadConfig() error = %v, want nil", err) + } + if cfg == nil { + t.Fatal("LoadConfig() returned nil config") + } + if cfg.Degraded != tc.wantDegraded { + t.Fatalf("cfg.Degraded = %v, want %v", cfg.Degraded, tc.wantDegraded) + } + + if !tc.wantDegraded { + if err := SaveConfig(cfg); err != nil { + t.Fatalf("SaveConfig() error = %v, want nil for a non-degraded config", err) + } + if tc.roundTripModel != "" { + reloaded, err := LoadConfig() + if err != nil { + t.Fatalf("LoadConfig() after save error = %v", err) + } + if reloaded.OpenAIModel != tc.roundTripModel { + t.Fatalf("round-tripped OpenAIModel = %q, want %q", reloaded.OpenAIModel, tc.roundTripModel) + } + if reloaded.Degraded { + t.Fatal("reloaded config after a normal save must not be degraded") + } + } + return + } + + before, err := os.ReadFile(configPath) + if err != nil { + t.Fatal(err) + } + saveErr := SaveConfig(cfg) + if saveErr == nil { + t.Fatal("SaveConfig() expected a refusal error for a degraded config, got nil") + } + if !strings.Contains(saveErr.Error(), "refusing to save config") { + t.Fatalf("SaveConfig() error = %q, want it to mention refusing to save", saveErr.Error()) + } + after, err := os.ReadFile(configPath) + if err != nil { + t.Fatal(err) + } + if string(after) != string(before) { + t.Fatalf("SaveConfig() modified a degraded config's file:\nbefore: %s\nafter: %s", before, after) + } + }) + } +} + +// TestConfig_DegradedNotSerialized pins that the runtime-only Degraded flag +// never leaks into config.json (json:"-"). +func TestConfig_DegradedNotSerialized(t *testing.T) { + data, err := json.Marshal(&Config{Degraded: true}) + if err != nil { + t.Fatalf("Marshal() error = %v", err) + } + var raw map[string]any + if err := json.Unmarshal(data, &raw); err != nil { + t.Fatalf("Unmarshal() error = %v", err) + } + if _, ok := raw["Degraded"]; ok { + t.Fatalf("Degraded must not be serialized, got %s", data) + } +} diff --git a/internal/tui/config_persist_feedback_test.go b/internal/tui/config_persist_feedback_test.go new file mode 100644 index 0000000..31a864d --- /dev/null +++ b/internal/tui/config_persist_feedback_test.go @@ -0,0 +1,145 @@ +package tui + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "late/internal/config" + "late/internal/pathutil" +) + +// isolateConfigHome points the OS user-config directory at a fresh temp dir +// for the duration of the test, so SaveConfig never touches the developer's +// real config.json (the same isolation model_picker_test.go uses; darwin's +// os.UserConfigDir ignores XDG_CONFIG_HOME, hence the HOME/APPDATA override). +func isolateConfigHome(t *testing.T) string { + t.Helper() + configHome := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", configHome) + t.Setenv("HOME", configHome) + t.Setenv("APPDATA", configHome) + return configHome +} + +// TestThemePickerWarnsWhenSaveRefusesDegradedConfig drives the theme picker's +// enter path with a degraded config (constructed the way the app really gets +// one: config.LoadConfig over a malformed config.json). The theme applies for +// the session, but SaveConfig must refuse to overwrite the user's broken file +// — and the refusal must surface on the focused agent's status line instead +// of being silently swallowed. +func TestThemePickerWarnsWhenSaveRefusesDegradedConfig(t *testing.T) { + isolateConfigHome(t) + lateDir, err := pathutil.LateConfigDir() + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(lateDir, 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(lateDir, "config.json"), []byte("{not json"), 0o600); err != nil { + t.Fatal(err) + } + cfg, err := config.LoadConfig() + if err == nil { + t.Fatal("expected a parse error from the malformed config.json") + } + if !cfg.Degraded { + t.Fatal("expected LoadConfig to mark the fallback config degraded") + } + + model := NewModel(&mockOrchestrator{}, nil, cfg) + model.Mode = ViewThemes + model.ThemeEntries = []ThemeEntry{makeTheme("ocean:deep", "ocean", "deep")} + model.ThemeIndex = 0 + + updated, _ := model.updateChat(mockKey{code: '\r', text: "enter"}) + + if !strings.Contains(updated.ToastMessage, "theme applied") { + t.Fatalf("toast = %q, want the theme-applied confirmation", updated.ToastMessage) + } + if updated.SelectedTheme != "ocean:deep" { + t.Fatalf("SelectedTheme = %q, want ocean:deep (the session keeps the theme)", updated.SelectedTheme) + } + status := updated.GetAgentState(updated.Focused.ID()).StatusText + if !strings.Contains(status, "settings changed but won't persist") { + t.Fatalf("StatusText = %q, want the persistence-failure warning", status) + } + if !strings.Contains(status, "refusing to save config") { + t.Fatalf("StatusText = %q, want it to carry SaveConfig's refusal reason", status) + } + // The refusal is the point: the malformed file must survive untouched. + data, err := os.ReadFile(filepath.Join(lateDir, "config.json")) + if err != nil { + t.Fatal(err) + } + if string(data) != "{not json" { + t.Fatalf("degraded config.json was overwritten: %q", string(data)) + } +} + +// TestThemesCommandWarnsWhenSaveRefusesDegradedConfig drives the +// /themes inline path with a config flagged degraded directly: same +// contract as the picker — apply in memory, warn on the status line. +func TestThemesCommandWarnsWhenSaveRefusesDegradedConfig(t *testing.T) { + isolateConfigHome(t) + cfg := &config.Config{Degraded: true} + model := NewModel(&mockOrchestrator{}, nil, cfg) + model.ThemeEntries = []ThemeEntry{makeTheme("ocean:deep", "ocean", "deep")} + model.Input.SetValue("/themes deep") + + updated, _ := model.updateChat(mockKey{code: '\r', text: "enter"}) + + if updated.SelectedTheme != "ocean:deep" { + t.Fatalf("SelectedTheme = %q, want ocean:deep", updated.SelectedTheme) + } + status := updated.GetAgentState(updated.Focused.ID()).StatusText + if !strings.Contains(status, "settings changed but won't persist") { + t.Fatalf("StatusText = %q, want the persistence-failure warning", status) + } + if !strings.Contains(status, "refusing to save config") { + t.Fatalf("StatusText = %q, want it to carry SaveConfig's refusal reason", status) + } +} + +// TestThemePickerNoWarningWhenPersistenceSucceeds pins the healthy path: a +// non-degraded config saves, and no persistence warning appears on the +// status line. +func TestThemePickerNoWarningWhenPersistenceSucceeds(t *testing.T) { + isolateConfigHome(t) + lateDir, err := pathutil.LateConfigDir() + if err != nil { + t.Fatal(err) + } + // LoadConfig (fresh install) is what normally creates the dir; mirror + // that here so SaveConfig's permission tightening finds it. + if err := os.MkdirAll(lateDir, 0o700); err != nil { + t.Fatal(err) + } + cfg := &config.Config{} + model := NewModel(&mockOrchestrator{}, nil, cfg) + model.Mode = ViewThemes + model.ThemeEntries = []ThemeEntry{makeTheme("ocean:deep", "ocean", "deep")} + model.ThemeIndex = 0 + + updated, _ := model.updateChat(mockKey{code: '\r', text: "enter"}) + + status := updated.GetAgentState(updated.Focused.ID()).StatusText + if strings.Contains(status, "won't persist") { + t.Fatalf("StatusText = %q, want no persistence warning when SaveConfig succeeds", status) + } + // And the choice really persisted to the isolated config dir. + data, err := os.ReadFile(filepath.Join(lateDir, "config.json")) + if err != nil { + t.Fatal(err) + } + var saved config.Config + if err := json.Unmarshal(data, &saved); err != nil { + t.Fatalf("saved config.json is not valid JSON: %v", err) + } + if saved.Theme != "ocean:deep" { + t.Fatalf("persisted theme = %q, want ocean:deep", saved.Theme) + } +} diff --git a/internal/tui/model_picker_test.go b/internal/tui/model_picker_test.go index 00c45ef..7881160 100644 --- a/internal/tui/model_picker_test.go +++ b/internal/tui/model_picker_test.go @@ -1,12 +1,16 @@ package tui import ( - "late/internal/config" - "late/internal/pathutil" "os" + "path/filepath" + "strings" "testing" tea "charm.land/bubbletea/v2" + "github.com/charmbracelet/x/ansi" + + "late/internal/config" + "late/internal/pathutil" ) func TestModelPickerAppliesOrchestratorModelImmediately(t *testing.T) { @@ -180,3 +184,36 @@ func TestModelPickerPublishesOnlyAfterSaveSucceeds(t *testing.T) { t.Fatal("model was applied after config save failed") } } + +// TestModelPickerEmptyStateShowsRealConfigPath verifies the empty-state hint +// points at the OS-real config location (pathutil.LateConfigDir) instead of the +// hardcoded ~/.config/late literal (Diagnosis item 2). +func TestModelPickerEmptyStateShowsRealConfigPath(t *testing.T) { + configHome := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", configHome) + t.Setenv("HOME", configHome) + t.Setenv("APPDATA", configHome) + + wantPath, err := pathutil.LateConfigDir() + if err != nil { + t.Skipf("LateConfigDir unavailable in test env: %v", err) + } + wantPath = filepath.Join(wantPath, "config.json") + + // Empty picker: AppConfig present but zero models, only the default row. + model := NewModel(&mockOrchestrator{}, nil, &config.Config{}) + model.Viewport.SetWidth(200) + model.Viewport.SetHeight(40) + model.Mode = ViewModelPicker + model.ModelPickerAgents = []string{"orchestrator"} + model.ModelPickerModels = []string{"default"} + model.renderModelPickerView() + + plain := ansi.Strip(model.Viewport.View()) + if !strings.Contains(plain, "No models configured in "+wantPath) { + t.Fatalf("model picker empty state missing real config path %q, got:\n%s", wantPath, plain) + } + if strings.Contains(plain, "~/.config/late") { + t.Fatalf("model picker empty state shows hardcoded ~ path, got:\n%s", plain) + } +} diff --git a/internal/tui/update.go b/internal/tui/update.go index 892d195..51c0f09 100644 --- a/internal/tui/update.go +++ b/internal/tui/update.go @@ -657,7 +657,14 @@ func (m Model) updateChat(msg tea.Msg) (Model, tea.Cmd) { m.ToastExpireTime = time.Now().UnixMilli() + 3000 if m.AppConfig != nil { m.AppConfig.Theme = info.ID - _ = config.SaveConfig(m.AppConfig) + if err := config.SaveConfig(m.AppConfig); err != nil { + // The theme IS applied for this session, but a + // degraded config (config.json existed but could + // not be read/parsed) makes SaveConfig refuse to + // overwrite it: say so instead of silently + // dropping the user's choice on the next start. + focusedState.StatusText = "settings changed but won't persist: " + err.Error() + } } } clearCmd := tea.Tick(4*time.Second, func(t time.Time) tea.Msg { @@ -1283,7 +1290,14 @@ func (m Model) updateChat(msg tea.Msg) (Model, tea.Cmd) { m.ToastExpireTime = time.Now().UnixMilli() + 3000 if m.AppConfig != nil { m.AppConfig.Theme = info.ID - _ = config.SaveConfig(m.AppConfig) + if err := config.SaveConfig(m.AppConfig); err != nil { + // The theme IS applied for this session, but a + // degraded config (config.json existed but could + // not be read/parsed) makes SaveConfig refuse to + // overwrite it: say so instead of silently + // dropping the user's choice on the next start. + focusedState.StatusText = "settings changed but won't persist: " + err.Error() + } } } clearCmd := tea.Tick(4*time.Second, func(t time.Time) tea.Msg { diff --git a/internal/tui/view.go b/internal/tui/view.go index 8f7bf36..f1f65c4 100644 --- a/internal/tui/view.go +++ b/internal/tui/view.go @@ -11,6 +11,7 @@ import ( "unicode/utf8" "late/internal/common" + "late/internal/pathutil" "charm.land/bubbles/v2/spinner" tea "charm.land/bubbletea/v2" @@ -360,7 +361,7 @@ func (m *Model) renderMinimalEqualizerAt(now time.Time) string { // Gentle incommensurate harmonic (golden ratio 1.618) creates organic, non-repeating crests // Low amplitude ensures it never causes erratic snap or jitter - w2 := 0.35 * math.Sin(t*0.93 + float64(i)*0.55 + 1.2) + w2 := 0.35 * math.Sin(t*0.93+float64(i)*0.55+1.2) // Breathing envelope gives gentle natural cadence swell := 0.88 + 0.20*math.Sin(t*0.38+float64(i)*0.25) @@ -1414,7 +1415,13 @@ func (m *Model) renderModelPickerView() { lines = append(lines, header, "") if len(m.ModelPickerModels) <= 1 && (m.AppConfig == nil || len(m.AppConfig.Models) == 0) { - lines = append(lines, viewEmptyStyle.Copy().Foreground(warnBorderColor).Render("No models configured in ~/.config/late/config.json")) + // Point the user at the real OS config location; fall back to the + // literal only if the platform config dir cannot be resolved. + emptyMsg := "No models configured in ~/.config/late/config.json" + if cfgDir, dirErr := pathutil.LateConfigDir(); dirErr == nil { + emptyMsg = fmt.Sprintf("No models configured in %s", filepath.Join(cfgDir, "config.json")) + } + lines = append(lines, viewEmptyStyle.Copy().Foreground(warnBorderColor).Render(emptyMsg)) lines = append(lines, "", viewEmptyStyle.Render("Please add a 'models' array to your config file first.")) } else { // Instructions