Add unit tests for views.go (#134)

* Fix nil pointer dereference panic when acknowledging incidents

Resolves panic that occurred when trying to acknowledge incidents by
removing unsafe dereferences of m.selectedIncident in message handler
closures. The closures were capturing the incident pointer at creation
time, which could be nil, rather than at execution time when the
incident data is guaranteed to be available.

Changes:
- Modified acknowledge/unacknowledge message handlers to create empty
  message structs instead of dereferencing m.selectedIncident in closures
- Updated Update() function handlers to safely retrieve incident from
  model state when processing messages
- Added nil check in silence incidents handler to prevent panic when
  appending selected incident

The fix ensures that incident data is accessed only after it has been
fetched from PagerDuty and stored in the model, preventing nil pointer
dereferences.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Fix performance issues and re-escalation bug

Multiple performance and functionality improvements:

- Fix re-escalation "Assignee cannot be empty" error by skipping
  unnecessary un-acknowledge step and going directly to re-escalation
  with proper user email authentication
- Add comprehensive terminal escape sequence filtering to prevent fake
  keypresses from clogging the message queue (OSC, CSI, CPR responses)
- Implement priority handling for all user keypresses to ensure
  responsive UI even when async messages are queued
- Optimize auto-acknowledge feature with early return when disabled
  and cached on-call check to reduce API calls from N to 1 per cycle
- Remove triple template rendering - now renders once on explicit request
- Fix viewport consuming ESC and navigation keys with handledKey flag
- Add debug logging to track re-escalation flow
- Clean up unused code: remove unAcknowledgedIncidentsMsg and
  reEscalate parameter from acknowledgeIncidents()

Performance impact:
- ESC/Enter keys now respond immediately instead of waiting for queued messages
- Auto-acknowledge overhead reduced to near-zero when feature is disabled
- Eliminated redundant template renders (3x → 1x per incident view)

Fixes: https://github.com/clcollins/srepd/issues/XXX

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Add unit tests for views.go helper functions

Adds comprehensive unit tests for pure display/formatting functions
in pkg/tui/views.go that were previously untested:

- assigneeArea(): Tests formatting of "You", "Team", and custom assignee displays
- statusArea(): Tests formatting of status messages with various content
- refreshArea(): Tests all combinations of auto-refresh and auto-acknowledge flags
- summarizeNotes(): Tests conversion of PagerDuty notes to display format
- summarizeIncident(): Tests incident conversion including priority, assignments,
  and duplicate acknowledgement deduplication
- addNoteTemplate(): Tests note template generation with incident context

All 6 functions now have test coverage with 16 test cases total, covering:
- Edge cases (empty inputs, nil values)
- Normal operation (single/multiple items)
- Business logic (deduplication, conditional formatting)

Test coverage: pkg/tui/views_test.go
Tests pass: go test ./pkg/tui -v

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Fix golangci-lint errors and add lint to test target

Fixes ineffectual assignment warnings from golangci-lint and ensures
linting runs before tests to catch issues early.

Changes:
- pkg/tui/msgHandlers.go:
  - Remove ineffectual assignments to handledKey in switchIncidentFocusMode()
  - All removed assignments were in cases that return immediately, making
    the assignment unreachable and unnecessary
  - Affected cases: Refresh, Ack, UnAck, Silence, Note, Login, Open
  - handledKey is only checked at the end of the function to determine
    whether to pass the message to the viewport

- Makefile:
  - Add lint as a dependency to the test target
  - Ensures golangci-lint runs before tests to catch linting issues early
  - Updated help text to "Run tests (after linting)"

Testing:
- All golangci-lint checks pass (0 issues)
- All unit tests pass
- `make test` now runs linting automatically before tests

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: clcollins

Makefile

@@ -45,7 +45,7 @@ tidy: ## Tidy up go modules
 	go mod tidy
 
 .PHONY: test
-test: ## Run tests
+test: lint ## Run tests (after linting)
 	@echo "Running tests..."
 	go test ./... -v $(TESTOPTS)
 

pkg/pd/pd.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 
 	"github.com/PagerDuty/go-pagerduty"
+	"github.com/charmbracelet/log"
 )
 
 const (
@@ -115,21 +116,26 @@ func NewListIncidentOptsFromDefaults() pagerduty.ListIncidentsOptions {
 
 }
 
-func AcknowledgeIncident(client PagerDutyClient, incidents []pagerduty.Incident, user *pagerduty.User) ([]pagerduty.Incident, error) {
+func AcknowledgeIncident(client PagerDutyClient, incidents []pagerduty.Incident, user *pagerduty.User, currentUser *pagerduty.User) ([]pagerduty.Incident, error) {
 	var ctx = context.Background()
 	var email string
 
 	opts := []pagerduty.ManageIncidentsOptions{}
 
+	// Use currentUser's email for API call authentication
+	if currentUser != nil {
+		email = currentUser.Email
+	}
+
 	for _, incident := range incidents {
 		if user == nil {
-			email = ""
+			// Un-acknowledge: set escalation level without assignment
 			opts = append(opts, pagerduty.ManageIncidentsOptions{
 				ID:              incident.ID,
 				EscalationLevel: 1,
 			})
 		} else {
-			email = user.Email
+			// Acknowledge: assign to specific user
 			opts = append(opts, pagerduty.ManageIncidentsOptions{
 				ID:     incident.ID,
 				Status: "acknowledged",
@@ -302,8 +308,10 @@ func GetUserOnCalls(client PagerDutyClient, id string, opts pagerduty.ListOnCall
 
 func loopManageIncidents(client PagerDutyClient, ctx context.Context, email string, opts []pagerduty.ManageIncidentsOptions) (incidentList []pagerduty.Incident, err error) {
 	for {
+		log.Debug("pd.loopManageIncidents", "email", email, "opts_count", len(opts))
 		response, err := client.ManageIncidentsWithContext(ctx, email, opts)
 		if err != nil {
+			log.Error("pd.loopManageIncidents", "error", err, "email", email)
 			return incidentList, err
 		}
 
@@ -347,7 +355,7 @@ func ReassignIncidents(client PagerDutyClient, incidents []pagerduty.Incident, u
 }
 
 // ReEscalateIncidents re-escalates a list of incidents to an escalation policy at a specific level
-func ReEscalateIncidents(client PagerDutyClient, incidents []pagerduty.Incident, policy *pagerduty.EscalationPolicy, level uint) ([]pagerduty.Incident, error) {
+func ReEscalateIncidents(client PagerDutyClient, incidents []pagerduty.Incident, user *pagerduty.User, policy *pagerduty.EscalationPolicy, level uint) ([]pagerduty.Incident, error) {
 	var ctx = context.Background()
 
 	opts := []pagerduty.ManageIncidentsOptions{}
@@ -364,7 +372,7 @@ func ReEscalateIncidents(client PagerDutyClient, incidents []pagerduty.Incident,
 		})
 	}
 
-	return loopManageIncidents(client, ctx, "", opts)
+	return loopManageIncidents(client, ctx, user.Email, opts)
 }
 
 func PostNote(client PagerDutyClient, id string, user *pagerduty.User, content string) (*pagerduty.IncidentNote, error) {

pkg/tui/commands.go

@@ -251,7 +251,13 @@ func ShouldBeAcknowledged(p *pd.Config, i pagerduty.Incident, id string, autoAck
 		"userIsOnCall", userIsOnCall,
 		"doIt", doIt,
 	)
-	return AssignedToUser(i, id) && !AcknowledgedByUser(i, id) && autoAcknowledge
+	return doIt
+}
+
+// ShouldBeAcknowledgedCached checks if an incident should be auto-acknowledged using a cached userIsOnCall result.
+// This avoids repeated calls to UserIsOnCall() when checking multiple incidents in the same update cycle.
+func ShouldBeAcknowledgedCached(i pagerduty.Incident, id string, userIsOnCall bool) bool {
+	return AssignedToUser(i, id) && !AcknowledgedByUser(i, id) && userIsOnCall
 }
 
 // AssignedToUser returns true if the incident is assigned to the given user
@@ -523,22 +529,9 @@ type acknowledgedIncidentsMsg struct {
 	incidents []pagerduty.Incident
 	err       error
 }
-type unAcknowledgedIncidentsMsg struct {
-	incidents []pagerduty.Incident
-	err       error
-}
-
-func acknowledgeIncidents(p *pd.Config, incidents []pagerduty.Incident, reEscalate bool) tea.Cmd {
+func acknowledgeIncidents(p *pd.Config, incidents []pagerduty.Incident) tea.Cmd {
 	return func() tea.Msg {
-		var err error
-
-		if reEscalate {
-			a, err := pd.AcknowledgeIncident(p.Client, incidents, &pagerduty.User{})
-			return unAcknowledgedIncidentsMsg{a, err}
-		}
-
-		a, err := pd.AcknowledgeIncident(p.Client, incidents, p.CurrentUser)
-
+		a, err := pd.AcknowledgeIncident(p.Client, incidents, p.CurrentUser, p.CurrentUser)
 		return acknowledgedIncidentsMsg{a, err}
 	}
 }
@@ -573,7 +566,7 @@ type reEscalatedIncidentsMsg []pagerduty.Incident
 
 func reEscalateIncidents(p *pd.Config, i []pagerduty.Incident, e *pagerduty.EscalationPolicy, l uint) tea.Cmd {
 	return func() tea.Msg {
-		r, err := pd.ReEscalateIncidents(p.Client, i, e, l)
+		r, err := pd.ReEscalateIncidents(p.Client, i, p.CurrentUser, e, l)
 		if err != nil {
 			return errMsg{err}
 		}

pkg/tui/commands_test.go

@@ -30,41 +30,21 @@ func TestAcknowledgeIncident(t *testing.T) {
 	}
 
 	tests := []struct {
-		name       string
-		incidents  []pagerduty.Incident
-		reEscalate bool
-		expected   tea.Msg
+		name      string
+		incidents []pagerduty.Incident
+		expected  tea.Msg
 	}{
 		{
-			name:       "return unAcknowledgedIncidentMsg with non-nil error if error occurs while re-escalating",
-			incidents:  errIncidents,
-			reEscalate: true,
-			expected: unAcknowledgedIncidentsMsg{
-				incidents: []pagerduty.Incident(nil),
-				err:       pd.ErrMockError,
-			},
-		},
-		{
-			name:       "return unAcknowledgedIncidentMsg with an incident list if no error occurs while re-escalating",
-			incidents:  incidents,
-			reEscalate: true,
-			expected: unAcknowledgedIncidentsMsg{
-				incidents: incidents,
-			},
-		},
-		{
-			name:       "return acknowledgedIncidentMsg with non-nil error if error occurs while acknowledging",
-			incidents:  errIncidents,
-			reEscalate: false,
+			name:      "return acknowledgedIncidentMsg with non-nil error if error occurs while acknowledging",
+			incidents: errIncidents,
 			expected: acknowledgedIncidentsMsg{
 				incidents: []pagerduty.Incident(nil),
 				err:       pd.ErrMockError,
 			},
 		},
 		{
-			name:       "return acknowledgedIncidentMsg with an incident list if no error occurs while acknowledging",
-			incidents:  incidents,
-			reEscalate: false,
+			name:      "return acknowledgedIncidentMsg with an incident list if no error occurs while acknowledging",
+			incidents: incidents,
 			expected: acknowledgedIncidentsMsg{
 				incidents: incidents,
 			},
@@ -73,7 +53,7 @@ func TestAcknowledgeIncident(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			cmd := acknowledgeIncidents(mockConfig, test.incidents, test.reEscalate)
+			cmd := acknowledgeIncidents(mockConfig, test.incidents)
 			actual := cmd()
 			assert.Equal(t, test.expected, actual)
 		})

pkg/tui/msgHandlers.go

@@ -269,7 +269,7 @@ func switchTableFocusMode(m model, msg tea.Msg) (tea.Model, tea.Cmd) {
 					return waitForSelectedIncidentThenDoMsg{
 						msg: "acknowledge",
 						action: func() tea.Msg {
-							return acknowledgeIncidentsMsg{incidents: []pagerduty.Incident{*m.selectedIncident}}
+							return acknowledgeIncidentsMsg{}
 						},
 					}
 				},
@@ -282,7 +282,7 @@ func switchTableFocusMode(m model, msg tea.Msg) (tea.Model, tea.Cmd) {
 					return waitForSelectedIncidentThenDoMsg{
 						msg: "un-acknowledge",
 						action: func() tea.Msg {
-							return unAcknowledgeIncidentsMsg{incidents: []pagerduty.Incident{*m.selectedIncident}}
+							return unAcknowledgeIncidentsMsg{}
 						},
 					}
 				},
@@ -346,24 +346,31 @@ func switchIncidentFocusMode(m model, msg tea.Msg) (tea.Model, tea.Cmd) {
 	var cmd tea.Cmd
 	var cmds []tea.Cmd
 
+	// Track if we handled the key ourselves
+	handledKey := false
+
 	switch msg := msg.(type) {
 	case tea.KeyMsg:
 		switch {
 		case key.Matches(msg, defaultKeyMap.Help):
 			m.toggleHelp()
+			handledKey = true
 
 		// This un-sets the selected incident and returns to the table view
 		case key.Matches(msg, defaultKeyMap.Back):
 			m.clearSelectedIncident(msg.String() + " (back)")
+			m.table.Focus()  // Ensure table regains focus immediately
+			// Return immediately - no need to process anything else or update viewport
+			return m, nil
 
 		case key.Matches(msg, defaultKeyMap.Refresh):
 			return m, func() tea.Msg { return getIncidentMsg(m.selectedIncident.ID) }
 
 		case key.Matches(msg, defaultKeyMap.Ack):
-			return m, func() tea.Msg { return acknowledgeIncidentsMsg{incidents: []pagerduty.Incident{*m.selectedIncident}} }
+			return m, func() tea.Msg { return acknowledgeIncidentsMsg{} }
 
 		case key.Matches(msg, defaultKeyMap.UnAck):
-			return m, func() tea.Msg { return unAcknowledgeIncidentsMsg{incidents: []pagerduty.Incident{*m.selectedIncident}} }
+			return m, func() tea.Msg { return unAcknowledgeIncidentsMsg{} }
 
 		case key.Matches(msg, defaultKeyMap.Silence):
 			return m, func() tea.Msg { return silenceSelectedIncidentMsg{} }
@@ -395,8 +402,12 @@ func switchIncidentFocusMode(m model, msg tea.Msg) (tea.Model, tea.Cmd) {
 		}
 	}
 
-	m.incidentViewer, cmd = m.incidentViewer.Update(msg)
-	cmds = append(cmds, cmd)
+	// Only pass the message to the viewport if we didn't handle it as a key command
+	// This prevents the viewport from consuming ESC and other navigation keys
+	if !handledKey {
+		m.incidentViewer, cmd = m.incidentViewer.Update(msg)
+		cmds = append(cmds, cmd)
+	}
 
 	return m, tea.Batch(cmds...)
 }