From 7cf81dda98e6bc128b84515756effefdabe04111 Mon Sep 17 00:00:00 2001 From: Chris Collins Date: Mon, 9 Jun 2025 12:26:11 -1000 Subject: [PATCH 01/12] Sanitizes log messages by removing instances where an entire pagerduty.Incident or a pointer to one is printed to the logs. Signed-off-by: Chris Collins --- pkg/tui/commands.go | 36 ++++++++++++++++++++++++++++++++---- pkg/tui/tui.go | 22 ++++++++-------------- pkg/tui/views.go | 3 --- 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/pkg/tui/commands.go b/pkg/tui/commands.go index 03813dc..9d52d17 100644 --- a/pkg/tui/commands.go +++ b/pkg/tui/commands.go @@ -607,12 +607,32 @@ var errSilenceIncidentInvalidArgs = errors.New("silenceIncidents: invalid argume func silenceIncidents(incidents []pagerduty.Incident, policy *pagerduty.EscalationPolicy, level uint) tea.Cmd { // SilenceIncidents doesn't have it's own "silencedIncidentsMessage" because it's really just a re-escalation - log.Printf("silence requested for incident(s) %v; reassigning to %s(%s), level %d", incidents, policy.Name, policy.ID, level) - return func() tea.Msg { - if len(incidents) == 0 || policy.Name == "" || level == 0 { + if policy == nil { + log.Debug("silenceIncidents: nil escalation policy provided") + return func() tea.Msg { + return errMsg{errSilenceIncidentInvalidArgs} + } + } + + if len(incidents) == 0 { + log.Debug("silenceIncidents: no incidents provided") + return func() tea.Msg { return errMsg{errSilenceIncidentInvalidArgs} } - // + } + + if policy.Name == "" || policy.ID == "" || level == 0 { + log.Debug("silenceIncidents: invalid arguments", "incident(s) count: %d; policy: %s(%s), level: %d", len(incidents), policy.Name, policy.ID, level) + return func() tea.Msg { + return errMsg{errSilenceIncidentInvalidArgs} + } + } + + incidentIDs := getIDsFromIncidents(incidents) + + log.Printf("silence requested for incident(s) %v; reassigning to %s(%s), level %d", incidentIDs, policy.Name, policy.ID, level) + + return func() tea.Msg { return reEscalateIncidentsMsg{incidents, policy, level} } } @@ -734,3 +754,11 @@ func runScheduledJobs(m *model) []tea.Cmd { } return cmds } + +func getIDsFromIncidents(incidents []pagerduty.Incident) []string { + var ids []string + for _, i := range incidents { + ids = append(ids, i.ID) + } + return ids +} \ No newline at end of file diff --git a/pkg/tui/tui.go b/pkg/tui/tui.go index 62dbd0b..7e7bf78 100644 --- a/pkg/tui/tui.go +++ b/pkg/tui/tui.go @@ -436,28 +436,20 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { ) case acknowledgedIncidentsMsg: - var incidentIDs []string if msg.err != nil { return m, func() tea.Msg { return errMsg{msg.err} } } - for _, i := range msg.incidents { - incidentIDs = append(incidentIDs, i.ID) - } - incidents := strings.Join(incidentIDs, " ") - m.setStatus(fmt.Sprintf("acknowledged incidents: %s", incidents)) + incidentIDs := strings.Join(getIDsFromIncidents(msg.incidents), " ") + m.setStatus(fmt.Sprintf("acknowledged incidents: %s", incidentIDs)) return m, func() tea.Msg { return updateIncidentListMsg("sender: acknowledgedIncidentsMsg") } case unAcknowledgedIncidentsMsg: - var incidentIDs []string if msg.err != nil { return m, func() tea.Msg { return errMsg{msg.err} } } - for _, i := range msg.incidents { - incidentIDs = append(incidentIDs, i.ID) - } - incidents := strings.Join(incidentIDs, " ") - m.setStatus(fmt.Sprintf("re-escalated incidents: %s", incidents)) + incidentIDs := strings.Join(getIDsFromIncidents(msg.incidents), " ") + m.setStatus(fmt.Sprintf("re-escalated incidents: %s", incidentIDs)) return m, func() tea.Msg { return updateIncidentListMsg("sender: unAcknowledgedIncidentsMsg") } @@ -473,7 +465,8 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { ) case reassignedIncidentsMsg: - m.setStatus(fmt.Sprintf("reassigned incidents %v; refreshing Incident List ", msg)) + incidentIDs := getIDsFromIncidents(msg) + m.setStatus(fmt.Sprintf("reassigned incidents %v; refreshing Incident List ", incidentIDs)) return m, func() tea.Msg { return updateIncidentListMsg("sender: reassignedIncidentsMsg") } case reEscalateIncidentsMsg: @@ -488,7 +481,8 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { ) case reEscalatedIncidentsMsg: - m.setStatus(fmt.Sprintf("re-escalated incidents %v; refreshing Incident List ", msg)) + incidentIDs := getIDsFromIncidents(msg) + m.setStatus(fmt.Sprintf("re-escalated incidents %v; refreshing Incident List ", incidentIDs)) return m, func() tea.Msg { return updateIncidentListMsg("sender: reEscalatedIncidentsMsg") } case silenceSelectedIncidentMsg: diff --git a/pkg/tui/views.go b/pkg/tui/views.go index 92387b9..aff8d94 100644 --- a/pkg/tui/views.go +++ b/pkg/tui/views.go @@ -145,12 +145,9 @@ func (m model) View() string { return errorStyle.Render(s.String()) case m.viewingIncident: - log.Debug("viewingIncident") - s.WriteString(m.incidentViewer.View()) default: - s.WriteString(tableContainerStyle.Render(m.table.View())) } From bbf1c75226df9f38ff30b66cb085a30bede6fe76 Mon Sep 17 00:00:00 2001 From: Chris Collins Date: Sun, 9 Nov 2025 12:58:19 -1000 Subject: [PATCH 02/12] Refactor incident view loading to improve responsiveness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplifies the incident loading flow by: - Fetching incident details, alerts, and notes in parallel immediately when incident is selected - Removing wait conditions and sequential loading patterns - Making input handler available in all focus modes - Setting immediate loading state for better UX feedback - Adding direct action handlers in incident view This improves perceived performance and makes the UI more responsive when viewing incident details. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- pkg/tui/commands.go | 12 +++++++++- pkg/tui/msgHandlers.go | 53 +++++++++++++++++++++++++++++------------- pkg/tui/tui.go | 25 ++++++++++++++++---- pkg/tui/views.go | 2 +- 4 files changed, 69 insertions(+), 23 deletions(-) diff --git a/pkg/tui/commands.go b/pkg/tui/commands.go index 9d52d17..9824893 100644 --- a/pkg/tui/commands.go +++ b/pkg/tui/commands.go @@ -83,6 +83,11 @@ type gotIncidentAlertsMsg struct { err error } +// getIncidentAlertsMsg is a message that triggers the fetching of alerts for an incident +type getIncidentAlertsMsg struct { + id string +} + // getIncidentAlerts returns a command that fetches the alerts for the given incident func getIncidentAlerts(p *pd.Config, id string) tea.Cmd { return func() tea.Msg { @@ -100,6 +105,11 @@ type gotIncidentNotesMsg struct { err error } +// getIncidentNotesMsg is a message that triggers the fetching of notes for an incident +type getIncidentNotesMsg struct { + id string +} + // getIncidentNotes returns a command that fetches the notes for the given incident func getIncidentNotes(p *pd.Config, id string) tea.Cmd { return func() tea.Msg { @@ -761,4 +771,4 @@ func getIDsFromIncidents(incidents []pagerduty.Incident) []string { ids = append(ids, i.ID) } return ids -} \ No newline at end of file +} diff --git a/pkg/tui/msgHandlers.go b/pkg/tui/msgHandlers.go index 887386f..1fcd2d2 100644 --- a/pkg/tui/msgHandlers.go +++ b/pkg/tui/msgHandlers.go @@ -122,6 +122,13 @@ func (m model) keyMsgHandler(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil } + // Commands for any focus mode + if key.Matches(msg.(tea.KeyMsg), defaultKeyMap.Input) { + return m, tea.Sequence( + m.input.Focus(), + ) + } + // Default commands for the table view switch { case m.err != nil: @@ -175,11 +182,6 @@ func switchTableFocusMode(m model, msg tea.Msg) (tea.Model, tea.Cmd) { case key.Matches(msg, defaultKeyMap.Bottom): m.table.GotoBottom() - case key.Matches(msg, defaultKeyMap.Input): - return m, tea.Sequence( - m.input.Focus(), - ) - case key.Matches(msg, defaultKeyMap.Team): m.teamMode = !m.teamMode log.Debug("switchTableFocusMode", "teamMode", m.teamMode) @@ -195,12 +197,23 @@ func switchTableFocusMode(m model, msg tea.Msg) (tea.Model, tea.Cmd) { // "waitForSelectedIncidentsThen..." functions are used to wait for the selected incident // to be retrieved from PagerDuty case key.Matches(msg, defaultKeyMap.Enter): + m.selectedIncident = &pagerduty.Incident{ + APIObject: pagerduty.APIObject{ + ID: incidentID, + Summary: "Loading incident details...", + }, + } + m.viewingIncident = true - return m, doIfIncidentSelected(&m, func() tea.Msg { - return waitForSelectedIncidentThenDoMsg{ - action: func() tea.Msg { return renderIncidentMsg("render") }, msg: "render", - } - }) + return m, tea.Sequence( + func() tea.Msg { return renderIncidentMsg("Render incident: " + incidentID) }, + func() tea.Msg { return getIncidentMsg(incidentID) }, + ) + // return m, doIfIncidentSelected(&m, func() tea.Msg { + // return waitForSelectedIncidentThenDoMsg{ + // action: func() tea.Msg { return renderIncidentMsg("render") }, msg: "render", + // } + // }) case key.Matches(msg, defaultKeyMap.Silence): return m, doIfIncidentSelected(&m, tea.Sequence( @@ -319,12 +332,20 @@ func switchIncidentFocusMode(m model, msg tea.Msg) (tea.Model, tea.Cmd) { return m, func() tea.Msg { return getIncidentMsg(m.selectedIncident.ID) } case key.Matches(msg, defaultKeyMap.Login): - return m, tea.Sequence( - func() tea.Msg { return getIncidentMsg(m.selectedIncident.ID) }, - func() tea.Msg { - return waitForSelectedIncidentThenDoMsg{action: func() tea.Msg { return loginMsg("login") }, msg: "wait"} - }, - ) + return m, func() tea.Msg { return loginMsg("login") } + + case key.Matches(msg, defaultKeyMap.Ack): + return m, func() tea.Msg { return acknowledgeIncidentsMsg{incidents: []pagerduty.Incident{*m.selectedIncident}} } + + case key.Matches(msg, defaultKeyMap.UnAck): + return m, func() tea.Msg { return unAcknowledgeIncidentsMsg{incidents: []pagerduty.Incident{*m.selectedIncident}} } + + case key.Matches(msg, defaultKeyMap.Note): + return m, func() tea.Msg { return parseTemplateForNoteMsg("add note") } + + case key.Matches(msg, defaultKeyMap.Open): + return m, func() tea.Msg { return openBrowserMsg("incident") } + } } diff --git a/pkg/tui/tui.go b/pkg/tui/tui.go index 7e7bf78..b2d4855 100644 --- a/pkg/tui/tui.go +++ b/pkg/tui/tui.go @@ -146,7 +146,12 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } m.setStatus(fmt.Sprintf("getting details for incident %v...", msg)) - cmds = append(cmds, getIncident(m.config, string(msg))) + id := string(msg) + cmds = append(cmds, + getIncident(m.config, id), + getIncidentAlerts(m.config, id), + getIncidentNotes(m.config, id), + ) // Set the selected incident to the incident returned from the getIncident command case gotIncidentMsg: @@ -156,10 +161,19 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.setStatus(fmt.Sprintf("got incident %s", msg.incident.ID)) m.selectedIncident = msg.incident - return m, tea.Batch( - getIncidentAlerts(m.config, msg.incident.ID), - getIncidentNotes(m.config, msg.incident.ID), - ) + // return m, tea.Batch( + // getIncidentAlerts(m.config, msg.incident.ID), + // getIncidentNotes(m.config, msg.incident.ID), + // ) + + case getIncidentAlertsMsg: + if msg.id == "" { + return m, func() tea.Msg { + return errMsg{fmt.Errorf(nilIncidentMsg)} + } + } + m.setStatus(fmt.Sprintf("getting alerts for incident %s...", msg.id)) + case gotIncidentNotesMsg: if msg.err != nil { @@ -400,6 +414,7 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case renderIncidentMsg: if m.selectedIncident == nil { m.setStatus("failed render incidents - no incidents provided") + m.viewingIncident = false return m, nil } diff --git a/pkg/tui/views.go b/pkg/tui/views.go index aff8d94..e7c5f03 100644 --- a/pkg/tui/views.go +++ b/pkg/tui/views.go @@ -145,7 +145,7 @@ func (m model) View() string { return errorStyle.Render(s.String()) case m.viewingIncident: - s.WriteString(m.incidentViewer.View()) + s.WriteString(tableContainerStyle.Render(m.incidentViewer.View())) default: s.WriteString(tableContainerStyle.Render(m.table.View())) From a4719a2a3aeef4f1c8117fac629412988dc13ad6 Mon Sep 17 00:00:00 2001 From: Chris Collins Date: Sun, 9 Nov 2025 13:04:23 -1000 Subject: [PATCH 03/12] Fix code quality issues in incident loading refactor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses issues from previous commit: - Remove unused message types (getIncidentAlertsMsg, getIncidentNotesMsg) - Delete commented-out legacy code - Remove incomplete getIncidentAlertsMsg handler that was unreachable - Add re-render trigger when incident data arrives during viewing - Clear placeholder incident on error to prevent stale state These fixes improve code clarity, eliminate dead code, and prevent potential race conditions with partial data rendering. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- pkg/tui/commands.go | 10 ---------- pkg/tui/msgHandlers.go | 5 ----- pkg/tui/tui.go | 16 ++++------------ 3 files changed, 4 insertions(+), 27 deletions(-) diff --git a/pkg/tui/commands.go b/pkg/tui/commands.go index 9824893..538123b 100644 --- a/pkg/tui/commands.go +++ b/pkg/tui/commands.go @@ -83,11 +83,6 @@ type gotIncidentAlertsMsg struct { err error } -// getIncidentAlertsMsg is a message that triggers the fetching of alerts for an incident -type getIncidentAlertsMsg struct { - id string -} - // getIncidentAlerts returns a command that fetches the alerts for the given incident func getIncidentAlerts(p *pd.Config, id string) tea.Cmd { return func() tea.Msg { @@ -105,11 +100,6 @@ type gotIncidentNotesMsg struct { err error } -// getIncidentNotesMsg is a message that triggers the fetching of notes for an incident -type getIncidentNotesMsg struct { - id string -} - // getIncidentNotes returns a command that fetches the notes for the given incident func getIncidentNotes(p *pd.Config, id string) tea.Cmd { return func() tea.Msg { diff --git a/pkg/tui/msgHandlers.go b/pkg/tui/msgHandlers.go index 1fcd2d2..014df70 100644 --- a/pkg/tui/msgHandlers.go +++ b/pkg/tui/msgHandlers.go @@ -209,11 +209,6 @@ func switchTableFocusMode(m model, msg tea.Msg) (tea.Model, tea.Cmd) { func() tea.Msg { return renderIncidentMsg("Render incident: " + incidentID) }, func() tea.Msg { return getIncidentMsg(incidentID) }, ) - // return m, doIfIncidentSelected(&m, func() tea.Msg { - // return waitForSelectedIncidentThenDoMsg{ - // action: func() tea.Msg { return renderIncidentMsg("render") }, msg: "render", - // } - // }) case key.Matches(msg, defaultKeyMap.Silence): return m, doIfIncidentSelected(&m, tea.Sequence( diff --git a/pkg/tui/tui.go b/pkg/tui/tui.go index b2d4855..3bf142b 100644 --- a/pkg/tui/tui.go +++ b/pkg/tui/tui.go @@ -156,24 +156,16 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // Set the selected incident to the incident returned from the getIncident command case gotIncidentMsg: if msg.err != nil { + m.selectedIncident = nil + m.viewingIncident = false return m, func() tea.Msg { return errMsg{msg.err} } } m.setStatus(fmt.Sprintf("got incident %s", msg.incident.ID)) m.selectedIncident = msg.incident - // return m, tea.Batch( - // getIncidentAlerts(m.config, msg.incident.ID), - // getIncidentNotes(m.config, msg.incident.ID), - // ) - - case getIncidentAlertsMsg: - if msg.id == "" { - return m, func() tea.Msg { - return errMsg{fmt.Errorf(nilIncidentMsg)} - } + if m.viewingIncident { + return m, func() tea.Msg { return renderIncidentMsg("refresh") } } - m.setStatus(fmt.Sprintf("getting alerts for incident %s...", msg.id)) - case gotIncidentNotesMsg: if msg.err != nil { From 4ba48804cd67746083a326f2d816f2317eeb2ca9 Mon Sep 17 00:00:00 2001 From: Chris Collins Date: Sun, 9 Nov 2025 13:35:02 -1000 Subject: [PATCH 04/12] Add progressive rendering with loading state tracking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces polling wait loops with reactive loading state tracking: **Loading State Tracking:** - Add incidentDataLoaded, incidentNotesLoaded, incidentAlertsLoaded flags - Set flags when each piece of data arrives - Clear flags when leaving incident view **Progressive Rendering:** - Show incident details immediately when available - Display "Loading notes..." while notes fetch in progress - Display "Loading alerts..." while alerts fetch in progress - Re-render automatically as each piece of data arrives **Action Guards:** - Prevent Note action until full incident data loaded (needs HTMLURL, Title, Service) - Prevent Login action until full incident data loaded (needs Service.Summary) - Prevent Open action until full incident data loaded (needs HTMLURL) - Allow Ack/UnAck immediately (only need ID from placeholder) **Benefits:** - Eliminates 86+ message polling loops that flood event queue - Keeps UI responsive to user input during data loading - Provides clear feedback on loading progress - Prevents nil pointer errors on incomplete data - Maintains fast "show what you have" UX This replaces the waitForSelectedIncident* polling patterns with reactive state management for better performance and reliability. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- pkg/tui/model.go | 9 +++++++++ pkg/tui/msgHandlers.go | 19 +++++++++++++++++++ pkg/tui/tui.go | 3 +++ pkg/tui/views.go | 40 ++++++++++++++++++++++++++++++++++++---- 4 files changed, 67 insertions(+), 4 deletions(-) diff --git a/pkg/tui/model.go b/pkg/tui/model.go index 75b59ea..b6b5274 100644 --- a/pkg/tui/model.go +++ b/pkg/tui/model.go @@ -42,6 +42,11 @@ type model struct { selectedIncidentNotes []pagerduty.IncidentNote selectedIncidentAlerts []pagerduty.IncidentAlert + // Loading state tracking - enables progressive rendering and action guards + incidentDataLoaded bool + incidentNotesLoaded bool + incidentAlertsLoaded bool + scheduledJobs []*scheduledJob autoAcknowledge bool @@ -103,6 +108,10 @@ func (m *model) clearSelectedIncident(reason interface{}) { m.selectedIncidentNotes = nil m.selectedIncidentAlerts = nil m.viewingIncident = false + // Clear loading flags + m.incidentDataLoaded = false + m.incidentNotesLoaded = false + m.incidentAlertsLoaded = false log.Debug("clearSelectedIncident", "selectedIncident", m.selectedIncident, "cleared", true, "reason", reason) } diff --git a/pkg/tui/msgHandlers.go b/pkg/tui/msgHandlers.go index 014df70..78d88eb 100644 --- a/pkg/tui/msgHandlers.go +++ b/pkg/tui/msgHandlers.go @@ -203,6 +203,10 @@ func switchTableFocusMode(m model, msg tea.Msg) (tea.Model, tea.Cmd) { Summary: "Loading incident details...", }, } + // Mark all data as not loaded yet - will be set as each arrives + m.incidentDataLoaded = false + m.incidentNotesLoaded = false + m.incidentAlertsLoaded = false m.viewingIncident = true return m, tea.Sequence( @@ -327,6 +331,11 @@ func switchIncidentFocusMode(m model, msg tea.Msg) (tea.Model, tea.Cmd) { return m, func() tea.Msg { return getIncidentMsg(m.selectedIncident.ID) } case key.Matches(msg, defaultKeyMap.Login): + // Login requires Service.Summary for cluster ID extraction + if !m.incidentDataLoaded { + m.setStatus("Loading incident details, please wait...") + return m, nil + } return m, func() tea.Msg { return loginMsg("login") } case key.Matches(msg, defaultKeyMap.Ack): @@ -336,9 +345,19 @@ func switchIncidentFocusMode(m model, msg tea.Msg) (tea.Model, tea.Cmd) { return m, func() tea.Msg { return unAcknowledgeIncidentsMsg{incidents: []pagerduty.Incident{*m.selectedIncident}} } case key.Matches(msg, defaultKeyMap.Note): + // Note template requires full incident data (HTMLURL, Title, Service.Summary) + if !m.incidentDataLoaded { + m.setStatus("Loading incident details, please wait...") + return m, nil + } return m, func() tea.Msg { return parseTemplateForNoteMsg("add note") } case key.Matches(msg, defaultKeyMap.Open): + // Browser open requires HTMLURL from full incident data + if !m.incidentDataLoaded { + m.setStatus("Loading incident details, please wait...") + return m, nil + } return m, func() tea.Msg { return openBrowserMsg("incident") } } diff --git a/pkg/tui/tui.go b/pkg/tui/tui.go index 3bf142b..19c1b16 100644 --- a/pkg/tui/tui.go +++ b/pkg/tui/tui.go @@ -163,6 +163,7 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.setStatus(fmt.Sprintf("got incident %s", msg.incident.ID)) m.selectedIncident = msg.incident + m.incidentDataLoaded = true if m.viewingIncident { return m, func() tea.Msg { return renderIncidentMsg("refresh") } } @@ -182,6 +183,7 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } m.selectedIncidentNotes = msg.notes + m.incidentNotesLoaded = true if m.viewingIncident { cmds = append(cmds, renderIncident(&m)) } @@ -201,6 +203,7 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } m.selectedIncidentAlerts = msg.alerts + m.incidentAlertsLoaded = true if m.viewingIncident { cmds = append(cmds, renderIncident(&m)) } diff --git a/pkg/tui/views.go b/pkg/tui/views.go index e7c5f03..434c9f2 100644 --- a/pkg/tui/views.go +++ b/pkg/tui/views.go @@ -236,7 +236,30 @@ func (m model) template() (string, error) { } o := new(bytes.Buffer) - summary := summarize(m.selectedIncident, m.selectedIncidentAlerts, m.selectedIncidentNotes) + + // Progressive rendering: show what we have, mark missing parts as loading + var alerts []pagerduty.IncidentAlert + var notes []pagerduty.IncidentNote + + if m.incidentAlertsLoaded { + alerts = m.selectedIncidentAlerts + } else { + // Alerts not loaded yet - will show "Loading alerts..." in template + alerts = nil + } + + if m.incidentNotesLoaded { + notes = m.selectedIncidentNotes + } else { + // Notes not loaded yet - will show "Loading notes..." in template + notes = nil + } + + summary := summarize(m.selectedIncident, alerts, notes) + // Add loading state info to summary for template to use + summary.AlertsLoading = !m.incidentAlertsLoaded + summary.NotesLoading = !m.incidentNotesLoaded + err = template.Execute(o, summary) if err != nil { // TODO: Figure out how to handle this with a proper errMsg @@ -356,6 +379,9 @@ type incidentSummary struct { Alerts []alertSummary Notes []noteSummary Clusters []string + // Progressive rendering flags + AlertsLoading bool + NotesLoading bool } func summarizeIncident(i *pagerduty.Incident) incidentSummary { @@ -430,7 +456,9 @@ Acknowledged by: {{ range $i, $ack := .Acknowledged -}} ## Notes -{{ if .Notes }} +{{ if .NotesLoading }} +_Loading notes..._ +{{ else if .Notes }} {{ range $note := .Notes }} > {{ $note.Content }} -- {{ $note.User }} @ {{ $note.Created }} @@ -439,8 +467,11 @@ Acknowledged by: {{ range $i, $ack := .Acknowledged -}} _none_ {{ end }} -## Alerts ({{ len .Alerts }}) +## Alerts{{ if not .AlertsLoading }} ({{ len .Alerts }}){{ end }} +{{ if .AlertsLoading }} +_Loading alerts..._ +{{ else }} {{ range $alert := .Alerts }} {{ $alert.ID }} - {{ $alert.Status }} {{ if $alert.Name }} @@ -456,9 +487,10 @@ _{{- $alert.Name }}_ Details : {{ range $key, $value := $alert.Details }} -* {{ $key }}: {{ $value }} +* {{ $key }}: {{ $value }} {{ end }} +{{ end }} {{ end }} ` From 3d808983dcd676140dd584cc71fc1f4f4151bede Mon Sep 17 00:00:00 2001 From: Chris Collins Date: Sun, 9 Nov 2025 13:38:46 -1000 Subject: [PATCH 05/12] Add tests for progressive rendering and loading state tracking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Comprehensive test coverage for the new loading state system: **TestLoadingStateTracking:** - Verifies incidentDataLoaded flag set on gotIncidentMsg - Verifies incidentNotesLoaded flag set on gotIncidentNotesMsg - Verifies incidentAlertsLoaded flag set on gotIncidentAlertsMsg - Verifies all flags cleared on clearSelectedIncidentsMsg **TestActionGuards:** - Tests Note action blocked when data not loaded - Tests Login action blocked when data not loaded - Tests Open action blocked when data not loaded - Tests Acknowledge action always allowed (only needs ID) - Verifies appropriate status messages shown when blocked **TestProgressiveRendering:** - Tests "Loading notes..." shown when notes not loaded - Tests "Loading alerts..." shown when alerts not loaded - Tests both loading messages shown when neither loaded - Verifies template rendering handles partial data gracefully These tests ensure the loading state system works correctly and prevents nil pointer errors while maintaining good UX. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- pkg/tui/model_test.go | 256 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 256 insertions(+) create mode 100644 pkg/tui/model_test.go diff --git a/pkg/tui/model_test.go b/pkg/tui/model_test.go new file mode 100644 index 0000000..f07dc34 --- /dev/null +++ b/pkg/tui/model_test.go @@ -0,0 +1,256 @@ +package tui + +import ( + "testing" + + "github.com/PagerDuty/go-pagerduty" + tea "github.com/charmbracelet/bubbletea" + "github.com/stretchr/testify/assert" +) + +func TestLoadingStateTracking(t *testing.T) { + tests := []struct { + name string + msg tea.Msg + initialDataLoaded bool + initialNotesLoaded bool + initialAlertsLoaded bool + expectedDataLoaded bool + expectedNotesLoaded bool + expectedAlertsLoaded bool + setupSelectedIncident bool + }{ + { + name: "gotIncidentMsg sets incidentDataLoaded to true", + msg: gotIncidentMsg{ + incident: &pagerduty.Incident{ + APIObject: pagerduty.APIObject{ID: "Q123"}, + }, + err: nil, + }, + initialDataLoaded: false, + initialNotesLoaded: false, + initialAlertsLoaded: false, + expectedDataLoaded: true, + expectedNotesLoaded: false, + expectedAlertsLoaded: false, + }, + { + name: "gotIncidentNotesMsg sets incidentNotesLoaded to true", + msg: gotIncidentNotesMsg{ + notes: []pagerduty.IncidentNote{ + {ID: "N123", Content: "Test note"}, + }, + err: nil, + }, + initialDataLoaded: false, + initialNotesLoaded: false, + initialAlertsLoaded: false, + expectedDataLoaded: false, + expectedNotesLoaded: true, + expectedAlertsLoaded: false, + }, + { + name: "gotIncidentAlertsMsg sets incidentAlertsLoaded to true", + msg: gotIncidentAlertsMsg{ + alerts: []pagerduty.IncidentAlert{ + {APIObject: pagerduty.APIObject{ID: "A123"}}, + }, + err: nil, + }, + initialDataLoaded: false, + initialNotesLoaded: false, + initialAlertsLoaded: false, + expectedDataLoaded: false, + expectedNotesLoaded: false, + expectedAlertsLoaded: true, + }, + { + name: "clearSelectedIncidentsMsg clears all loading flags", + msg: clearSelectedIncidentsMsg("test"), + initialDataLoaded: true, + initialNotesLoaded: true, + initialAlertsLoaded: true, + expectedDataLoaded: false, + expectedNotesLoaded: false, + expectedAlertsLoaded: false, + setupSelectedIncident: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := model{ + incidentDataLoaded: tt.initialDataLoaded, + incidentNotesLoaded: tt.initialNotesLoaded, + incidentAlertsLoaded: tt.initialAlertsLoaded, + } + + if tt.setupSelectedIncident { + m.selectedIncident = &pagerduty.Incident{ + APIObject: pagerduty.APIObject{ID: "Q123"}, + } + } + + result, _ := m.Update(tt.msg) + m = result.(model) + + assert.Equal(t, tt.expectedDataLoaded, m.incidentDataLoaded, "incidentDataLoaded mismatch") + assert.Equal(t, tt.expectedNotesLoaded, m.incidentNotesLoaded, "incidentNotesLoaded mismatch") + assert.Equal(t, tt.expectedAlertsLoaded, m.incidentAlertsLoaded, "incidentAlertsLoaded mismatch") + }) + } +} + +func TestActionGuards(t *testing.T) { + tests := []struct { + name string + keyMsg tea.KeyMsg + incidentDataLoaded bool + expectedAction bool + expectedStatus string + }{ + { + name: "Note action blocked when data not loaded", + keyMsg: tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'n'}}, + incidentDataLoaded: false, + expectedAction: false, + expectedStatus: "Loading incident details, please wait...", + }, + { + name: "Note action allowed when data loaded", + keyMsg: tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'n'}}, + incidentDataLoaded: true, + expectedAction: true, + expectedStatus: "", + }, + { + name: "Login action blocked when data not loaded", + keyMsg: tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'l'}}, + incidentDataLoaded: false, + expectedAction: false, + expectedStatus: "Loading incident details, please wait...", + }, + { + name: "Login action allowed when data loaded", + keyMsg: tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'l'}}, + incidentDataLoaded: true, + expectedAction: true, + expectedStatus: "", + }, + { + name: "Open action blocked when data not loaded", + keyMsg: tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'o'}}, + incidentDataLoaded: false, + expectedAction: false, + expectedStatus: "Loading incident details, please wait...", + }, + { + name: "Open action allowed when data loaded", + keyMsg: tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'o'}}, + incidentDataLoaded: true, + expectedAction: true, + expectedStatus: "", + }, + { + name: "Acknowledge action always allowed (only needs ID)", + keyMsg: tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'a'}}, + incidentDataLoaded: false, + expectedAction: true, + expectedStatus: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := model{ + viewingIncident: true, + incidentDataLoaded: tt.incidentDataLoaded, + selectedIncident: &pagerduty.Incident{ + APIObject: pagerduty.APIObject{ID: "Q123"}, + }, + } + + initialStatus := m.status + result, cmd := m.Update(tt.keyMsg) + m = result.(model) + + if !tt.expectedAction { + // Action should be blocked - no command returned + assert.Nil(t, cmd, "Expected no command when action is blocked") + assert.Equal(t, tt.expectedStatus, m.status, "Status message mismatch") + } else if tt.keyMsg.Runes[0] != 'a' { // Skip acknowledge since it generates a different message + // Action should be allowed - command returned + assert.NotNil(t, cmd, "Expected command when action is allowed") + // Status should not be the "waiting" message + assert.NotEqual(t, "Loading incident details, please wait...", m.status, "Should not show waiting message") + } + + // Reset status for next iteration + m.status = initialStatus + }) + } +} + +func TestProgressiveRendering(t *testing.T) { + tests := []struct { + name string + incidentNotesLoaded bool + incidentAlertsLoaded bool + expectedNotesContent string + expectedAlertsMarker string + }{ + { + name: "Shows loading message for notes when not loaded", + incidentNotesLoaded: false, + incidentAlertsLoaded: true, + expectedNotesContent: "Loading notes...", + expectedAlertsMarker: "", + }, + { + name: "Shows loading message for alerts when not loaded", + incidentNotesLoaded: true, + incidentAlertsLoaded: false, + expectedNotesContent: "", + expectedAlertsMarker: "Loading alerts...", + }, + { + name: "Shows both loading messages when neither loaded", + incidentNotesLoaded: false, + incidentAlertsLoaded: false, + expectedNotesContent: "Loading notes...", + expectedAlertsMarker: "Loading alerts...", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + incident := &pagerduty.Incident{} + incident.ID = "Q123" + incident.Summary = "Test Incident" + incident.HTMLURL = "https://example.com/incident" + incident.Title = "Test Incident Title" + incident.Status = "triggered" + incident.Urgency = "high" + // Set Service summary for template + incident.Service.Summary = "test-service" + + m := model{ + selectedIncident: incident, + incidentNotesLoaded: tt.incidentNotesLoaded, + incidentAlertsLoaded: tt.incidentAlertsLoaded, + } + + content, err := m.template() + assert.NoError(t, err, "Template should render without error") + + if tt.expectedNotesContent != "" { + assert.Contains(t, content, tt.expectedNotesContent, "Should contain notes loading message") + } + + if tt.expectedAlertsMarker != "" { + assert.Contains(t, content, tt.expectedAlertsMarker, "Should contain alerts loading message") + } + }) + } +} From b9d57f364be25b117a4d4e42b73f516ae254cecf Mon Sep 17 00:00:00 2001 From: Chris Collins Date: Sun, 9 Nov 2025 18:56:03 -1000 Subject: [PATCH 06/12] Add incident data caching and improve logging configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements incident data caching with pre-fetching and automatic cleanup to improve responsiveness when viewing incidents. Cached data is shown immediately while fresh data is fetched in the background. Cache features: - Pre-fetch incident details, notes, and alerts for all incidents in the list - Reuse cached data when opening incidents for instant display - Automatic cache cleanup when incidents are removed from the list - Debug logging for cache operations (new data, refreshes, cleanup) Logging improvements: - Made log destination configurable via log_to_journal config option - Support systemd journal (default), file logging, or stderr fallback - Added unit tests for logging configuration logic - Filter arrow key messages from debug log to reduce noise UI improvements: - Reset viewport to top when opening incident view - Progressive rendering shows cached data immediately 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- cmd/root.go | 72 +++++++++++++++++++++---------- cmd/root_test.go | 91 ++++++++++++++++++++++++++++++++++++++++ pkg/tui/commands.go | 14 ++++--- pkg/tui/commands_test.go | 18 +++++--- pkg/tui/model.go | 15 +++++++ pkg/tui/model_test.go | 3 ++ pkg/tui/msgHandlers.go | 53 +++++++++++++++++++---- pkg/tui/tui.go | 81 ++++++++++++++++++++++++++++++++++- 8 files changed, 302 insertions(+), 45 deletions(-) create mode 100644 cmd/root_test.go diff --git a/cmd/root.go b/cmd/root.go index 3be8661..28c5822 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -194,35 +194,63 @@ func initConfig() { } } -func configureLogging() { - log.SetPrefix("srepd") +// LogDestination represents where logs should be written +type LogDestination int - switch runtime.GOOS { +const ( + LogToJournal LogDestination = iota + LogToFile + LogToStderr +) + +// determineLogDestination returns the appropriate log destination based on OS and config +func determineLogDestination(goos string, logToJournal bool, journalEnabled bool) (LogDestination, string) { + switch goos { case "linux": - // Check if running under systemd - if journal.Enabled() { - log.SetOutput(journalWriter{}) - log.Info("Logging to systemd journal") - return + if logToJournal { + if journalEnabled { + return LogToJournal, "" + } + return LogToFile, "/var/log/srepd.log" } - - // Fallback to /var/log/srepd.log for non-systemd Linux - logFile := "/var/log/srepd.log" - setupFileLogging(logFile) - log.Info("Logging to /var/log/srepd.log") + // User explicitly wants file logging + return LogToFile, "~/.config/srepd/debug.log" case "darwin": - // macOS: Log to ~/Library/Logs/srepd.log - home, err := os.UserHomeDir() - if err != nil { - log.Fatal("Failed to get user home directory:", err) - } - logFile := home + "/Library/Logs/srepd.log" - setupFileLogging(logFile) - log.Info("Logging to ~/Library/Logs/srepd.log") + return LogToFile, "~/Library/Logs/srepd.log" default: - // Default fallback for other OSes + return LogToStderr, "" + } +} + +func configureLogging() { + log.SetPrefix("srepd") + + // Check if user wants to log to journal (default: true) + viper.SetDefault("log_to_journal", true) + logToJournal := viper.GetBool("log_to_journal") + + dest, logPath := determineLogDestination(runtime.GOOS, logToJournal, journal.Enabled()) + + switch dest { + case LogToJournal: + log.SetOutput(journalWriter{}) + log.Info("Logging to systemd journal") + + case LogToFile: + // Expand home directory if needed + if strings.HasPrefix(logPath, "~/") { + home, err := os.UserHomeDir() + if err != nil { + log.Fatal("Failed to get user home directory:", err) + } + logPath = home + logPath[1:] + } + setupFileLogging(logPath) + log.Info("Logging to " + logPath) + + case LogToStderr: log.SetOutput(os.Stderr) log.Warn("Unsupported OS: logging to stderr") } diff --git a/cmd/root_test.go b/cmd/root_test.go new file mode 100644 index 0000000..bd70878 --- /dev/null +++ b/cmd/root_test.go @@ -0,0 +1,91 @@ +package cmd + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDetermineLogDestination(t *testing.T) { + tests := []struct { + name string + goos string + logToJournal bool + journalEnabled bool + expectedDest LogDestination + expectedPath string + }{ + { + name: "Linux with journal enabled and log_to_journal=true", + goos: "linux", + logToJournal: true, + journalEnabled: true, + expectedDest: LogToJournal, + expectedPath: "", + }, + { + name: "Linux with journal disabled and log_to_journal=true", + goos: "linux", + logToJournal: true, + journalEnabled: false, + expectedDest: LogToFile, + expectedPath: "/var/log/srepd.log", + }, + { + name: "Linux with log_to_journal=false (user wants file logging)", + goos: "linux", + logToJournal: false, + journalEnabled: true, // Journal enabled but user wants file + expectedDest: LogToFile, + expectedPath: "~/.config/srepd/debug.log", + }, + { + name: "Linux with log_to_journal=false and journal disabled", + goos: "linux", + logToJournal: false, + journalEnabled: false, + expectedDest: LogToFile, + expectedPath: "~/.config/srepd/debug.log", + }, + { + name: "macOS always logs to file", + goos: "darwin", + logToJournal: true, // Ignored on macOS + journalEnabled: false, + expectedDest: LogToFile, + expectedPath: "~/Library/Logs/srepd.log", + }, + { + name: "macOS with log_to_journal=false", + goos: "darwin", + logToJournal: false, + journalEnabled: false, + expectedDest: LogToFile, + expectedPath: "~/Library/Logs/srepd.log", + }, + { + name: "Unsupported OS logs to stderr", + goos: "windows", + logToJournal: true, + journalEnabled: false, + expectedDest: LogToStderr, + expectedPath: "", + }, + { + name: "Unknown OS logs to stderr", + goos: "freebsd", + logToJournal: false, + journalEnabled: false, + expectedDest: LogToStderr, + expectedPath: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dest, path := determineLogDestination(tt.goos, tt.logToJournal, tt.journalEnabled) + assert.Equal(t, tt.expectedDest, dest, "Log destination mismatch") + assert.Equal(t, tt.expectedPath, path, "Log path mismatch") + }) + } +} diff --git a/pkg/tui/commands.go b/pkg/tui/commands.go index 538123b..d73a7ba 100644 --- a/pkg/tui/commands.go +++ b/pkg/tui/commands.go @@ -79,8 +79,9 @@ func getIncident(p *pd.Config, id string) tea.Cmd { // gotIncidentAlertsMsg is a message that contains the fetched incident alerts type gotIncidentAlertsMsg struct { - alerts []pagerduty.IncidentAlert - err error + incidentID string + alerts []pagerduty.IncidentAlert + err error } // getIncidentAlerts returns a command that fetches the alerts for the given incident @@ -90,14 +91,15 @@ func getIncidentAlerts(p *pd.Config, id string) tea.Cmd { return setStatusMsg{nilIncidentMsg} } a, err := pd.GetAlerts(p.Client, id, pagerduty.ListIncidentAlertsOptions{}) - return gotIncidentAlertsMsg{a, err} + return gotIncidentAlertsMsg{incidentID: id, alerts: a, err: err} } } // gotIncidentNotesMsg is a message that contains the fetched incident notes type gotIncidentNotesMsg struct { - notes []pagerduty.IncidentNote - err error + incidentID string + notes []pagerduty.IncidentNote + err error } // getIncidentNotes returns a command that fetches the notes for the given incident @@ -107,7 +109,7 @@ func getIncidentNotes(p *pd.Config, id string) tea.Cmd { return setStatusMsg{nilIncidentMsg} } n, err := pd.GetNotes(p.Client, id) - return gotIncidentNotesMsg{n, err} + return gotIncidentNotesMsg{incidentID: id, notes: n, err: err} } } diff --git a/pkg/tui/commands_test.go b/pkg/tui/commands_test.go index 17f510a..58a53d5 100644 --- a/pkg/tui/commands_test.go +++ b/pkg/tui/commands_test.go @@ -133,6 +133,7 @@ func TestGetIncidentAlerts(t *testing.T) { mockConfig := &pd.Config{ Client: &pd.MockPagerDutyClient{}, } + testID := rand.ID("Q") tests := []struct { name string config *pd.Config @@ -148,8 +149,9 @@ func TestGetIncidentAlerts(t *testing.T) { { name: "return gotIncidentAlertsMsg if incident id is provided", config: mockConfig, - id: rand.ID("Q"), + id: testID, expected: gotIncidentAlertsMsg{ + incidentID: testID, alerts: []pagerduty.IncidentAlert{ {APIObject: pagerduty.APIObject{ID: "QABCDEFG1234567"}}, {APIObject: pagerduty.APIObject{ID: "QABCDEFG7654321"}}, @@ -162,8 +164,9 @@ func TestGetIncidentAlerts(t *testing.T) { config: mockConfig, id: "err", // "err" signals the mock client to produce a mock error expected: gotIncidentAlertsMsg{ - alerts: nil, - err: fmt.Errorf("pd.GetAlerts(): failed to get alerts for incident `%v`: %v", "err", pd.ErrMockError), + incidentID: "err", + alerts: nil, + err: fmt.Errorf("pd.GetAlerts(): failed to get alerts for incident `%v`: %v", "err", pd.ErrMockError), }, }, } @@ -181,6 +184,7 @@ func TestGetIncidentNotes(t *testing.T) { mockConfig := &pd.Config{ Client: &pd.MockPagerDutyClient{}, } + testID := rand.ID("Q") tests := []struct { name string config *pd.Config @@ -196,8 +200,9 @@ func TestGetIncidentNotes(t *testing.T) { { name: "return gotIncidentNotesMsg if incident id is provided", config: mockConfig, - id: rand.ID("Q"), + id: testID, expected: gotIncidentNotesMsg{ + incidentID: testID, notes: []pagerduty.IncidentNote{ {ID: "QABCDEFG1234567"}, {ID: "QABCDEFG7654321"}, @@ -210,8 +215,9 @@ func TestGetIncidentNotes(t *testing.T) { config: mockConfig, id: "err", // "err" signals the mock client to produce a mock error expected: gotIncidentNotesMsg{ - notes: []pagerduty.IncidentNote{}, - err: fmt.Errorf("pd.GetNotes(): failed to get incident notes `%v`: %v", "err", pd.ErrMockError), + incidentID: "err", + notes: []pagerduty.IncidentNote{}, + err: fmt.Errorf("pd.GetNotes(): failed to get incident notes `%v`: %v", "err", pd.ErrMockError), }, }, } diff --git a/pkg/tui/model.go b/pkg/tui/model.go index b6b5274..70a8676 100644 --- a/pkg/tui/model.go +++ b/pkg/tui/model.go @@ -14,6 +14,17 @@ import ( "github.com/clcollins/srepd/pkg/pd" ) +// cachedIncidentData stores fetched incident data for reuse +type cachedIncidentData struct { + incident *pagerduty.Incident + notes []pagerduty.IncidentNote + alerts []pagerduty.IncidentAlert + dataLoaded bool + notesLoaded bool + alertsLoaded bool + lastFetched time.Time +} + var initialScheduledJobs = []*scheduledJob{ { jobMsg: func() tea.Msg { return PollIncidentsMsg{} }, @@ -47,6 +58,9 @@ type model struct { incidentNotesLoaded bool incidentAlertsLoaded bool + // Incident data cache - stores fetched data for reuse and pre-fetching + incidentCache map[string]*cachedIncidentData + scheduledJobs []*scheduledJob autoAcknowledge bool @@ -76,6 +90,7 @@ func InitialModel( input: newTextInput(), incidentViewer: newIncidentViewer(), status: "", + incidentCache: make(map[string]*cachedIncidentData), scheduledJobs: append([]*scheduledJob{}, initialScheduledJobs...), } diff --git a/pkg/tui/model_test.go b/pkg/tui/model_test.go index f07dc34..a00d41c 100644 --- a/pkg/tui/model_test.go +++ b/pkg/tui/model_test.go @@ -84,6 +84,7 @@ func TestLoadingStateTracking(t *testing.T) { incidentDataLoaded: tt.initialDataLoaded, incidentNotesLoaded: tt.initialNotesLoaded, incidentAlertsLoaded: tt.initialAlertsLoaded, + incidentCache: make(map[string]*cachedIncidentData), } if tt.setupSelectedIncident { @@ -169,6 +170,7 @@ func TestActionGuards(t *testing.T) { selectedIncident: &pagerduty.Incident{ APIObject: pagerduty.APIObject{ID: "Q123"}, }, + incidentCache: make(map[string]*cachedIncidentData), } initialStatus := m.status @@ -239,6 +241,7 @@ func TestProgressiveRendering(t *testing.T) { selectedIncident: incident, incidentNotesLoaded: tt.incidentNotesLoaded, incidentAlertsLoaded: tt.incidentAlertsLoaded, + incidentCache: make(map[string]*cachedIncidentData), } content, err := m.template() diff --git a/pkg/tui/msgHandlers.go b/pkg/tui/msgHandlers.go index 78d88eb..11e8d3a 100644 --- a/pkg/tui/msgHandlers.go +++ b/pkg/tui/msgHandlers.go @@ -197,18 +197,53 @@ func switchTableFocusMode(m model, msg tea.Msg) (tea.Model, tea.Cmd) { // "waitForSelectedIncidentsThen..." functions are used to wait for the selected incident // to be retrieved from PagerDuty case key.Matches(msg, defaultKeyMap.Enter): - m.selectedIncident = &pagerduty.Incident{ - APIObject: pagerduty.APIObject{ - ID: incidentID, - Summary: "Loading incident details...", - }, + // Check if we have cached data for this incident + if cached, exists := m.incidentCache[incidentID]; exists { + // Use cached data immediately + if cached.incident != nil { + m.selectedIncident = cached.incident + m.incidentDataLoaded = cached.dataLoaded + } else { + m.selectedIncident = &pagerduty.Incident{ + APIObject: pagerduty.APIObject{ + ID: incidentID, + Summary: "Loading incident details...", + }, + } + m.incidentDataLoaded = false + } + + if cached.notes != nil { + m.selectedIncidentNotes = cached.notes + m.incidentNotesLoaded = cached.notesLoaded + } else { + m.incidentNotesLoaded = false + } + + if cached.alerts != nil { + m.selectedIncidentAlerts = cached.alerts + m.incidentAlertsLoaded = cached.alertsLoaded + } else { + m.incidentAlertsLoaded = false + } + } else { + // No cache - show loading placeholder + m.selectedIncident = &pagerduty.Incident{ + APIObject: pagerduty.APIObject{ + ID: incidentID, + Summary: "Loading incident details...", + }, + } + m.incidentDataLoaded = false + m.incidentNotesLoaded = false + m.incidentAlertsLoaded = false } - // Mark all data as not loaded yet - will be set as each arrives - m.incidentDataLoaded = false - m.incidentNotesLoaded = false - m.incidentAlertsLoaded = false m.viewingIncident = true + // Reset viewport to top when opening incident + m.incidentViewer.GotoTop() + // Render with whatever data we have (cached or loading placeholder) + // Then refresh from PagerDuty in the background return m, tea.Sequence( func() tea.Msg { return renderIncidentMsg("Render incident: " + incidentID) }, func() tea.Msg { return getIncidentMsg(incidentID) }, diff --git a/pkg/tui/tui.go b/pkg/tui/tui.go index 19c1b16..4679db4 100644 --- a/pkg/tui/tui.go +++ b/pkg/tui/tui.go @@ -106,8 +106,15 @@ func filterMsgContent(msg tea.Msg) tea.Msg { // return m, func() tea.Msg { getIncident(m.config, msg.incident.ID) } func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { msgType := reflect.TypeOf(msg) - // TickMsg are not helpful for logging - if msgType != reflect.TypeOf(TickMsg{}) { + // TickMsg and arrow key messages are not helpful for logging + shouldLog := msgType != reflect.TypeOf(TickMsg{}) + if keyMsg, ok := msg.(tea.KeyMsg); ok && shouldLog { + // Skip logging for arrow keys used in scrolling + if keyMsg.Type == tea.KeyUp || keyMsg.Type == tea.KeyDown { + shouldLog = false + } + } + if shouldLog { log.Debug("Update", msgType, filterMsgContent(msg)) } @@ -164,6 +171,22 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.setStatus(fmt.Sprintf("got incident %s", msg.incident.ID)) m.selectedIncident = msg.incident m.incidentDataLoaded = true + + // Update cache with fetched incident data + if cached, exists := m.incidentCache[msg.incident.ID]; exists { + log.Debug("Update", "gotIncidentMsg", "refreshing cached incident data", "incident", msg.incident.ID) + cached.incident = msg.incident + cached.dataLoaded = true + cached.lastFetched = time.Now() + } else { + log.Debug("Update", "gotIncidentMsg", "caching new incident data", "incident", msg.incident.ID) + m.incidentCache[msg.incident.ID] = &cachedIncidentData{ + incident: msg.incident, + dataLoaded: true, + lastFetched: time.Now(), + } + } + if m.viewingIncident { return m, func() tea.Msg { return renderIncidentMsg("refresh") } } @@ -184,6 +207,20 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.selectedIncidentNotes = msg.notes m.incidentNotesLoaded = true + + // Update cache with fetched notes + if cached, exists := m.incidentCache[msg.incidentID]; exists { + log.Debug("Update", "gotIncidentNotesMsg", "refreshing cached notes", "incident", msg.incidentID, "count", len(msg.notes)) + cached.notes = msg.notes + cached.notesLoaded = true + } else { + log.Debug("Update", "gotIncidentNotesMsg", "caching new notes", "incident", msg.incidentID, "count", len(msg.notes)) + m.incidentCache[msg.incidentID] = &cachedIncidentData{ + notes: msg.notes, + notesLoaded: true, + } + } + if m.viewingIncident { cmds = append(cmds, renderIncident(&m)) } @@ -204,6 +241,20 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.selectedIncidentAlerts = msg.alerts m.incidentAlertsLoaded = true + + // Update cache with fetched alerts + if cached, exists := m.incidentCache[msg.incidentID]; exists { + log.Debug("Update", "gotIncidentAlertsMsg", "refreshing cached alerts", "incident", msg.incidentID, "count", len(msg.alerts)) + cached.alerts = msg.alerts + cached.alertsLoaded = true + } else { + log.Debug("Update", "gotIncidentAlertsMsg", "caching new alerts", "incident", msg.incidentID, "count", len(msg.alerts)) + m.incidentCache[msg.incidentID] = &cachedIncidentData{ + alerts: msg.alerts, + alertsLoaded: true, + } + } + if m.viewingIncident { cmds = append(cmds, renderIncident(&m)) } @@ -254,6 +305,19 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // Overwrite m.incidentList with current incidents m.incidentList = msg.incidents + // Pre-fetch incident data for all incidents in the list + for _, i := range m.incidentList { + // Check if incident is already cached + if _, exists := m.incidentCache[i.ID]; !exists { + // Not cached - pre-fetch all data in the background + cmds = append(cmds, + getIncident(m.config, i.ID), + getIncidentAlerts(m.config, i.ID), + getIncidentNotes(m.config, i.ID), + ) + } + } + // Check if any incidents should be auto-acknowledged; // This must be done before adding the stale incidents for _, i := range m.incidentList { @@ -269,6 +333,19 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // Add stale incidents to the list m.incidentList = append(m.incidentList, staleIncidentList...) + // Clean up cache - remove entries for incidents no longer in the list + // (including STALE incidents, but excluding those that have aged out) + incidentIDs := make(map[string]bool) + for _, i := range m.incidentList { + incidentIDs[i.ID] = true + } + for id := range m.incidentCache { + if !incidentIDs[id] { + delete(m.incidentCache, id) + log.Debug("Update", "updatedIncidentListMsg", "removing cached data for incident no longer in list", "incident", id) + } + } + var totalIncidentCount int var rows []table.Row From d6f1994c607ec2778ff617e0cdec22c39eea50aa Mon Sep 17 00:00:00 2001 From: Chris Collins Date: Sun, 9 Nov 2025 18:56:59 -1000 Subject: [PATCH 07/12] Enable auto-refresh by default on startup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changes the default behavior so the application automatically watches for incident updates on startup. Users can pause auto-refresh by pressing ctrl+r (toggle). Previous behavior: Started paused, user enables watching New behavior: Starts watching, user can pause if desired This provides a better on-call experience by keeping the incident list current without requiring manual intervention. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- pkg/tui/model.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/tui/model.go b/pkg/tui/model.go index 70a8676..adbc411 100644 --- a/pkg/tui/model.go +++ b/pkg/tui/model.go @@ -92,6 +92,7 @@ func InitialModel( status: "", incidentCache: make(map[string]*cachedIncidentData), scheduledJobs: append([]*scheduledJob{}, initialScheduledJobs...), + autoRefresh: true, // Start watching for updates on startup } // This is an ugly way to handle this error From 9fd5df32f817e931e2f65f8d8feb6c9bc454caef Mon Sep 17 00:00:00 2001 From: Chris Collins Date: Sun, 9 Nov 2025 19:08:56 -1000 Subject: [PATCH 08/12] Fix incident view key commands and browser error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Incident view improvements: - Removed duplicate acknowledge handler - Fixed action guards to check correct loading state: - Login now checks incidentAlertsLoaded (needs cluster_id from alerts) - Open now checks incidentDataLoaded (needs HTMLURL) - Note checks incidentDataLoaded (needs incident metadata) - All key commands now work from incident view Browser command improvements: - Changed browser opening to fire-and-forget behavior - Removed stderr checking that treated browser warnings as errors - Browser sandbox warnings are now ignored (normal browser behavior) - Added test coverage for browser command All commands working in incident view: - a: Acknowledge - ctrl+e: Re-escalate - ctrl+s: Silence - n: Add note (requires data loaded) - l: Login to cluster (requires alerts loaded) - o: Open in browser (requires data loaded) - r: Refresh incident - h: Help - esc: Back to table 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- pkg/tui/commands.go | 23 ++------------------- pkg/tui/commands_test.go | 43 ++++++++++++++++++++++++++++++++++++++++ pkg/tui/model_test.go | 41 ++++++++++++++++++++------------------ pkg/tui/msgHandlers.go | 25 ++++++++++------------- 4 files changed, 78 insertions(+), 54 deletions(-) diff --git a/pkg/tui/commands.go b/pkg/tui/commands.go index d73a7ba..fa00f88 100644 --- a/pkg/tui/commands.go +++ b/pkg/tui/commands.go @@ -329,13 +329,6 @@ func openBrowserCmd(browser []string, url string) tea.Cmd { c := exec.Command(browser[0], args...) log.Debug(fmt.Sprintf("tui.openBrowserCmd(): %v", c.String())) - stderr, pipeErr := c.StderrPipe() - if pipeErr != nil { - log.Debug(fmt.Sprintf("tui.openBrowserCmd(): %v", pipeErr.Error())) - return func() tea.Msg { - return browserFinishedMsg{err: pipeErr} - } - } err := c.Start() if err != nil { @@ -345,20 +338,8 @@ func openBrowserCmd(browser []string, url string) tea.Cmd { } } - out, err := io.ReadAll(stderr) - if err != nil { - log.Debug(fmt.Sprintf("tui.openBrowserCmd(): %v", err.Error())) - return func() tea.Msg { - return browserFinishedMsg{err} - } - } - - if len(out) > 0 { - log.Debug(fmt.Sprintf("tui.openBrowserCmd(): error: %s", out)) - return func() tea.Msg { - return browserFinishedMsg{fmt.Errorf("%s", out)} - } - } + // Browser opened successfully - don't wait for it or check stderr + // Browsers write warnings/info to stderr that aren't actual errors return func() tea.Msg { return browserFinishedMsg{} diff --git a/pkg/tui/commands_test.go b/pkg/tui/commands_test.go index 58a53d5..5acdff2 100644 --- a/pkg/tui/commands_test.go +++ b/pkg/tui/commands_test.go @@ -363,3 +363,46 @@ func TestGetEscalationPolicyKey(t *testing.T) { }) } } + +func TestOpenBrowserCmd(t *testing.T) { + tests := []struct { + name string + browser []string + url string + expectError bool + errorContains string + }{ + { + name: "successful command execution", + browser: []string{"echo", "browser"}, + url: "https://example.com", + expectError: false, + }, + { + name: "command not found returns error", + browser: []string{"nonexistent-browser-command-xyz"}, + url: "https://example.com", + expectError: true, + errorContains: "not found", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cmd := openBrowserCmd(tt.browser, tt.url) + result := cmd() + + msg, ok := result.(browserFinishedMsg) + assert.True(t, ok, "Expected browserFinishedMsg type") + + if tt.expectError { + assert.NotNil(t, msg.err, "Expected error but got nil") + if tt.errorContains != "" { + assert.Contains(t, msg.err.Error(), tt.errorContains, "Error message mismatch") + } + } else { + assert.Nil(t, msg.err, "Expected no error but got: %v", msg.err) + } + }) + } +} diff --git a/pkg/tui/model_test.go b/pkg/tui/model_test.go index a00d41c..04258eb 100644 --- a/pkg/tui/model_test.go +++ b/pkg/tui/model_test.go @@ -105,11 +105,12 @@ func TestLoadingStateTracking(t *testing.T) { func TestActionGuards(t *testing.T) { tests := []struct { - name string - keyMsg tea.KeyMsg - incidentDataLoaded bool - expectedAction bool - expectedStatus string + name string + keyMsg tea.KeyMsg + incidentDataLoaded bool + incidentAlertsLoaded bool + expectedAction bool + expectedStatus string }{ { name: "Note action blocked when data not loaded", @@ -126,18 +127,18 @@ func TestActionGuards(t *testing.T) { expectedStatus: "", }, { - name: "Login action blocked when data not loaded", - keyMsg: tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'l'}}, - incidentDataLoaded: false, - expectedAction: false, - expectedStatus: "Loading incident details, please wait...", + name: "Login action blocked when alerts not loaded", + keyMsg: tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'l'}}, + incidentAlertsLoaded: false, + expectedAction: false, + expectedStatus: "Loading incident alerts, please wait...", }, { - name: "Login action allowed when data loaded", - keyMsg: tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'l'}}, - incidentDataLoaded: true, - expectedAction: true, - expectedStatus: "", + name: "Login action allowed when alerts loaded", + keyMsg: tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'l'}}, + incidentAlertsLoaded: true, + expectedAction: true, + expectedStatus: "", }, { name: "Open action blocked when data not loaded", @@ -165,8 +166,9 @@ func TestActionGuards(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { m := model{ - viewingIncident: true, - incidentDataLoaded: tt.incidentDataLoaded, + viewingIncident: true, + incidentDataLoaded: tt.incidentDataLoaded, + incidentAlertsLoaded: tt.incidentAlertsLoaded, selectedIncident: &pagerduty.Incident{ APIObject: pagerduty.APIObject{ID: "Q123"}, }, @@ -184,8 +186,9 @@ func TestActionGuards(t *testing.T) { } else if tt.keyMsg.Runes[0] != 'a' { // Skip acknowledge since it generates a different message // Action should be allowed - command returned assert.NotNil(t, cmd, "Expected command when action is allowed") - // Status should not be the "waiting" message - assert.NotEqual(t, "Loading incident details, please wait...", m.status, "Should not show waiting message") + // Status should not be the "waiting" messages + assert.NotEqual(t, "Loading incident details, please wait...", m.status, "Should not show data waiting message") + assert.NotEqual(t, "Loading incident alerts, please wait...", m.status, "Should not show alerts waiting message") } // Reset status for next iteration diff --git a/pkg/tui/msgHandlers.go b/pkg/tui/msgHandlers.go index 11e8d3a..90a49fc 100644 --- a/pkg/tui/msgHandlers.go +++ b/pkg/tui/msgHandlers.go @@ -356,29 +356,18 @@ func switchIncidentFocusMode(m model, msg tea.Msg) (tea.Model, tea.Cmd) { case key.Matches(msg, defaultKeyMap.Back): m.clearSelectedIncident(msg.String() + " (back)") - case key.Matches(msg, defaultKeyMap.Ack): - return m, func() tea.Msg { return acknowledgeIncidentsMsg{incidents: []pagerduty.Incident{*m.selectedIncident}} } - - case key.Matches(msg, defaultKeyMap.Silence): - return m, func() tea.Msg { return silenceSelectedIncidentMsg{} } - case key.Matches(msg, defaultKeyMap.Refresh): return m, func() tea.Msg { return getIncidentMsg(m.selectedIncident.ID) } - case key.Matches(msg, defaultKeyMap.Login): - // Login requires Service.Summary for cluster ID extraction - if !m.incidentDataLoaded { - m.setStatus("Loading incident details, please wait...") - return m, nil - } - return m, func() tea.Msg { return loginMsg("login") } - case key.Matches(msg, defaultKeyMap.Ack): return m, func() tea.Msg { return acknowledgeIncidentsMsg{incidents: []pagerduty.Incident{*m.selectedIncident}} } case key.Matches(msg, defaultKeyMap.UnAck): return m, func() tea.Msg { return unAcknowledgeIncidentsMsg{incidents: []pagerduty.Incident{*m.selectedIncident}} } + case key.Matches(msg, defaultKeyMap.Silence): + return m, func() tea.Msg { return silenceSelectedIncidentMsg{} } + case key.Matches(msg, defaultKeyMap.Note): // Note template requires full incident data (HTMLURL, Title, Service.Summary) if !m.incidentDataLoaded { @@ -387,6 +376,14 @@ func switchIncidentFocusMode(m model, msg tea.Msg) (tea.Model, tea.Cmd) { } return m, func() tea.Msg { return parseTemplateForNoteMsg("add note") } + case key.Matches(msg, defaultKeyMap.Login): + // Login requires alerts to extract cluster_id + if !m.incidentAlertsLoaded { + m.setStatus("Loading incident alerts, please wait...") + return m, nil + } + return m, func() tea.Msg { return loginMsg("login") } + case key.Matches(msg, defaultKeyMap.Open): // Browser open requires HTMLURL from full incident data if !m.incidentDataLoaded { From fcef78a50de6ab3852616cef5fc3b5b7ecab57cb Mon Sep 17 00:00:00 2001 From: Chris Collins Date: Tue, 11 Nov 2025 07:48:34 -1000 Subject: [PATCH 09/12] Fix UI sluggishness by isolating pre-fetch from selected incident MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-fetching was causing UI responsiveness issues by: - Overwriting m.selectedIncident* fields with background fetches - Setting loading flags globally instead of per-incident - Triggering unnecessary re-renders (expensive template parsing) Changes: - Modified gotIncidentMsg/gotIncidentNotesMsg/gotIncidentAlertsMsg to only update selected incident fields if msg is for current incident - Cache updates still happen for all incidents (background pre-fetch) - Only trigger re-render if viewing AND this is the selected incident - Updated tests to match new behavior Result: Pre-fetching now happens silently in background without affecting currently viewed incident or causing sluggish navigation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- pkg/tui/model_test.go | 41 +++++++++++++----------- pkg/tui/tui.go | 73 +++++++++++++++++++++++-------------------- 2 files changed, 62 insertions(+), 52 deletions(-) diff --git a/pkg/tui/model_test.go b/pkg/tui/model_test.go index 04258eb..59f4ff9 100644 --- a/pkg/tui/model_test.go +++ b/pkg/tui/model_test.go @@ -28,42 +28,47 @@ func TestLoadingStateTracking(t *testing.T) { }, err: nil, }, - initialDataLoaded: false, - initialNotesLoaded: false, - initialAlertsLoaded: false, - expectedDataLoaded: true, - expectedNotesLoaded: false, - expectedAlertsLoaded: false, + initialDataLoaded: false, + initialNotesLoaded: false, + initialAlertsLoaded: false, + expectedDataLoaded: true, + expectedNotesLoaded: false, + expectedAlertsLoaded: false, + setupSelectedIncident: true, }, { name: "gotIncidentNotesMsg sets incidentNotesLoaded to true", msg: gotIncidentNotesMsg{ + incidentID: "Q123", notes: []pagerduty.IncidentNote{ {ID: "N123", Content: "Test note"}, }, err: nil, }, - initialDataLoaded: false, - initialNotesLoaded: false, - initialAlertsLoaded: false, - expectedDataLoaded: false, - expectedNotesLoaded: true, - expectedAlertsLoaded: false, + initialDataLoaded: false, + initialNotesLoaded: false, + initialAlertsLoaded: false, + expectedDataLoaded: false, + expectedNotesLoaded: true, + expectedAlertsLoaded: false, + setupSelectedIncident: true, }, { name: "gotIncidentAlertsMsg sets incidentAlertsLoaded to true", msg: gotIncidentAlertsMsg{ + incidentID: "Q123", alerts: []pagerduty.IncidentAlert{ {APIObject: pagerduty.APIObject{ID: "A123"}}, }, err: nil, }, - initialDataLoaded: false, - initialNotesLoaded: false, - initialAlertsLoaded: false, - expectedDataLoaded: false, - expectedNotesLoaded: false, - expectedAlertsLoaded: true, + initialDataLoaded: false, + initialNotesLoaded: false, + initialAlertsLoaded: false, + expectedDataLoaded: false, + expectedNotesLoaded: false, + expectedAlertsLoaded: true, + setupSelectedIncident: true, }, { name: "clearSelectedIncidentsMsg clears all loading flags", diff --git a/pkg/tui/tui.go b/pkg/tui/tui.go index 4679db4..ed6377d 100644 --- a/pkg/tui/tui.go +++ b/pkg/tui/tui.go @@ -168,10 +168,6 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, func() tea.Msg { return errMsg{msg.err} } } - m.setStatus(fmt.Sprintf("got incident %s", msg.incident.ID)) - m.selectedIncident = msg.incident - m.incidentDataLoaded = true - // Update cache with fetched incident data if cached, exists := m.incidentCache[msg.incident.ID]; exists { log.Debug("Update", "gotIncidentMsg", "refreshing cached incident data", "incident", msg.incident.ID) @@ -187,8 +183,15 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } } - if m.viewingIncident { - return m, func() tea.Msg { return renderIncidentMsg("refresh") } + // Only update selected incident if this is the one we're viewing + if m.selectedIncident != nil && msg.incident.ID == m.selectedIncident.ID { + m.setStatus(fmt.Sprintf("got incident %s", msg.incident.ID)) + m.selectedIncident = msg.incident + m.incidentDataLoaded = true + + if m.viewingIncident { + return m, func() tea.Msg { return renderIncidentMsg("refresh") } + } } case gotIncidentNotesMsg: @@ -196,18 +199,6 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, func() tea.Msg { return errMsg{msg.err} } } - // CANNOT refer to the m.SelectedIncident, because it may not have - // completed yet, and will be nil - switch { - case len(msg.notes) == 1: - m.setStatus(fmt.Sprintf("got %d note for incident", len(msg.notes))) - case len(msg.notes) > 1: - m.setStatus(fmt.Sprintf("got %d notes for incident", len(msg.notes))) - } - - m.selectedIncidentNotes = msg.notes - m.incidentNotesLoaded = true - // Update cache with fetched notes if cached, exists := m.incidentCache[msg.incidentID]; exists { log.Debug("Update", "gotIncidentNotesMsg", "refreshing cached notes", "incident", msg.incidentID, "count", len(msg.notes)) @@ -221,8 +212,21 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } } - if m.viewingIncident { - cmds = append(cmds, renderIncident(&m)) + // Only update selected incident notes if this is the incident we're viewing + if m.selectedIncident != nil && msg.incidentID == m.selectedIncident.ID { + switch { + case len(msg.notes) == 1: + m.setStatus(fmt.Sprintf("got %d note for incident", len(msg.notes))) + case len(msg.notes) > 1: + m.setStatus(fmt.Sprintf("got %d notes for incident", len(msg.notes))) + } + + m.selectedIncidentNotes = msg.notes + m.incidentNotesLoaded = true + + if m.viewingIncident { + cmds = append(cmds, renderIncident(&m)) + } } case gotIncidentAlertsMsg: @@ -230,18 +234,6 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, func() tea.Msg { return errMsg{msg.err} } } - // CANNOT refer to the m.SelectedIncident, because it may not have - // completed yet, and will be nil - switch { - case len(msg.alerts) == 1: - m.setStatus(fmt.Sprintf("got %d alert for incident", len(msg.alerts))) - case len(msg.alerts) > 1: - m.setStatus(fmt.Sprintf("got %d alerts for incident", len(msg.alerts))) - } - - m.selectedIncidentAlerts = msg.alerts - m.incidentAlertsLoaded = true - // Update cache with fetched alerts if cached, exists := m.incidentCache[msg.incidentID]; exists { log.Debug("Update", "gotIncidentAlertsMsg", "refreshing cached alerts", "incident", msg.incidentID, "count", len(msg.alerts)) @@ -255,8 +247,21 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } } - if m.viewingIncident { - cmds = append(cmds, renderIncident(&m)) + // Only update selected incident alerts if this is the incident we're viewing + if m.selectedIncident != nil && msg.incidentID == m.selectedIncident.ID { + switch { + case len(msg.alerts) == 1: + m.setStatus(fmt.Sprintf("got %d alert for incident", len(msg.alerts))) + case len(msg.alerts) > 1: + m.setStatus(fmt.Sprintf("got %d alerts for incident", len(msg.alerts))) + } + + m.selectedIncidentAlerts = msg.alerts + m.incidentAlertsLoaded = true + + if m.viewingIncident { + cmds = append(cmds, renderIncident(&m)) + } } case updateIncidentListMsg: From 4a8d5a5153b9eb08e4ee6d2b8fbd28bc3af805af Mon Sep 17 00:00:00 2001 From: Chris Collins Date: Tue, 11 Nov 2025 08:03:33 -1000 Subject: [PATCH 10/12] Fix infinite loop when triggering actions from table view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous fix for UI responsiveness broke action handlers that depend on waitForSelectedIncidentThenDoMsg. The handlers only updated m.selectedIncident if it was already non-nil AND matched, which meant when initiating actions from table view (where selectedIncident starts as nil), it would never get set, causing infinite re-queuing. Changes: - Modified gotIncidentMsg/gotIncidentNotesMsg/gotIncidentAlertsMsg to set selectedIncident if it's nil OR if it matches current selection - Only skip updating if we have a DIFFERENT incident selected - This allows initial selection while still preventing background pre-fetch from overwriting the current selection Result: Actions like add note, login, and open from table view now work again without getting stuck in "waiting for incident info..." loop. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- pkg/tui/tui.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/pkg/tui/tui.go b/pkg/tui/tui.go index ed6377d..84b26bf 100644 --- a/pkg/tui/tui.go +++ b/pkg/tui/tui.go @@ -183,8 +183,9 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } } - // Only update selected incident if this is the one we're viewing - if m.selectedIncident != nil && msg.incident.ID == m.selectedIncident.ID { + // Only update selected incident if no incident is selected or this matches the selected one + // Skip if we're viewing a different incident (don't let background pre-fetch overwrite it) + if m.selectedIncident == nil || msg.incident.ID == m.selectedIncident.ID { m.setStatus(fmt.Sprintf("got incident %s", msg.incident.ID)) m.selectedIncident = msg.incident m.incidentDataLoaded = true @@ -212,8 +213,9 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } } - // Only update selected incident notes if this is the incident we're viewing - if m.selectedIncident != nil && msg.incidentID == m.selectedIncident.ID { + // Only update selected incident notes if no incident is selected or this matches the selected one + // Skip if we're viewing a different incident (don't let background pre-fetch overwrite it) + if m.selectedIncident == nil || msg.incidentID == m.selectedIncident.ID { switch { case len(msg.notes) == 1: m.setStatus(fmt.Sprintf("got %d note for incident", len(msg.notes))) @@ -247,8 +249,9 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } } - // Only update selected incident alerts if this is the incident we're viewing - if m.selectedIncident != nil && msg.incidentID == m.selectedIncident.ID { + // Only update selected incident alerts if no incident is selected or this matches the selected one + // Skip if we're viewing a different incident (don't let background pre-fetch overwrite it) + if m.selectedIncident == nil || msg.incidentID == m.selectedIncident.ID { switch { case len(msg.alerts) == 1: m.setStatus(fmt.Sprintf("got %d alert for incident", len(msg.alerts))) From 5bf8f90ea815abee444bc200616652b1b5971835 Mon Sep 17 00:00:00 2001 From: Chris Collins Date: Tue, 11 Nov 2025 08:08:00 -1000 Subject: [PATCH 11/12] Fix un-acknowledge (Ctrl-E) to actually re-escalate incidents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The un-acknowledge key command (Ctrl-E) was only un-acknowledging incidents without re-escalating them to their escalation policies. This meant the current on-call person would not be paged. Problem: - unAcknowledgedIncidentsMsg handler only updated status and refreshed - No re-escalation was happening despite status message saying "re-escalated incidents" Changes: - After un-acknowledging, group incidents by escalation policy - Create reEscalateIncidentsMsg for each policy group - Each incident is re-escalated to its own service's escalation policy at level 2 (reEscalateDefaultPolicyLevel) - Use tea.Batch to send all re-escalation messages Result: Ctrl-E now properly un-acknowledges AND re-escalates incidents to page the current on-call person for each service. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- pkg/tui/tui.go | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/pkg/tui/tui.go b/pkg/tui/tui.go index 84b26bf..d97a664 100644 --- a/pkg/tui/tui.go +++ b/pkg/tui/tui.go @@ -544,7 +544,34 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, func() tea.Msg { return errMsg{msg.err} } } incidentIDs := strings.Join(getIDsFromIncidents(msg.incidents), " ") - m.setStatus(fmt.Sprintf("re-escalated incidents: %s", incidentIDs)) + m.setStatus(fmt.Sprintf("un-acknowledged incidents: %s", incidentIDs)) + + // After un-acknowledging, re-escalate each incident to its escalation policy to page current on-call + // Group incidents by escalation policy + policyGroups := make(map[string][]pagerduty.Incident) + for _, incident := range msg.incidents { + policyKey := getEscalationPolicyKey(incident.Service.ID, m.config.EscalationPolicies) + policyGroups[policyKey] = append(policyGroups[policyKey], incident) + } + + // Create re-escalate messages for each policy group + var cmds []tea.Cmd + for policyKey, incidents := range policyGroups { + policy := m.config.EscalationPolicies[policyKey] + if policy != nil && policy.ID != "" { + cmds = append(cmds, func() tea.Msg { + return reEscalateIncidentsMsg{ + incidents: incidents, + policy: policy, + level: reEscalateDefaultPolicyLevel, + } + }) + } + } + + if len(cmds) > 0 { + return m, tea.Batch(cmds...) + } return m, func() tea.Msg { return updateIncidentListMsg("sender: unAcknowledgedIncidentsMsg") } From 70f453d45dfd0fa6d92608628ae112fe09386b9b Mon Sep 17 00:00:00 2001 From: Chris Collins Date: Tue, 11 Nov 2025 08:12:13 -1000 Subject: [PATCH 12/12] Fix browser error handling to show error message and error view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Browser open errors were failing silently because the openBrowserMsg handler was incorrectly wrapping the openBrowserCmd result, preventing browserFinishedMsg from ever being received by Update. Problem: - openBrowserMsg handler wrapped openBrowserCmd in another function: return m, func() tea.Msg { return openBrowserCmd(...) } - This prevented browserFinishedMsg from being sent to Update - Errors were logged but never displayed to user - No error view was triggered (unlike login errors) Changes: - Changed openBrowserMsg handler to directly return tea.Cmd: return m, openBrowserCmd(...) - Matches the pattern used by loginMsg handler - Added status message in browserFinishedMsg error handler - Error now triggers error view via errMsg Result: Browser open failures now: 1. Display "failed to open browser: " in status bar 2. Send errMsg to trigger error view 3. Match the behavior of login command errors 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- pkg/tui/tui.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/tui/tui.go b/pkg/tui/tui.go index d97a664..b49460a 100644 --- a/pkg/tui/tui.go +++ b/pkg/tui/tui.go @@ -460,15 +460,15 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, func() tea.Msg { return errMsg{fmt.Errorf("unsupported OS: no browser open command available")} } } c := []string{defaultBrowserOpenCommand} - return m, func() tea.Msg { return openBrowserCmd(c, m.selectedIncident.HTMLURL) } + return m, openBrowserCmd(c, m.selectedIncident.HTMLURL) case browserFinishedMsg: if msg.err != nil { + m.setStatus(fmt.Sprintf("failed to open browser: %s", msg.err)) return m, func() tea.Msg { return errMsg{msg.err} } - } else { - m.setStatus(fmt.Sprintf("opened incident %s in browser - check browser window", m.selectedIncident.ID)) - return m, nil } + m.setStatus(fmt.Sprintf("opened incident %s in browser - check browser window", m.selectedIncident.ID)) + return m, nil // This is a catch all for any action that requires a selected incident case waitForSelectedIncidentThenDoMsg: