fix(tui): move all store writes out of Update into tea.Cmds
CI / test (pull_request) Successful in 2m35s
CI / lint (pull_request) Successful in 56s
CI / vulncheck (pull_request) Successful in 51s

Deletes, pause toggles, maintenance end, theme/collapse prefs, and all
four form submits wrote to the store synchronously on the UI goroutine;
with busy_timeout=5000 a contended DB froze input for up to 5s.

Writes now run through a writeCmd helper returning writeDoneMsg. The
in-memory engine/model mutations stay in Update so rows react
instantly; the reply logs failures and reloads tab data, so the UI
converges on what was actually written. Closures capture snapshotted
values only — never the model.
This commit was merged in pull request #102.
This commit is contained in:
2026-06-11 11:39:15 -04:00
parent 634c3ee03c
commit a3711c652c
8 changed files with 168 additions and 74 deletions
+12 -2
View File
@@ -27,7 +27,9 @@ func loadCollapsed(s store.Store) map[int]bool {
return m return m
} }
func saveCollapsed(s store.Store, collapsed map[int]bool) { // collapsedJSON snapshots the collapsed-group set for persistence. Marshaling
// happens on the UI goroutine so the write Cmd never reads the live map.
func collapsedJSON(collapsed map[int]bool) string {
var ids []int var ids []int
for id, v := range collapsed { for id, v := range collapsed {
if v { if v {
@@ -35,7 +37,15 @@ func saveCollapsed(s store.Store, collapsed map[int]bool) {
} }
} }
data, _ := json.Marshal(ids) data, _ := json.Marshal(ids)
_ = s.SetPreference("collapsed_groups", string(data)) return string(data)
}
// writeCmd runs a store mutation off the UI goroutine. The closure must only
// capture values snapshotted in Update — never the model itself.
func writeCmd(op string, fn func() error) tea.Cmd {
return func() tea.Msg {
return writeDoneMsg{op: op, err: fn()}
}
} }
func sortSitesForDisplay(allSites []models.Site, collapsed map[int]bool) []models.Site { func sortSitesForDisplay(allSites []models.Site, collapsed map[int]bool) []models.Site {
+8
View File
@@ -54,3 +54,11 @@ type slaDataMsg struct {
periodIdx int periodIdx int
changes []models.StateChange changes []models.StateChange
} }
// writeDoneMsg reports a store mutation that ran off the UI goroutine. op
// names the action for the error log; the handler reloads tab data so the UI
// converges on what was actually written.
type writeDoneMsg struct {
op string
err error
}
+12 -10
View File
@@ -445,7 +445,7 @@ func (m *Model) initAlertHuhForm() tea.Cmd {
return m.huhForm.Init() return m.huhForm.Init()
} }
func (m *Model) submitAlertForm() { func (m *Model) submitAlertForm() tea.Cmd {
d := m.alertFormData d := m.alertFormData
settings := make(map[string]string) settings := make(map[string]string)
@@ -486,14 +486,16 @@ func (m *Model) submitAlertForm() {
settings["url"] = d.WebhookURL settings["url"] = d.WebhookURL
} }
if m.editID > 0 { st := m.store
if err := m.store.UpdateAlert(m.editID, d.Name, d.AlertType, settings); err != nil { id := m.editID
m.engine.AddLog("Update alert failed: " + err.Error()) name, aType := d.Name, d.AlertType
}
} else {
if err := m.store.AddAlert(d.Name, d.AlertType, settings); err != nil {
m.engine.AddLog("Add alert failed: " + err.Error())
}
}
m.state = stateDashboard m.state = stateDashboard
if id > 0 {
return writeCmd("Update alert", func() error {
return st.UpdateAlert(id, name, aType, settings)
})
}
return writeCmd("Add alert", func() error {
return st.AddAlert(name, aType, settings)
})
} }
+5 -4
View File
@@ -206,7 +206,7 @@ func (m *Model) initMaintHuhForm() tea.Cmd {
return m.huhForm.Init() return m.huhForm.Init()
} }
func (m *Model) submitMaintForm() { func (m *Model) submitMaintForm() tea.Cmd {
d := m.maintFormData d := m.maintFormData
monitorID, _ := strconv.Atoi(d.MonitorID) monitorID, _ := strconv.Atoi(d.MonitorID)
@@ -237,8 +237,9 @@ func (m *Model) submitMaintForm() {
} }
} }
if err := m.store.AddMaintenanceWindow(mw); err != nil { st := m.store
m.engine.AddLog("Add maintenance window failed: " + err.Error())
}
m.state = stateDashboard m.state = stateDashboard
return writeCmd("Add maintenance window", func() error {
return st.AddMaintenanceWindow(mw)
})
} }
+10 -11
View File
@@ -518,7 +518,7 @@ func (m *Model) initSiteHuhForm() tea.Cmd {
return m.huhForm.Init() return m.huhForm.Init()
} }
func (m *Model) submitSiteForm() { func (m *Model) submitSiteForm() tea.Cmd {
d := m.siteFormData d := m.siteFormData
interval, _ := strconv.Atoi(d.Interval) interval, _ := strconv.Atoi(d.Interval)
alertID, _ := strconv.Atoi(d.AlertID) alertID, _ := strconv.Atoi(d.AlertID)
@@ -555,15 +555,14 @@ func (m *Model) submitSiteForm() {
Regions: d.Regions, Regions: d.Regions,
} }
if m.editID > 0 { st := m.store
if err := m.store.UpdateSite(site); err != nil {
m.engine.AddLog("Update site failed: " + err.Error())
}
m.engine.UpdateSiteConfig(site)
} else {
if err := m.store.AddSite(site); err != nil {
m.engine.AddLog("Add site failed: " + err.Error())
}
}
m.state = stateDashboard m.state = stateDashboard
if m.editID > 0 {
// The engine's in-memory config updates immediately; the DB write
// follows in the Cmd. New sites enter the engine via its poll loop
// once the insert lands.
m.engine.UpdateSiteConfig(site)
return writeCmd("Update site", func() error { return st.UpdateSite(site) })
}
return writeCmd("Add site", func() error { return st.AddSite(site) })
} }
+12 -10
View File
@@ -110,16 +110,18 @@ func (m *Model) initUserHuhForm() tea.Cmd {
return m.huhForm.Init() return m.huhForm.Init()
} }
func (m *Model) submitUserForm() { func (m *Model) submitUserForm() tea.Cmd {
d := m.userFormData d := m.userFormData
if m.editID > 0 { st := m.store
if err := m.store.UpdateUser(m.editID, d.Username, d.PublicKey, d.Role); err != nil { id := m.editID
m.engine.AddLog("Update user failed: " + err.Error()) username, key, role := d.Username, d.PublicKey, d.Role
}
} else {
if err := m.store.AddUser(d.Username, d.PublicKey, d.Role); err != nil {
m.engine.AddLog("Add user failed: " + err.Error())
}
}
m.state = stateUsers m.state = stateUsers
if id > 0 {
return writeCmd("Update user", func() error {
return st.UpdateUser(id, username, key, role)
})
}
return writeCmd("Add user", func() error {
return st.AddUser(username, key, role)
})
} }
+50 -31
View File
@@ -38,6 +38,12 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
return m, nil return m, nil
case slaDataMsg: case slaDataMsg:
return m.handleSLAData(msg) return m.handleSLAData(msg)
case writeDoneMsg:
if msg.err != nil {
m.engine.AddLog(msg.op + " failed: " + msg.err.Error())
}
m.refreshLive()
return m, m.loadTabDataCmd()
} }
if m.state == stateConfirmDelete { if m.state == stateConfirmDelete {
@@ -63,27 +69,26 @@ func (m *Model) handleConfirmDelete(msg tea.Msg) (tea.Model, tea.Cmd) {
} }
switch keyMsg.String() { switch keyMsg.String() {
case "y", "Y": case "y", "Y":
// The store delete runs in a Cmd; the in-memory engine/model updates
// stay here so the row vanishes immediately. If the delete fails, the
// writeDoneMsg reload converges the UI back to the DB state (and the
// engine poll loop re-adds a site that is still in the DB).
st := m.store
id := m.deleteID
var cmd tea.Cmd
switch m.deleteTab { switch m.deleteTab {
case 0: case 0:
if err := m.store.DeleteSite(m.deleteID); err != nil { cmd = writeCmd("Delete site", func() error { return st.DeleteSite(id) })
m.engine.AddLog("Delete site failed: " + err.Error()) m.engine.RemoveSite(id)
}
m.engine.RemoveSite(m.deleteID)
m.adjustCursor(len(m.sites) - 1) m.adjustCursor(len(m.sites) - 1)
case 1: case 1:
if err := m.store.DeleteAlert(m.deleteID); err != nil { cmd = writeCmd("Delete alert", func() error { return st.DeleteAlert(id) })
m.engine.AddLog("Delete alert failed: " + err.Error())
}
m.adjustCursor(len(m.alerts) - 1) m.adjustCursor(len(m.alerts) - 1)
case 4: case 4:
if err := m.store.DeleteMaintenanceWindow(m.deleteID); err != nil { cmd = writeCmd("Delete maintenance window", func() error { return st.DeleteMaintenanceWindow(id) })
m.engine.AddLog("Delete maintenance window failed: " + err.Error())
}
m.adjustCursor(len(m.maintenanceWindows) - 1) m.adjustCursor(len(m.maintenanceWindows) - 1)
case 5: case 5:
if err := m.store.DeleteUser(m.deleteID); err != nil { cmd = writeCmd("Delete user", func() error { return st.DeleteUser(id) })
m.engine.AddLog("Delete user failed: " + err.Error())
}
m.adjustCursor(len(m.users) - 1) m.adjustCursor(len(m.users) - 1)
} }
m.refreshLive() m.refreshLive()
@@ -91,7 +96,7 @@ func (m *Model) handleConfirmDelete(msg tea.Msg) (tea.Model, tea.Cmd) {
if m.deleteTab == 5 { if m.deleteTab == 5 {
m.state = stateUsers m.state = stateUsers
} }
return m, m.loadTabDataCmd() return m, cmd
case "n", "N", "esc": case "n", "N", "esc":
m.state = stateDashboard m.state = stateDashboard
if m.deleteTab == 5 { if m.deleteTab == 5 {
@@ -127,10 +132,12 @@ func (m *Model) handleFormMsg(msg tea.Msg) (tea.Model, tea.Cmd) {
m.huhForm = f m.huhForm = f
} }
if m.huhForm.State == huh.StateCompleted { if m.huhForm.State == huh.StateCompleted {
m.submitForm() // The store write runs in the returned Cmd; its writeDoneMsg
// triggers the tab-data reload once the row actually exists.
cmd := m.submitForm()
m.refreshLive() m.refreshLive()
m.huhForm = nil m.huhForm = nil
return m, m.loadTabDataCmd() return m, cmd
} }
return m, formCmd return m, formCmd
} }
@@ -555,16 +562,22 @@ func (m *Model) handleDashboardKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
if m.currentTab == 0 && len(m.sites) > 0 && m.sites[m.cursor].Type == "group" { if m.currentTab == 0 && len(m.sites) > 0 && m.sites[m.cursor].Type == "group" {
gid := m.sites[m.cursor].ID gid := m.sites[m.cursor].ID
m.collapsed[gid] = !m.collapsed[gid] m.collapsed[gid] = !m.collapsed[gid]
saveCollapsed(m.store, m.collapsed) payload := collapsedJSON(m.collapsed)
st := m.store
m.refreshLive() m.refreshLive()
return m, writeCmd("Save collapsed groups", func() error {
return st.SetPreference("collapsed_groups", payload)
})
} }
case "p": case "p":
if m.currentTab == 0 && len(m.sites) > 0 { if m.currentTab == 0 && len(m.sites) > 0 {
site := m.sites[m.cursor] id := m.sites[m.cursor].ID
m.engine.ToggleSitePause(site.ID) paused := m.engine.ToggleSitePause(id)
site.Paused = !site.Paused st := m.store
_ = m.store.UpdateSitePaused(site.ID, site.Paused)
m.refreshLive() m.refreshLive()
return m, writeCmd("Update pause state", func() error {
return st.UpdateSitePaused(id, paused)
})
} }
case "i": case "i":
if m.currentTab == 0 && len(m.sites) > 0 { if m.currentTab == 0 && len(m.sites) > 0 {
@@ -579,18 +592,23 @@ func (m *Model) handleDashboardKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
now := time.Now() now := time.Now()
isActive := !mw.StartTime.After(now) && (mw.EndTime.IsZero() || mw.EndTime.After(now)) isActive := !mw.StartTime.After(now) && (mw.EndTime.IsZero() || mw.EndTime.After(now))
if isActive { if isActive {
if err := m.store.EndMaintenanceWindow(mw.ID); err != nil { st := m.store
m.engine.AddLog("End maintenance failed: " + err.Error()) id := mw.ID
}
m.refreshLive() m.refreshLive()
return m, m.loadTabDataCmd() return m, writeCmd("End maintenance", func() error {
return st.EndMaintenanceWindow(id)
})
} }
} }
case "T": case "T":
m.themeIndex = (m.themeIndex + 1) % len(themes) m.themeIndex = (m.themeIndex + 1) % len(themes)
m.theme = themes[m.themeIndex] m.theme = themes[m.themeIndex]
m.st = newStyles(m.theme) m.st = newStyles(m.theme)
_ = m.store.SetPreference("theme", m.theme.Name) st := m.store
name := m.theme.Name
return m, writeCmd("Save theme", func() error {
return st.SetPreference("theme", name)
})
case "d", "backspace": case "d", "backspace":
return m.handleDeleteItem() return m.handleDeleteItem()
} }
@@ -738,25 +756,26 @@ func (m *Model) adjustCursor(newLen int) {
} }
} }
func (m *Model) submitForm() { func (m *Model) submitForm() tea.Cmd {
switch m.state { switch m.state {
case stateFormSite: case stateFormSite:
if m.siteFormData != nil { if m.siteFormData != nil {
m.submitSiteForm() return m.submitSiteForm()
} }
case stateFormAlert: case stateFormAlert:
if m.alertFormData != nil { if m.alertFormData != nil {
m.submitAlertForm() return m.submitAlertForm()
} }
case stateFormUser: case stateFormUser:
if m.userFormData != nil { if m.userFormData != nil {
m.submitUserForm() return m.submitUserForm()
} }
case stateFormMaint: case stateFormMaint:
if m.maintFormData != nil { if m.maintFormData != nil {
m.submitMaintForm() return m.submitMaintForm()
} }
} }
return nil
} }
func (m Model) currentListLen() int { func (m Model) currentListLen() int {
+59 -6
View File
@@ -1,6 +1,7 @@
package tui package tui
import ( import (
"strings"
"testing" "testing"
"time" "time"
@@ -19,6 +20,7 @@ type tuiMockStore struct {
maint []models.MaintenanceWindow maint []models.MaintenanceWindow
stateChanges []models.StateChange stateChanges []models.StateChange
stateChangeCalls int // counts GetStateChanges hits (to prove View does no IO) stateChangeCalls int // counts GetStateChanges hits (to prove View does no IO)
deleteSiteCalls int // counts DeleteSite hits (to prove writes run in Cmds)
} }
func (m *tuiMockStore) GetAllAlerts() ([]models.AlertConfig, error) { return m.alerts, nil } func (m *tuiMockStore) GetAllAlerts() ([]models.AlertConfig, error) { return m.alerts, nil }
@@ -32,12 +34,15 @@ func (m *tuiMockStore) GetAllMaintenanceWindows(int) ([]models.MaintenanceWindow
return m.maint, nil return m.maint, nil
} }
func (m *tuiMockStore) Init() error { return nil } func (m *tuiMockStore) Init() error { return nil }
func (m *tuiMockStore) GetSites() ([]models.Site, error) { return nil, nil } func (m *tuiMockStore) GetSites() ([]models.Site, error) { return nil, nil }
func (m *tuiMockStore) AddSite(models.Site) error { return nil } func (m *tuiMockStore) AddSite(models.Site) error { return nil }
func (m *tuiMockStore) UpdateSite(models.Site) error { return nil } func (m *tuiMockStore) UpdateSite(models.Site) error { return nil }
func (m *tuiMockStore) UpdateSitePaused(int, bool) error { return nil } func (m *tuiMockStore) UpdateSitePaused(int, bool) error { return nil }
func (m *tuiMockStore) DeleteSite(int) error { return nil } func (m *tuiMockStore) DeleteSite(int) error {
m.deleteSiteCalls++
return nil
}
func (m *tuiMockStore) GetAlert(int) (models.AlertConfig, error) { return models.AlertConfig{}, nil } func (m *tuiMockStore) GetAlert(int) (models.AlertConfig, error) { return models.AlertConfig{}, nil }
func (m *tuiMockStore) AddAlert(string, string, map[string]string) error { return nil } func (m *tuiMockStore) AddAlert(string, string, map[string]string) error { return nil }
func (m *tuiMockStore) UpdateAlert(int, string, string, map[string]string) error { return nil } func (m *tuiMockStore) UpdateAlert(int, string, string, map[string]string) error { return nil }
@@ -309,6 +314,54 @@ func TestSLAData_DropsStaleReply(t *testing.T) {
} }
} }
func TestConfirmDelete_WritesOffUIGoroutine(t *testing.T) {
ms := &tuiMockStore{}
m := newTestModel(ms)
m.sites = []models.Site{{ID: 4, Name: "s"}}
m.state = stateConfirmDelete
m.deleteTab = 0
m.deleteID = 4
updated, cmd := (&m).handleConfirmDelete(keyMsg("y"))
if ms.deleteSiteCalls != 0 {
t.Fatal("delete hit the store synchronously in Update")
}
if cmd == nil {
t.Fatal("expected a write Cmd")
}
if got := updated.(*Model); got.state != stateDashboard {
t.Fatalf("expected return to dashboard, got state %v", got.state)
}
wd, ok := cmd().(writeDoneMsg)
if !ok || wd.err != nil {
t.Fatalf("unexpected write result: %+v", wd)
}
if ms.deleteSiteCalls != 1 {
t.Fatalf("expected exactly 1 store delete from the Cmd, got %d", ms.deleteSiteCalls)
}
}
func TestWriteDoneMsg_LogsErrorAndReloads(t *testing.T) {
m := newTestModel(&tuiMockStore{})
updated, cmd := m.Update(writeDoneMsg{op: "Delete site", err: errSentinel})
if cmd == nil {
t.Error("writeDoneMsg did not trigger a tab-data reload")
}
mm := updated.(Model)
found := false
for _, line := range mm.engine.GetLogs() {
if strings.Contains(line, "Delete site failed: boom") {
found = true
}
}
if !found {
t.Error("write error was not logged")
}
}
func TestDetailRefreshCmd_OnlyWhileDetailOpen(t *testing.T) { func TestDetailRefreshCmd_OnlyWhileDetailOpen(t *testing.T) {
ms := &tuiMockStore{stateChanges: []models.StateChange{{FromStatus: "UP", ToStatus: "DOWN"}}} ms := &tuiMockStore{stateChanges: []models.StateChange{{FromStatus: "UP", ToStatus: "DOWN"}}}
m := newTestModel(ms) m := newTestModel(ms)