Merge pull request #57 from bnema/fix/bell-pulse-agent-sessions
fix(daemon): show active agent bell pulse
5 files · +91 -14
M internal/usecase/daemon/attention.go +11 -7
| @@ -9,8 +9,9 @@ import ( | |||
| 9 | 9 | "github.com/bnema/vev/internal/usecase/picker" | |
| 10 | 10 | ) | |
| 11 | 11 | ||
| 12 | - | // noteAttention records attention for tabs that are not currently visible to an | |
| 13 | - | // attached client. The caller must not hold tab.mu. | |
| 12 | + | // noteAttention records attention for a tab. If the tab is currently visible, | |
| 13 | + | // it is kept long enough to render one pulse before paint acknowledges it. The | |
| 14 | + | // caller must not hold tab.mu. | |
| 14 | 15 | func (d *Daemon) noteAttention(sess *session, tb *tab) { | |
| 15 | 16 | if sess == nil || tb == nil { | |
| 16 | 17 | return | |
| @@ -23,14 +24,13 @@ func (d *Daemon) noteAttention(sess *session, tb *tab) { | |||
| 23 | 24 | ||
| 24 | 25 | sess.mu.Lock() | |
| 25 | 26 | visible := sess.client != nil && sess.active >= 0 && sess.active < len(sess.tabs) && sess.tabs[sess.active] == tb | |
| 26 | - | if visible { | |
| 27 | - | sess.mu.Unlock() | |
| 28 | - | return | |
| 29 | - | } | |
| 30 | 27 | if !tb.attention { | |
| 31 | 28 | tb.attentionAt = now | |
| 32 | 29 | } | |
| 33 | 30 | tb.attention = true | |
| 31 | + | if visible { | |
| 32 | + | tb.attentionVisiblePaint = true | |
| 33 | + | } | |
| 34 | 34 | sess.mu.Unlock() | |
| 35 | 35 | ||
| 36 | 36 | // Do not repaint here: this runs on the PTY reader goroutine, and paint | |
| @@ -135,7 +135,7 @@ func (d *Daemon) repaintAllAttachedClients() { | |||
| 135 | 135 | } | |
| 136 | 136 | } | |
| 137 | 137 | ||
| 138 | - | func (s *session) ackAttention(tb *tab) bool { | |
| 138 | + | func (s *session) ackAttention(tb *tab, visible bool) bool { | |
| 139 | 139 | if tb == nil { | |
| 140 | 140 | return false | |
| 141 | 141 | } | |
| @@ -144,8 +144,12 @@ func (s *session) ackAttention(tb *tab) bool { | |||
| 144 | 144 | if s.active < 0 || s.active >= len(s.tabs) || s.tabs[s.active] != tb || !tb.attention { | |
| 145 | 145 | return false | |
| 146 | 146 | } | |
| 147 | + | if tb.attentionVisiblePaint && !visible { | |
| 148 | + | return false | |
| 149 | + | } | |
| 147 | 150 | tb.attention = false | |
| 148 | 151 | tb.attentionAt = time.Time{} | |
| 152 | + | tb.attentionVisiblePaint = false | |
| 149 | 153 | return true | |
| 150 | 154 | } | |
| 151 | 155 | ||
| @@ -9,8 +9,9 @@ import ( | |||||
| 9 | "github.com/bnema/vev/internal/usecase/picker" | 9 | "github.com/bnema/vev/internal/usecase/picker" | ||
| 10 | ) | 10 | ) | ||
| 11 | 11 | ||||
| 12 | - | // noteAttention records attention for tabs that are not currently visible to an | 12 | + | // noteAttention records attention for a tab. If the tab is currently visible, |
| 13 | - | // attached client. The caller must not hold tab.mu. | 13 | + | // it is kept long enough to render one pulse before paint acknowledges it. The |
| 14 | + | // caller must not hold tab.mu. | |||
| 14 | func (d *Daemon) noteAttention(sess *session, tb *tab) { | 15 | func (d *Daemon) noteAttention(sess *session, tb *tab) { | ||
| 15 | if sess == nil || tb == nil { | 16 | if sess == nil || tb == nil { | ||
| 16 | return | 17 | return | ||
| @@ -23,14 +24,13 @@ func (d *Daemon) noteAttention(sess *session, tb *tab) { | |||||
| 23 | 24 | ||||
| 24 | sess.mu.Lock() | 25 | sess.mu.Lock() | ||
| 25 | visible := sess.client != nil && sess.active >= 0 && sess.active < len(sess.tabs) && sess.tabs[sess.active] == tb | 26 | visible := sess.client != nil && sess.active >= 0 && sess.active < len(sess.tabs) && sess.tabs[sess.active] == tb | ||
| 26 | - | if visible { | |||
| 27 | - | sess.mu.Unlock() | |||
| 28 | - | return | |||
| 29 | - | } | |||
| 30 | if !tb.attention { | 27 | if !tb.attention { | ||
| 31 | tb.attentionAt = now | 28 | tb.attentionAt = now | ||
| 32 | } | 29 | } | ||
| 33 | tb.attention = true | 30 | tb.attention = true | ||
| 31 | + | if visible { | |||
| 32 | + | tb.attentionVisiblePaint = true | |||
| 33 | + | } | |||
| 34 | sess.mu.Unlock() | 34 | sess.mu.Unlock() | ||
| 35 | 35 | ||||
| 36 | // Do not repaint here: this runs on the PTY reader goroutine, and paint | 36 | // Do not repaint here: this runs on the PTY reader goroutine, and paint | ||
| @@ -135,7 +135,7 @@ func (d *Daemon) repaintAllAttachedClients() { | |||||
| 135 | } | 135 | } | ||
| 136 | } | 136 | } | ||
| 137 | 137 | ||||
| 138 | - | func (s *session) ackAttention(tb *tab) bool { | 138 | + | func (s *session) ackAttention(tb *tab, visible bool) bool { |
| 139 | if tb == nil { | 139 | if tb == nil { | ||
| 140 | return false | 140 | return false | ||
| 141 | } | 141 | } | ||
| @@ -144,8 +144,12 @@ func (s *session) ackAttention(tb *tab) bool { | |||||
| 144 | if s.active < 0 || s.active >= len(s.tabs) || s.tabs[s.active] != tb || !tb.attention { | 144 | if s.active < 0 || s.active >= len(s.tabs) || s.tabs[s.active] != tb || !tb.attention { | ||
| 145 | return false | 145 | return false | ||
| 146 | } | 146 | } | ||
| 147 | + | if tb.attentionVisiblePaint && !visible { | |||
| 148 | + | return false | |||
| 149 | + | } | |||
| 147 | tb.attention = false | 150 | tb.attention = false | ||
| 148 | tb.attentionAt = time.Time{} | 151 | tb.attentionAt = time.Time{} | ||
| 152 | + | tb.attentionVisiblePaint = false | |||
| 149 | return true | 153 | return true | ||
| 150 | } | 154 | } | ||
| 151 | 155 | ||||
M internal/usecase/daemon/attention_test.go +71 -2
| @@ -32,7 +32,7 @@ func TestNoteAttentionFlagsBackgroundTab(t *testing.T) { | |||
| 32 | 32 | require.False(t, sess.tabs[0].attention) | |
| 33 | 33 | } | |
| 34 | 34 | ||
| 35 | - | func TestNoteAttentionIgnoresVisibleTab(t *testing.T) { | |
| 35 | + | func TestNoteAttentionFlagsVisibleTabUntilPainted(t *testing.T) { | |
| 36 | 36 | d, sess, _, _, releases := newManualTabSession(t, 1) | |
| 37 | 37 | defer releases[0]() | |
| 38 | 38 | sess.active = 0 | |
| @@ -41,8 +41,77 @@ func TestNoteAttentionIgnoresVisibleTab(t *testing.T) { | |||
| 41 | 41 | ||
| 42 | 42 | sess.mu.Lock() | |
| 43 | 43 | defer sess.mu.Unlock() | |
| 44 | + | require.True(t, sess.tabs[0].attention) | |
| 45 | + | require.False(t, sess.tabs[0].attentionAt.IsZero()) | |
| 46 | + | } | |
| 47 | + | ||
| 48 | + | func TestPaintDoesNotAckActiveAttentionOnBlankPulseFrame(t *testing.T) { | |
| 49 | + | d, sess, ac, sends, releases := newManualTabSession(t, 1) | |
| 50 | + | defer releases[0]() | |
| 51 | + | sess.active = 0 | |
| 52 | + | d.setAttentionFrame(0) | |
| 53 | + | d.noteAttention(sess, sess.tabs[0]) | |
| 54 | + | ||
| 55 | + | d.paint(sess, ac, true) | |
| 56 | + | data := mustOutputData(t, sends) | |
| 57 | + | require.NotContains(t, string(data), string(attentionGlyph)) | |
| 58 | + | ||
| 59 | + | sess.mu.Lock() | |
| 60 | + | require.True(t, sess.tabs[0].attention) | |
| 61 | + | sess.mu.Unlock() | |
| 62 | + | ||
| 63 | + | d.setAttentionFrame(1) | |
| 64 | + | d.paint(sess, ac, true) | |
| 65 | + | data = mustOutputData(t, sends) | |
| 66 | + | require.Contains(t, string(data), string(attentionGlyph)) | |
| 67 | + | ||
| 68 | + | sess.mu.Lock() | |
| 44 | 69 | require.False(t, sess.tabs[0].attention) | |
| 45 | - | require.True(t, sess.tabs[0].attentionAt.IsZero()) | |
| 70 | + | sess.mu.Unlock() | |
| 71 | + | } | |
| 72 | + | ||
| 73 | + | func TestPTYReaderActiveVisibleAgentNotificationRendersBellBeforeAck(t *testing.T) { | |
| 74 | + | tests := []struct { | |
| 75 | + | name string | |
| 76 | + | data []byte | |
| 77 | + | }{ | |
| 78 | + | {name: "bell", data: []byte("\a")}, | |
| 79 | + | {name: "osc notify", data: []byte("\x1b]777;notify;Claude;needs input\x1b\\")}, | |
| 80 | + | {name: "osc progress complete", data: []byte("\x1b]9;4;1;50\a\x1b]9;4;0;100\a")}, | |
| 81 | + | } | |
| 82 | + | ||
| 83 | + | for _, tt := range tests { | |
| 84 | + | t.Run(tt.name, func(t *testing.T) { | |
| 85 | + | pty := newScriptPTY([][]byte{tt.data}) | |
| 86 | + | d, sess, ac, sends := newManualSessionWithPTYs(t, pty) | |
| 87 | + | sess.active = 0 | |
| 88 | + | d.setAttentionFrame(1) | |
| 89 | + | ||
| 90 | + | d.sessWg.Add(1) | |
| 91 | + | go d.ptyReader(sess, sess.tabs[0], sess.tabs[0].focusedPane()) | |
| 92 | + | ||
| 93 | + | require.Eventually(t, func() bool { | |
| 94 | + | sess.mu.Lock() | |
| 95 | + | defer sess.mu.Unlock() | |
| 96 | + | return sess.tabs[0].attention && !sess.tabs[0].attentionAt.IsZero() | |
| 97 | + | }, 2*time.Second, 5*time.Millisecond) | |
| 98 | + | ||
| 99 | + | d.paint(sess, ac, true) | |
| 100 | + | data := mustOutputData(t, sends) | |
| 101 | + | require.Contains(t, string(data), string(attentionGlyph)) | |
| 102 | + | ||
| 103 | + | sess.mu.Lock() | |
| 104 | + | require.False(t, sess.tabs[0].attention) | |
| 105 | + | require.True(t, sess.tabs[0].attentionAt.IsZero()) | |
| 106 | + | sess.mu.Unlock() | |
| 107 | + | ||
| 108 | + | cleared := mustOutputData(t, sends) | |
| 109 | + | require.NotContains(t, string(cleared), string(attentionGlyph)) | |
| 110 | + | ||
| 111 | + | _ = pty.Close() | |
| 112 | + | d.sessWg.Wait() | |
| 113 | + | }) | |
| 114 | + | } | |
| 46 | 115 | } | |
| 47 | 116 | ||
| 48 | 117 | func TestNoteAttentionFlagsDetachedActiveTab(t *testing.T) { | |
| @@ -32,7 +32,7 @@ func TestNoteAttentionFlagsBackgroundTab(t *testing.T) { | |||||
| 32 | require.False(t, sess.tabs[0].attention) | 32 | require.False(t, sess.tabs[0].attention) | ||
| 33 | } | 33 | } | ||
| 34 | 34 | ||||
| 35 | - | func TestNoteAttentionIgnoresVisibleTab(t *testing.T) { | 35 | + | func TestNoteAttentionFlagsVisibleTabUntilPainted(t *testing.T) { |
| 36 | d, sess, _, _, releases := newManualTabSession(t, 1) | 36 | d, sess, _, _, releases := newManualTabSession(t, 1) | ||
| 37 | defer releases[0]() | 37 | defer releases[0]() | ||
| 38 | sess.active = 0 | 38 | sess.active = 0 | ||
| @@ -41,8 +41,77 @@ func TestNoteAttentionIgnoresVisibleTab(t *testing.T) { | |||||
| 41 | 41 | ||||
| 42 | sess.mu.Lock() | 42 | sess.mu.Lock() | ||
| 43 | defer sess.mu.Unlock() | 43 | defer sess.mu.Unlock() | ||
| 44 | + | require.True(t, sess.tabs[0].attention) | |||
| 45 | + | require.False(t, sess.tabs[0].attentionAt.IsZero()) | |||
| 46 | + | } | |||
| 47 | + | ||||
| 48 | + | func TestPaintDoesNotAckActiveAttentionOnBlankPulseFrame(t *testing.T) { | |||
| 49 | + | d, sess, ac, sends, releases := newManualTabSession(t, 1) | |||
| 50 | + | defer releases[0]() | |||
| 51 | + | sess.active = 0 | |||
| 52 | + | d.setAttentionFrame(0) | |||
| 53 | + | d.noteAttention(sess, sess.tabs[0]) | |||
| 54 | + | ||||
| 55 | + | d.paint(sess, ac, true) | |||
| 56 | + | data := mustOutputData(t, sends) | |||
| 57 | + | require.NotContains(t, string(data), string(attentionGlyph)) | |||
| 58 | + | ||||
| 59 | + | sess.mu.Lock() | |||
| 60 | + | require.True(t, sess.tabs[0].attention) | |||
| 61 | + | sess.mu.Unlock() | |||
| 62 | + | ||||
| 63 | + | d.setAttentionFrame(1) | |||
| 64 | + | d.paint(sess, ac, true) | |||
| 65 | + | data = mustOutputData(t, sends) | |||
| 66 | + | require.Contains(t, string(data), string(attentionGlyph)) | |||
| 67 | + | ||||
| 68 | + | sess.mu.Lock() | |||
| 44 | require.False(t, sess.tabs[0].attention) | 69 | require.False(t, sess.tabs[0].attention) | ||
| 45 | - | require.True(t, sess.tabs[0].attentionAt.IsZero()) | 70 | + | sess.mu.Unlock() |
| 71 | + | } | |||
| 72 | + | ||||
| 73 | + | func TestPTYReaderActiveVisibleAgentNotificationRendersBellBeforeAck(t *testing.T) { | |||
| 74 | + | tests := []struct { | |||
| 75 | + | name string | |||
| 76 | + | data []byte | |||
| 77 | + | }{ | |||
| 78 | + | {name: "bell", data: []byte("\a")}, | |||
| 79 | + | {name: "osc notify", data: []byte("\x1b]777;notify;Claude;needs input\x1b\\")}, | |||
| 80 | + | {name: "osc progress complete", data: []byte("\x1b]9;4;1;50\a\x1b]9;4;0;100\a")}, | |||
| 81 | + | } | |||
| 82 | + | ||||
| 83 | + | for _, tt := range tests { | |||
| 84 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 85 | + | pty := newScriptPTY([][]byte{tt.data}) | |||
| 86 | + | d, sess, ac, sends := newManualSessionWithPTYs(t, pty) | |||
| 87 | + | sess.active = 0 | |||
| 88 | + | d.setAttentionFrame(1) | |||
| 89 | + | ||||
| 90 | + | d.sessWg.Add(1) | |||
| 91 | + | go d.ptyReader(sess, sess.tabs[0], sess.tabs[0].focusedPane()) | |||
| 92 | + | ||||
| 93 | + | require.Eventually(t, func() bool { | |||
| 94 | + | sess.mu.Lock() | |||
| 95 | + | defer sess.mu.Unlock() | |||
| 96 | + | return sess.tabs[0].attention && !sess.tabs[0].attentionAt.IsZero() | |||
| 97 | + | }, 2*time.Second, 5*time.Millisecond) | |||
| 98 | + | ||||
| 99 | + | d.paint(sess, ac, true) | |||
| 100 | + | data := mustOutputData(t, sends) | |||
| 101 | + | require.Contains(t, string(data), string(attentionGlyph)) | |||
| 102 | + | ||||
| 103 | + | sess.mu.Lock() | |||
| 104 | + | require.False(t, sess.tabs[0].attention) | |||
| 105 | + | require.True(t, sess.tabs[0].attentionAt.IsZero()) | |||
| 106 | + | sess.mu.Unlock() | |||
| 107 | + | ||||
| 108 | + | cleared := mustOutputData(t, sends) | |||
| 109 | + | require.NotContains(t, string(cleared), string(attentionGlyph)) | |||
| 110 | + | ||||
| 111 | + | _ = pty.Close() | |||
| 112 | + | d.sessWg.Wait() | |||
| 113 | + | }) | |||
| 114 | + | } | |||
| 46 | } | 115 | } | ||
| 47 | 116 | ||||
| 48 | func TestNoteAttentionFlagsDetachedActiveTab(t *testing.T) { | 117 | func TestNoteAttentionFlagsDetachedActiveTab(t *testing.T) { | ||
M internal/usecase/daemon/render.go +2 -1
| @@ -308,9 +308,10 @@ func (d *Daemon) paint(sess *session, ac *attachedClient, reset bool) { | |||
| 308 | 308 | if overlays.previewTab != tb { | |
| 309 | 309 | preview = snapshotPickerPreview(overlays.previewTab) | |
| 310 | 310 | } | |
| 311 | - | repaintAttachedClients = sess.ackAttention(tb) | |
| 312 | 311 | bars := d.barStateForClient(sess, ac, overlays.copyFeedback) | |
| 313 | 312 | bars.theme = ac.getTheme() | |
| 313 | + | _, attentionVisible := pulseStyle(bars.attentionFrame) | |
| 314 | + | repaintAttachedClients = sess.ackAttention(tb, attentionVisible) | |
| 314 | 315 | ||
| 315 | 316 | styles := newThemeStyles(ac.getTheme()) | |
| 316 | 317 | tb.mu.Lock() | |
| @@ -308,9 +308,10 @@ func (d *Daemon) paint(sess *session, ac *attachedClient, reset bool) { | |||||
| 308 | if overlays.previewTab != tb { | 308 | if overlays.previewTab != tb { | ||
| 309 | preview = snapshotPickerPreview(overlays.previewTab) | 309 | preview = snapshotPickerPreview(overlays.previewTab) | ||
| 310 | } | 310 | } | ||
| 311 | - | repaintAttachedClients = sess.ackAttention(tb) | |||
| 312 | bars := d.barStateForClient(sess, ac, overlays.copyFeedback) | 311 | bars := d.barStateForClient(sess, ac, overlays.copyFeedback) | ||
| 313 | bars.theme = ac.getTheme() | 312 | bars.theme = ac.getTheme() | ||
| 313 | + | _, attentionVisible := pulseStyle(bars.attentionFrame) | |||
| 314 | + | repaintAttachedClients = sess.ackAttention(tb, attentionVisible) | |||
| 314 | 315 | ||||
| 315 | styles := newThemeStyles(ac.getTheme()) | 316 | styles := newThemeStyles(ac.getTheme()) | ||
| 316 | tb.mu.Lock() | 317 | tb.mu.Lock() | ||
M internal/usecase/daemon/session.go +4 -3
| @@ -86,9 +86,10 @@ type tab struct { | |||
| 86 | 86 | // previewClient tracks the one client currently previewing this tab in the picker. | |
| 87 | 87 | // v1 is last-writer-wins: multiple clients previewing the same tab are not supported. | |
| 88 | 88 | previewClient *attachedClient | |
| 89 | - | // attention and attentionAt are guarded by the owning session.mu. | |
| 90 | - | attention bool | |
| 91 | - | attentionAt time.Time | |
| 89 | + | // attention fields are guarded by the owning session.mu. | |
| 90 | + | attention bool | |
| 91 | + | attentionAt time.Time | |
| 92 | + | attentionVisiblePaint bool | |
| 92 | 93 | } | |
| 93 | 94 | ||
| 94 | 95 | // attachedClient is a client currently attached to a session's tab. rend is | |
| @@ -86,9 +86,10 @@ type tab struct { | |||||
| 86 | // previewClient tracks the one client currently previewing this tab in the picker. | 86 | // previewClient tracks the one client currently previewing this tab in the picker. | ||
| 87 | // v1 is last-writer-wins: multiple clients previewing the same tab are not supported. | 87 | // v1 is last-writer-wins: multiple clients previewing the same tab are not supported. | ||
| 88 | previewClient *attachedClient | 88 | previewClient *attachedClient | ||
| 89 | - | // attention and attentionAt are guarded by the owning session.mu. | 89 | + | // attention fields are guarded by the owning session.mu. |
| 90 | - | attention bool | 90 | + | attention bool |
| 91 | - | attentionAt time.Time | 91 | + | attentionAt time.Time |
| 92 | + | attentionVisiblePaint bool | |||
| 92 | } | 93 | } | ||
| 93 | 94 | ||||
| 94 | // attachedClient is a client currently attached to a session's tab. rend is | 95 | // attachedClient is a client currently attached to a session's tab. rend is | ||
M internal/usecase/daemon/status.go +3 -1
| @@ -205,7 +205,9 @@ func (s *session) statusSegments() statusSnapshot { | |||
| 205 | 205 | snap := statusSnapshot{session: name, tabs: make([]statusTab, len(s.tabs))} | |
| 206 | 206 | for i, tb := range s.tabs { | |
| 207 | 207 | name := strconv.Itoa(i + 1) | |
| 208 | - | snap.tabs[i] = statusTab{name: name, active: i == s.active, attention: tb.attention} | |
| 208 | + | active := i == s.active | |
| 209 | + | attention := tb.attention && (!active || tb.attentionVisiblePaint) | |
| 210 | + | snap.tabs[i] = statusTab{name: name, active: active, attention: attention} | |
| 209 | 211 | } | |
| 210 | 212 | return snap | |
| 211 | 213 | } | |
| @@ -205,7 +205,9 @@ func (s *session) statusSegments() statusSnapshot { | |||||
| 205 | snap := statusSnapshot{session: name, tabs: make([]statusTab, len(s.tabs))} | 205 | snap := statusSnapshot{session: name, tabs: make([]statusTab, len(s.tabs))} | ||
| 206 | for i, tb := range s.tabs { | 206 | for i, tb := range s.tabs { | ||
| 207 | name := strconv.Itoa(i + 1) | 207 | name := strconv.Itoa(i + 1) | ||
| 208 | - | snap.tabs[i] = statusTab{name: name, active: i == s.active, attention: tb.attention} | 208 | + | active := i == s.active |
| 209 | + | attention := tb.attention && (!active || tb.attentionVisiblePaint) | |||
| 210 | + | snap.tabs[i] = statusTab{name: name, active: active, attention: attention} | |||
| 209 | } | 211 | } | ||
| 210 | return snap | 212 | return snap | ||
| 211 | } | 213 | } | ||