From 1a925b3a057150eee0ce383a18efc33c1a427b49 Mon Sep 17 00:00:00 2001 From: Felix Martin Date: Mon, 1 Jun 2026 20:13:33 -0400 Subject: [PATCH] Clarify settings apply/save comment; strengthen settings tests Co-Authored-By: Claude Opus 4.8 --- internal/web/settings_handlers.go | 8 +++++--- internal/web/settings_handlers_test.go | 26 ++++++++++++++++++++++++-- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/internal/web/settings_handlers.go b/internal/web/settings_handlers.go index f0ecd75..c147391 100644 --- a/internal/web/settings_handlers.go +++ b/internal/web/settings_handlers.go @@ -26,9 +26,11 @@ func (s *Server) handleGetSettings(c *gin.Context) { c.JSON(http.StatusOK, cur) } -// handlePostSettings validates-and-applies atomically, then persists. The applier -// checks the backend before mutating any state, so an invalid backend yields 400 -// with nothing saved and the prior wiring intact. +// handlePostSettings applies the new settings first, then persists them. The +// applier checks the backend before mutating any state, so an invalid backend +// yields 400 with nothing saved and the prior wiring intact. If Save fails after +// a successful apply, the running adapters are already rewired but the file and +// the in-memory copy are left unchanged; that surfaces as 500. func (s *Server) handlePostSettings(c *gin.Context) { var req settings.Settings if err := c.ShouldBindJSON(&req); err != nil { diff --git a/internal/web/settings_handlers_test.go b/internal/web/settings_handlers_test.go index 3813669..d49ae08 100644 --- a/internal/web/settings_handlers_test.go +++ b/internal/web/settings_handlers_test.go @@ -68,8 +68,12 @@ func TestPostSettingsValidAppliesAndSaves(t *testing.T) { if fa.last.AIBackend != "codex" { t.Errorf("applied backend = %q, want codex", fa.last.AIBackend) } - if _, err := os.Stat(path); err != nil { - t.Errorf("settings file not written: %v", err) + saved, err := settings.Load(path) + if err != nil { + t.Fatalf("settings file not loadable: %v", err) + } + if saved.AIBackend != "codex" || saved.MarvinCmd != "uv run am" || saved.KnowledgePath != "/tmp/k.md" { + t.Errorf("saved settings = %+v, want codex/uv run am//tmp/k.md", saved) } } @@ -87,4 +91,22 @@ func TestPostSettingsInvalidBackendIs400AndNoSave(t *testing.T) { if _, err := os.Stat(path); !os.IsNotExist(err) { t.Errorf("settings file should not exist after rejected save, stat err = %v", err) } + if fa.called != 0 { + t.Errorf("applier recorded %d successful applies, want 0", fa.called) + } +} + +func TestPostSettingsInvalidJSONIs400(t *testing.T) { + s := newTestServer(t) + fa := &fakeApplier{} + s.SetSettings(filepath.Join(t.TempDir(), "settings.json"), settings.Settings{}, fa.apply) + r := s.Router() + + w := post(t, r, "/settings", `{not json`) + if w.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", w.Code) + } + if fa.called != 0 { + t.Errorf("applier called on invalid json; want 0, got %d", fa.called) + } }