From faf03cccfc430c3a29f8d81bd98a55f5c77ddc4e Mon Sep 17 00:00:00 2001
From: Steve Calvert <steve.calvert@glean.com>
Date: Sat, 11 Apr 2026 14:00:40 -0700
Subject: [PATCH] fix: surface config save errors instead of silently
 swallowing them

SaveConfig() previously only returned an error when both keyring and
file writes failed. Now logs independently when one backend fails.
Also fixes AddTurn()/AppendTurn() to return save errors instead of
silently logging them.
---
 internal/config/config.go      | 18 +++++++++++++-----
 internal/config/config_test.go |  4 ++--
 internal/tui/commands.go       |  4 +++-
 internal/tui/model.go          |  8 ++++++--
 internal/tui/session.go        |  8 +++++---
 5 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/internal/config/config.go b/internal/config/config.go
index 94fb8d5..4240185 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -128,7 +128,6 @@ func loadFromEnv() *Config {
 }
 
 // SaveConfig stores host and token in both the system keyring and file storage.
-// It returns an error only if both storage methods fail.
 func SaveConfig(host, token string) error {
 	if host != "" {
 		validHost, err := ValidateAndTransformHost(host)
@@ -163,11 +162,20 @@ func SaveConfig(host, token string) error {
 		cfg.GleanToken = token
 	}
 
-	if err := saveToFile(cfg); err != nil && keyringErr != nil {
-		return fmt.Errorf("failed to save config: keyring error: %v, file error: %v", keyringErr, err)
-	}
+	fileErr := saveToFile(cfg)
 
-	return nil
+	switch {
+	case keyringErr != nil && fileErr != nil:
+		return fmt.Errorf("failed to save config: keyring: %v, file: %v", keyringErr, fileErr)
+	case fileErr != nil:
+		cfgLog.Log("warning: config file write failed (keyring OK): %v", fileErr)
+		return nil
+	case keyringErr != nil:
+		cfgLog.Log("keyring unavailable, config saved to file only: %v", keyringErr)
+		return nil
+	default:
+		return nil
+	}
 }
 
 // SaveHostToFile persists only the host in ~/.glean/config.json without touching
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
index 877b396..4b0ac0e 100644
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -270,8 +270,8 @@ func TestConfigOperations(t *testing.T) {
 		err = SaveConfig("linkedin", "test-token")
 		assert.Error(t, err)
 		assert.Contains(t, err.Error(), "failed to save config")
-		assert.Contains(t, err.Error(), "keyring error")
-		assert.Contains(t, err.Error(), "file error")
+		assert.Contains(t, err.Error(), "keyring:")
+		assert.Contains(t, err.Error(), "file:")
 
 		// Reset mock error for other tests
 		mock.err = nil
diff --git a/internal/tui/commands.go b/internal/tui/commands.go
index 2294ed3..bb51900 100644
--- a/internal/tui/commands.go
+++ b/internal/tui/commands.go
@@ -118,7 +118,9 @@ func (m *Model) handleSlashCommand(input string) (tea.Model, tea.Cmd) {
 // System turns are rendered in the viewport but never sent to the Glean API.
 func (m *Model) addSystemMessage(text string) {
 	turn := Turn{Role: roleSystem, Content: text}
-	m.session.AppendTurn(turn)
+	if err := m.session.AppendTurn(turn); err != nil {
+		sessionLog.Log("save failed: %v", err)
+	}
 	if !m.conversationActive {
 		m.conversationActive = true
 		m.viewport.Height = m.maxViewportHeight()
diff --git a/internal/tui/model.go b/internal/tui/model.go
index ed334c3..978430d 100644
--- a/internal/tui/model.go
+++ b/internal/tui/model.go
@@ -410,7 +410,9 @@ func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 			}
 
 			// Display turn uses original question for clean viewport rendering.
-			m.session.AddTurn(roleUser, question, nil)
+			if err := m.session.AddTurn(roleUser, question, nil); err != nil {
+				sessionLog.Log("save failed: %v", err)
+			}
 
 			// API message carries file context when present.
 			apiText := apiContent
@@ -489,7 +491,9 @@ func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 				Elapsed: msg.elapsed,
 			}
 			m.addTurnToConversation(turn)
-			m.session.AppendTurn(turn) // preserves Elapsed for renderConversation
+			if err := m.session.AppendTurn(turn); err != nil {
+				sessionLog.Log("save failed: %v", err)
+			}
 		}
 		m.viewport.SetContent(m.renderConversation())
 		m.viewport.GotoBottom()
diff --git a/internal/tui/session.go b/internal/tui/session.go
index 9e9eb20..51bc6f2 100644
--- a/internal/tui/session.go
+++ b/internal/tui/session.go
@@ -81,15 +81,17 @@ func (s *Session) Save() error {
 }
 
 // AddTurn appends a turn to the session and saves immediately.
-func (s *Session) AddTurn(role, content string, sources []Source) {
-	s.AppendTurn(Turn{Role: role, Content: content, Sources: sources})
+func (s *Session) AddTurn(role, content string, sources []Source) error {
+	return s.AppendTurn(Turn{Role: role, Content: content, Sources: sources})
 }
 
 // AppendTurn appends a complete Turn (including Elapsed and any other fields)
 // to the session and saves immediately.
-func (s *Session) AppendTurn(turn Turn) {
+func (s *Session) AppendTurn(turn Turn) error {
 	s.Turns = append(s.Turns, turn)
 	if err := s.Save(); err != nil {
 		sessionLog.Log("save failed: %v", err)
+		return err
 	}
+	return nil
 }