fix(chat): route ctrl+c through cancelChatOperations, not dead handler

cpcloud · cursoragent · cpcloud · commit b02e49e60e2a · 2026-02-13T17:22:39.000-05:00
The root cause of the frozen spinner and missing "Interrupted" notice:
the global ctrl+c handler in model.Update (line 151) intercepts ALL
ctrl+c events before they reach the chat-specific handler in
handleChatKey. The global handler called cancelChatOperations() which
only cancelled the context and set Streaming=false, but never cleaned
up StreamingSQL, channel references, the assistant message, or added
the "Interrupted" notice.

Fix: Move the full cleanup logic into cancelChatOperations:
- Cancel context (if CancelFn exists)
- Clear all streaming state (Streaming, StreamingSQL, channels, CancelFn)
- Remove the "generating query" notice
- Remove the incomplete assistant message
- Add "Interrupted" notice
- Refresh viewport

Also handle the edge case where CancelFn is nil (user presses ctrl+c
between submitChat and handleSQLStreamStarted).

Removed the dead ctrl+c handler in handleChatKey since it was never
reachable.

Co-authored-by: Cursor &lt;cursoragent@cursor.com&gt;
diff --git a/internal/app/chat.go b/internal/app/chat.go
@@ -224,20 +224,47 @@ func (m *Model) hideChat() {
 }
 
 // cancelChatOperations cancels any in-flight LLM streams or model pulls.
+// When the chat is visible, this also cleans up messages and shows an
+// "Interrupted" notice.
 func (m *Model) cancelChatOperations() {
 	if m.chat == nil {
 		return
 	}
-	if m.chat.Streaming && m.chat.CancelFn != nil {
-		m.chat.CancelFn()
+	if m.chat.Streaming {
+		if m.chat.CancelFn != nil {
+			m.chat.CancelFn()
+		}
 		m.chat.Streaming = false
+		m.chat.StreamingSQL = false
+		m.chat.SQLStreamCh = nil
+		m.chat.StreamCh = nil
+		m.chat.CancelFn = nil
+
+		if m.chat.Visible {
+			// Remove the "generating query" notice and incomplete assistant message.
+			m.removeLastNotice()
+			if len(m.chat.Messages) > 0 &&
+				m.chat.Messages[len(m.chat.Messages)-1].Role == roleAssistant {
+				m.chat.Messages = m.chat.Messages[:len(m.chat.Messages)-1]
+			}
+			m.chat.Messages = append(m.chat.Messages, chatMessage{
+				Role: roleNotice, Content: "Interrupted",
+			})
+			m.refreshChatViewport()
+		}
 	}
 	if m.chat.Pulling && m.chat.PullCancel != nil {
 		m.chat.PullCancel()
 		m.chat.Pulling = false
 		m.chat.PullCancel = nil
 		m.chat.PullDisplay = ""
 		m.chat.PullPeak = 0
+		if m.chat.Visible {
+			m.chat.Messages = append(m.chat.Messages, chatMessage{
+				Role: roleNotice, Content: "Pull cancelled",
+			})
+			m.refreshChatViewport()
+		}
 	}
 }
 
@@ -1258,7 +1285,11 @@ func (m *Model) renderChatMessages() string {
 			if msg.Content == "generating query" {
 				continue
 			}
-			rendered = m.styles.ChatNotice.Render(msg.Content)
+			if msg.Content == "Interrupted" || msg.Content == "Pull cancelled" {
+				rendered = m.styles.ChatInterrupted.Render(msg.Content)
+			} else {
+				rendered = m.styles.ChatNotice.Render(msg.Content)
+			}
 		}
 		parts = append(parts, rendered)
 	}
@@ -1341,45 +1372,8 @@ func (m *Model) handleChatKey(key tea.KeyMsg) (tea.Model, tea.Cmd) {
 		m.toggleChatMag()
 		return m, nil
 	case "ctrl+c":
-		// Cancel stream or pull if active.
-		if m.chat.Streaming && m.chat.CancelFn != nil {
-			cancelFn := m.chat.CancelFn
-			m.chat.Streaming = false
-			m.chat.StreamingSQL = false
-			m.chat.SQLStreamCh = nil
-			m.chat.CancelFn = nil
-			// Cancel context - this will close the stream channel and any
-			// pending waitForChunk/waitForSQLChunk will return nil.
-			cancelFn()
-			// Remove all trailing notices and incomplete assistant messages.
-			m.removeLastNotice()
-			// Remove the assistant message that was being streamed.
-			// It doesn't matter if it has partial content or SQL - if we're
-			// cancelling, we don't want to show it.
-			if len(m.chat.Messages) > 0 &&
-				m.chat.Messages[len(m.chat.Messages)-1].Role == roleAssistant {
-				m.chat.Messages = m.chat.Messages[:len(m.chat.Messages)-1]
-			}
-			// Add cancellation notice.
-			m.chat.Messages = append(m.chat.Messages, chatMessage{
-				Role: roleNotice, Content: "Interrupted",
-			})
-			m.refreshChatViewport()
-			return m, nil
-		}
-		if m.chat.Pulling && m.chat.PullCancel != nil {
-			m.chat.PullCancel()
-			m.chat.Pulling = false
-			m.chat.PullCancel = nil
-			m.chat.PullDisplay = ""
-			m.chat.PullPeak = 0
-			m.chat.Messages = append(m.chat.Messages, chatMessage{
-				Role: roleNotice, Content: "Pull cancelled",
-			})
-			m.refreshChatViewport()
-			return m, nil
-		}
-		// No active operations -- ctrl+c does nothing in chat.
+		// Handled by the global ctrl+c handler in model.Update which calls
+		// cancelChatOperations. This case is unreachable but kept for clarity.
 		return m, nil
 	case "up", "ctrl+p":
 		if m.chat.Input.Focused() && !m.chat.Streaming {
diff --git a/internal/app/chat_test.go b/internal/app/chat_test.go
@@ -157,6 +157,8 @@ func TestSQLStreamCancellation(t *testing.T) {
 
 // TestCancellationRemovesAssistantMessage verifies that pressing ctrl+c
 // removes the in-progress assistant message and shows "Interrupted" notice.
+// This tests cancelChatOperations which is the real ctrl+c handler (the global
+// handler in model.Update intercepts ctrl+c before the chat-specific handler).
 func TestCancellationRemovesAssistantMessage(t *testing.T) {
 	m := newTestModel()
 	m.openChat()
@@ -173,31 +175,25 @@ func TestCancellationRemovesAssistantMessage(t *testing.T) {
 		{Role: roleAssistant, Content: "", SQL: "SELECT * FROM"},
 	}
 
-	// Verify assistant message exists
+	// Verify assistant message exists before cancellation
 	assert.Len(t, m.chat.Messages, 3)
 	assert.Equal(t, roleAssistant, m.chat.Messages[2].Role)
 
-	// Simulate ctrl+c by calling the handler logic
-	cancelFn := m.chat.CancelFn
-	m.chat.Streaming = false
-	m.chat.StreamingSQL = false
-	m.chat.SQLStreamCh = nil
-	m.chat.CancelFn = nil
-	cancelFn()
-	m.removeLastNotice()
-	if len(m.chat.Messages) > 0 &&
-		m.chat.Messages[len(m.chat.Messages)-1].Role == roleAssistant {
-		m.chat.Messages = m.chat.Messages[:len(m.chat.Messages)-1]
-	}
-	m.chat.Messages = append(m.chat.Messages, chatMessage{
-		Role: roleNotice, Content: "Interrupted",
-	})
+	// This is what the global ctrl+c handler calls
+	m.cancelChatOperations()
 
 	// Verify assistant message was removed and Interrupted notice added
 	assert.Len(t, m.chat.Messages, 2, "should have user + interrupted notice")
 	assert.Equal(t, roleUser, m.chat.Messages[0].Role)
 	assert.Equal(t, roleNotice, m.chat.Messages[1].Role)
 	assert.Equal(t, "Interrupted", m.chat.Messages[1].Content)
+
+	// Verify all streaming state is cleaned up
+	assert.False(t, m.chat.Streaming)
+	assert.False(t, m.chat.StreamingSQL)
+	assert.Nil(t, m.chat.CancelFn)
+	assert.Nil(t, m.chat.SQLStreamCh)
+	assert.Nil(t, m.chat.StreamCh)
 }
 
 // TestCancellationRemovesAssistantWithPartialContent verifies that ctrl+c
@@ -221,25 +217,47 @@ func TestCancellationRemovesAssistantWithPartialContent(t *testing.T) {
 	assert.Len(t, m.chat.Messages, 2)
 	assert.Equal(t, "Based on the data", m.chat.Messages[1].Content)
 
-	// Simulate ctrl+c
-	cancelFn := m.chat.CancelFn
-	m.chat.Streaming = false
-	m.chat.StreamingSQL = false
-	m.chat.CancelFn = nil
-	cancelFn()
-	m.removeLastNotice()
-	if len(m.chat.Messages) > 0 &&
-		m.chat.Messages[len(m.chat.Messages)-1].Role == roleAssistant {
-		m.chat.Messages = m.chat.Messages[:len(m.chat.Messages)-1]
-	}
-	m.chat.Messages = append(m.chat.Messages, chatMessage{
-		Role: roleNotice, Content: "Interrupted",
-	})
+	// This is what the global ctrl+c handler calls
+	m.cancelChatOperations()
 
 	// Verify partial assistant message was removed
 	assert.Len(t, m.chat.Messages, 2, "should have user + interrupted notice")
 	assert.Equal(t, roleNotice, m.chat.Messages[1].Role)
 	assert.Equal(t, "Interrupted", m.chat.Messages[1].Content)
+
+	// Verify all streaming state is cleaned up
+	assert.False(t, m.chat.Streaming)
+	assert.False(t, m.chat.StreamingSQL)
+	assert.Nil(t, m.chat.CancelFn)
+}
+
+// TestCancellationWorksWithoutCancelFn verifies that ctrl+c still cleans up
+// even when CancelFn is nil (e.g. user presses ctrl+c before the LLM stream
+// has been established).
+func TestCancellationWorksWithoutCancelFn(t *testing.T) {
+	m := newTestModel()
+	m.openChat()
+
+	// Simulate the window between submitChat and handleSQLStreamStarted
+	// where Streaming is true but CancelFn hasn't been set yet
+	m.chat.Streaming = true
+	m.chat.StreamingSQL = true
+	m.chat.CancelFn = nil // not yet set
+	m.chat.Messages = []chatMessage{
+		{Role: roleUser, Content: testQuestion},
+		{Role: roleNotice, Content: "generating query"},
+		{Role: roleAssistant, Content: "", SQL: ""},
+	}
+
+	// This is what the global ctrl+c handler calls
+	m.cancelChatOperations()
+
+	// Should still clean up even without CancelFn
+	assert.Len(t, m.chat.Messages, 2, "should have user + interrupted notice")
+	assert.Equal(t, roleNotice, m.chat.Messages[1].Role)
+	assert.Equal(t, "Interrupted", m.chat.Messages[1].Content)
+	assert.False(t, m.chat.Streaming)
+	assert.False(t, m.chat.StreamingSQL)
 }
 
 // TestSpinnerOnlyShowsForLastMessage verifies that the spinner is only
@@ -290,26 +308,16 @@ func TestNoSpinnerAfterCancellation(t *testing.T) {
 		{Role: roleAssistant, Content: "", SQL: "SELECT"},
 	}
 
-	// Simulate full cancellation flow
-	m.chat.Streaming = false
-	m.chat.StreamingSQL = false
-	m.removeLastNotice()
-	if len(m.chat.Messages) > 0 &&
-		m.chat.Messages[len(m.chat.Messages)-1].Role == roleAssistant {
-		m.chat.Messages = m.chat.Messages[:len(m.chat.Messages)-1]
-	}
-	m.chat.Messages = append(m.chat.Messages, chatMessage{
-		Role: roleNotice, Content: "Interrupted",
-	})
+	// Use the real cancelChatOperations function
+	m.cancelChatOperations()
 
 	// Verify state
 	assert.Len(t, m.chat.Messages, 2)
 	assert.Equal(t, roleNotice, m.chat.Messages[1].Role)
 	assert.False(t, m.chat.Streaming)
 	assert.False(t, m.chat.StreamingSQL)
 
-	// Render should not include any spinner (can't test spinner.View() output
-	// but we verify the conditions for showing spinner are false)
+	// Last message is a notice, not assistant -- no spinner can render
 	lastMsg := m.chat.Messages[len(m.chat.Messages)-1]
 	assert.NotEqual(t, roleAssistant, lastMsg.Role, "last message should not be assistant")
 }
diff --git a/internal/app/styles.go b/internal/app/styles.go
@@ -53,6 +53,7 @@ type Styles struct {
 	ChatUser        lipgloss.Style // chat: user message label
 	ChatAssistant   lipgloss.Style // chat: assistant message label
 	ChatNotice      lipgloss.Style // chat: system notice (model switch, pull progress)
+	ChatInterrupted lipgloss.Style // chat: user-initiated cancellation
 	StatusStyles    map[string]lipgloss.Style
 }
 
@@ -246,6 +247,9 @@ func DefaultStyles() Styles {
 		ChatNotice: lipgloss.NewStyle().
 			Foreground(success).
 			Italic(true),
+		ChatInterrupted: lipgloss.NewStyle().
+			Foreground(secondary).
+			Italic(true),
 		StatusStyles: map[string]lipgloss.Style{
 			"ideating":  lipgloss.NewStyle().Foreground(muted),
 			"planned":   lipgloss.NewStyle().Foreground(accent),
