From d6c0f7508c405d0e8065308ef0835fd76da86871 Mon Sep 17 00:00:00 2001 From: Niklas Ye Date: Wed, 19 Aug 2026 21:22:46 +0200 Subject: [PATCH] Stop a table cursor from getting stuck at -1 Assigning an on-call week panicked with "index out of range [-1]" on an ordinary schedule. The index came from scheduleTable.Cursor(). The cursor is not ours. bubbles' SetRows clamps it down when rows shrink (`if m.cursor > len(rows)-1`) but never back up, so setting zero rows drives it to -1 and filling the table afterwards leaves it there -- -1 is not greater than len-1, so nothing corrects it. Every table in this package is rebuilt from empty exactly once, when the first WindowSizeMsg arrives before any fetch has returned, so every cursor started at -1 and stayed there until the user pressed up or down. Pressing a direction key first is why this was survivable at all. setRows restores the invariant the rest of the package already assumes: a table with rows has a usable cursor. Every rebuild goes through it. Five call sites also bounds-checked only the top of the range, and are now consistent with their siblings, which already had `i < 0 ||`. They were the same latent panic: deleting a schedule entry, deleting a user, editing a topic, opening the API key menu, and the week assignment that actually fired. The regression test deliberately never calls SetCursor. That is what the existing schedule tests do, and SetCursor clamps, which is exactly how this got past them. It drives the real order instead: size, then data, then keys. Also carries a gofmt pass, which is why untouched files appear in the diff. The move to git.ryuvia.com rewrote import paths without re-sorting them, and the new path sorts before github.com/charmbracelet/..., where the old one sorted after. go vet does not look at import order, so CI had nothing to say. --- internal/tui/model.go | 32 ++++++++++++++++++------ internal/tui/model_test.go | 2 +- internal/tui/styles.go | 2 +- internal/tui/update.go | 12 ++++----- internal/tui/update_test.go | 49 ++++++++++++++++++++++++++++++++++++- internal/tui/view.go | 2 +- internal/tui/view_test.go | 2 +- main.go | 2 +- 8 files changed, 84 insertions(+), 19 deletions(-) diff --git a/internal/tui/model.go b/internal/tui/model.go index 4299d73..ffe4e1a 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -4,13 +4,13 @@ import ( "fmt" "time" + "git.ryuvia.com/niklas/terdut-tui/internal/api" "github.com/charmbracelet/bubbles/help" "github.com/charmbracelet/bubbles/table" "github.com/charmbracelet/bubbles/textinput" "github.com/charmbracelet/bubbles/viewport" tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" - "git.ryuvia.com/niklas/terdut-tui/internal/api" ) // ── Enums ────────────────────────────────────────────────────────────────── @@ -354,27 +354,45 @@ func defaultTableStyles() table.Styles { return s } +// setRows replaces a table's rows and keeps its cursor in a state the rest of +// this package can rely on: valid whenever the table has any rows at all. +// +// bubbles does not do that on its own. SetRows only clamps the cursor *down* +// (`if m.cursor > len(rows)-1`), so setting zero rows drives it to -1 and +// nothing ever brings it back — filling the table later leaves -1 in place, +// because -1 is not greater than len-1. Every table here is rebuilt from empty +// once at startup, when the first WindowSizeMsg arrives before any fetch has +// returned, so without this every cursor is -1 until the user happens to press +// up or down. Indexing a slice with that panics, which is exactly what +// assigning an on-call week did. +func setRows(t *table.Model, rows []table.Row) { + t.SetRows(rows) + if len(rows) > 0 && t.Cursor() < 0 { + t.SetCursor(0) + } +} + func (m *Model) rebuildIncidentTable() { m.incidentTable.SetColumns(incidentColumns(m.width)) - m.incidentTable.SetRows(incidentRows(m.incidents)) + setRows(&m.incidentTable, incidentRows(m.incidents)) m.incidentTable.SetHeight(tableHeight(m.height, 8)) } func (m *Model) rebuildTable() { m.alertTable.SetColumns(alertColumns(m.width)) - m.alertTable.SetRows(alertRows(m.alerts)) + setRows(&m.alertTable, alertRows(m.alerts)) m.alertTable.SetHeight(tableHeight(m.height, 8)) } func (m *Model) rebuildArchivedTable() { m.archivedTable.SetColumns(incidentColumns(m.width)) - m.archivedTable.SetRows(incidentRows(m.archivedIncidents)) + setRows(&m.archivedTable, incidentRows(m.archivedIncidents)) m.archivedTable.SetHeight(tableHeight(m.height, 8)) } func (m *Model) rebuildScheduleTable() { m.scheduleTable.SetColumns(scheduleColumns(m.width)) - m.scheduleTable.SetRows(scheduleRows(m.scheduleDays)) + setRows(&m.scheduleTable, scheduleRows(m.scheduleDays)) m.scheduleTable.SetHeight(tableHeight(m.height, 10)) } @@ -384,7 +402,7 @@ func (m *Model) rebuildUserPickerTable() { for i, u := range m.users { rows[i] = table.Row{u.Username, u.Email} } - m.userPickerTable.SetRows(rows) + setRows(&m.userPickerTable, rows) m.userPickerTable.SetHeight(tableHeight(m.height, 10)) } @@ -398,7 +416,7 @@ func (m *Model) rebuildUserManageTable() { } rows[i] = table.Row{u.Username, u.Email, topic, u.CreatedAt.UTC().Format("2006-01-02")} } - m.userManageTable.SetRows(rows) + setRows(&m.userManageTable, rows) m.userManageTable.SetHeight(tableHeight(m.height, 10)) } diff --git a/internal/tui/model_test.go b/internal/tui/model_test.go index 2cec137..8c9cb9a 100644 --- a/internal/tui/model_test.go +++ b/internal/tui/model_test.go @@ -4,8 +4,8 @@ import ( "testing" "time" - "github.com/charmbracelet/bubbles/table" "git.ryuvia.com/niklas/terdut-tui/internal/api" + "github.com/charmbracelet/bubbles/table" ) func TestNextFilter(t *testing.T) { diff --git a/internal/tui/styles.go b/internal/tui/styles.go index f3a1eae..ea52c17 100644 --- a/internal/tui/styles.go +++ b/internal/tui/styles.go @@ -3,8 +3,8 @@ package tui import ( "strings" - "github.com/charmbracelet/lipgloss" "git.ryuvia.com/niklas/terdut-tui/internal/api" + "github.com/charmbracelet/lipgloss" ) var ( diff --git a/internal/tui/update.go b/internal/tui/update.go index d8712a3..1c38aba 100644 --- a/internal/tui/update.go +++ b/internal/tui/update.go @@ -4,10 +4,10 @@ import ( "strconv" "strings" + "git.ryuvia.com/niklas/terdut-tui/internal/api" "github.com/atotto/clipboard" "github.com/charmbracelet/bubbles/viewport" tea "github.com/charmbracelet/bubbletea" - "git.ryuvia.com/niklas/terdut-tui/internal/api" ) func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { @@ -468,7 +468,7 @@ func (m Model) handleDashboardKey(msg tea.KeyMsg) (Model, tea.Cmd) { return m, nil } cursor := m.scheduleTable.Cursor() - if cursor >= len(m.scheduleDays) { + if cursor < 0 || cursor >= len(m.scheduleDays) { return m, nil } day := m.scheduleDays[cursor] @@ -484,7 +484,7 @@ func (m Model) handleDashboardKey(msg tea.KeyMsg) (Model, tea.Cmd) { return m, nil } cursor := m.userManageTable.Cursor() - if cursor >= len(m.users) { + if cursor < 0 || cursor >= len(m.users) { return m, nil } m.selectedUser = m.users[cursor] @@ -510,7 +510,7 @@ func (m Model) handleDashboardKey(msg tea.KeyMsg) (Model, tea.Cmd) { return m, nil } cursor := m.userManageTable.Cursor() - if cursor >= len(m.users) { + if cursor < 0 || cursor >= len(m.users) { return m, nil } m.selectedUser = m.users[cursor] @@ -527,7 +527,7 @@ func (m Model) handleDashboardKey(msg tea.KeyMsg) (Model, tea.Cmd) { return m, nil } cursor := m.userManageTable.Cursor() - if cursor >= len(m.users) { + if cursor < 0 || cursor >= len(m.users) { return m, nil } m.selectedUser = m.users[cursor] @@ -893,7 +893,7 @@ func (m Model) handleUserPickerKey(msg tea.KeyMsg) (Model, tea.Cmd) { } scheduleCursor := m.scheduleTable.Cursor() - if scheduleCursor >= len(m.scheduleDays) { + if scheduleCursor < 0 || scheduleCursor >= len(m.scheduleDays) { m.mode = modeDashboard return m, nil } diff --git a/internal/tui/update_test.go b/internal/tui/update_test.go index acc2e9a..1091aac 100644 --- a/internal/tui/update_test.go +++ b/internal/tui/update_test.go @@ -5,8 +5,8 @@ import ( "testing" "time" - tea "github.com/charmbracelet/bubbletea" "git.ryuvia.com/niklas/terdut-tui/internal/api" + tea "github.com/charmbracelet/bubbletea" ) // press sends one key and returns the resulting model and command. A nil command @@ -676,3 +676,50 @@ func containsAll(s string, subs ...string) bool { } return true } + +// The bug: assigning an on-call week panicked with "index out of range [-1]" +// on a perfectly normal schedule, as long as nobody had moved the cursor first. +// +// The cause is not in this package. bubbles' SetRows clamps the cursor down but +// never up, so the empty rebuild every table gets from the first WindowSizeMsg +// -- which arrives before any fetch returns -- pins the cursor at -1, and +// loading real rows afterwards leaves it there. Pressing up or down hid it, +// which is why every existing test missed it: they all call SetCursor, and +// SetCursor clamps. +// +// So this test must NOT touch the cursor. It reproduces the real order of +// events: size first, data second, keys third. +func TestSchedule_AssignWeekAfterStartupSizingDoesNotPanic(t *testing.T) { + m := NewModel(nil, "http://test", time.Minute) + m.connected = true + m.activeSection = sectionSchedule + m.scheduleWindow = time.Date(2026, 7, 27, 0, 0, 0, 0, time.UTC) + + // 1. Terminal size arrives while every table is still empty. + next, _ := m.Update(tea.WindowSizeMsg{Width: 120, Height: 40}) + m = next.(Model) + + // 2. The schedule and the user list land. + next, _ = m.Update(scheduleFetchedMsg{entries: []api.ScheduleEntry{}}) + m = next.(Model) + next, _ = m.Update(usersFetchedMsg{users: []api.User{ + {ID: 1, Username: "niklas", Email: "n@example.com"}, + }}) + m = next.(Model) + + if got := m.scheduleTable.Cursor(); got < 0 { + t.Fatalf("schedule cursor is %d after loading %d days; a populated table must have a usable cursor", + got, len(m.scheduleDays)) + } + + // 3. Assign the week to the first user, without ever moving a cursor. + m, _ = press(t, m, "W") + if m.mode != modeUserPicker { + t.Fatalf("W did not open the user picker, got mode %v", m.mode) + } + m, _ = press(t, m, "enter") // panicked here + + if m.mode == modeUserPicker { + t.Fatal("enter left the picker open; the assignment never went anywhere") + } +} diff --git a/internal/tui/view.go b/internal/tui/view.go index 378b844..452bc37 100644 --- a/internal/tui/view.go +++ b/internal/tui/view.go @@ -6,8 +6,8 @@ import ( "strings" "time" - "github.com/charmbracelet/lipgloss" "git.ryuvia.com/niklas/terdut-tui/internal/api" + "github.com/charmbracelet/lipgloss" ) // Order must match the section constants — renderTabs indexes this by ordinal. diff --git a/internal/tui/view_test.go b/internal/tui/view_test.go index f2570a0..ff74968 100644 --- a/internal/tui/view_test.go +++ b/internal/tui/view_test.go @@ -6,8 +6,8 @@ import ( "testing" "time" - tea "github.com/charmbracelet/bubbletea" "git.ryuvia.com/niklas/terdut-tui/internal/api" + tea "github.com/charmbracelet/bubbletea" ) // ansi matches the escape sequences lipgloss emits when it decides the output diff --git a/main.go b/main.go index 57e4678..7adab91 100644 --- a/main.go +++ b/main.go @@ -5,11 +5,11 @@ import ( "fmt" "os" - tea "github.com/charmbracelet/bubbletea" "git.ryuvia.com/niklas/terdut-tui/internal/api" "git.ryuvia.com/niklas/terdut-tui/internal/config" "git.ryuvia.com/niklas/terdut-tui/internal/tui" "git.ryuvia.com/niklas/terdut-tui/internal/updater" + tea "github.com/charmbracelet/bubbletea" ) var version = "dev"