fix(session): handle stale recent navigation
- commit
- 8a95a4829ac921402e3df6030596f0c57888e337
- author
- brice · · 2026-07-05 14:07 UTC
- parent
- af9fec1
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +65 -21
M internal/usecase/daemon/input_test.go +35 -0
| @@ -287,6 +287,41 @@ func TestPaletteBackForwardSessionBoundaries(t *testing.T) { | |||
| 287 | 287 | }) | |
| 288 | 288 | } | |
| 289 | 289 | ||
| 290 | + | func TestPaletteBackForwardSessionStaleTrail(t *testing.T) { | |
| 291 | + | t.Run("forward skips a killed intermediate session", func(t *testing.T) { | |
| 292 | + | d, current, ac, _, releases := newRecentNavigationTestSessions(t) | |
| 293 | + | defer releaseAll(releases) | |
| 294 | + | recent := d.sessions[domain.SessionID("recent")] | |
| 295 | + | older := d.sessions[domain.SessionID("older")] | |
| 296 | + | ||
| 297 | + | runPaletteCommand(t, d, current, ac, "BSK") | |
| 298 | + | runPaletteCommand(t, d, recent, ac, "BSK") | |
| 299 | + | require.Same(t, older, ac.currentSession()) | |
| 300 | + | ||
| 301 | + | d.mu.Lock() | |
| 302 | + | delete(d.sessions, recent.id) | |
| 303 | + | d.mu.Unlock() | |
| 304 | + | ||
| 305 | + | runPaletteCommand(t, d, older, ac, "FSK") | |
| 306 | + | require.Same(t, current, ac.currentSession()) | |
| 307 | + | }) | |
| 308 | + | ||
| 309 | + | t.Run("back from current refreshes newly recent sessions", func(t *testing.T) { | |
| 310 | + | d, current, ac, _, releases := newRecentNavigationTestSessions(t) | |
| 311 | + | defer releaseAll(releases) | |
| 312 | + | p, release := newBlockingPTY(t) | |
| 313 | + | defer release() | |
| 314 | + | newer := &session{id: "newer", name: "newer", ctx: current.ctx, cancel: func() {}, tabs: []*tab{newTab(p, domain.Size{Cols: 80, Rows: 23})}} | |
| 315 | + | newer.mruAt.Store(25) | |
| 316 | + | d.mu.Lock() | |
| 317 | + | d.sessions[newer.id] = newer | |
| 318 | + | d.mu.Unlock() | |
| 319 | + | ||
| 320 | + | runPaletteCommand(t, d, current, ac, "BSK") | |
| 321 | + | require.Same(t, newer, ac.currentSession()) | |
| 322 | + | }) | |
| 323 | + | } | |
| 324 | + | ||
| 290 | 325 | func newRecentNavigationTestSessions(t *testing.T) (*Daemon, *session, *attachedClient, chan ports.Frame, []func()) { | |
| 291 | 326 | t.Helper() | |
| 292 | 327 | p1, release1 := newBlockingPTY(t) | |
| @@ -287,6 +287,41 @@ func TestPaletteBackForwardSessionBoundaries(t *testing.T) { | |||||
| 287 | }) | 287 | }) | ||
| 288 | } | 288 | } | ||
| 289 | 289 | ||||
| 290 | + | func TestPaletteBackForwardSessionStaleTrail(t *testing.T) { | |||
| 291 | + | t.Run("forward skips a killed intermediate session", func(t *testing.T) { | |||
| 292 | + | d, current, ac, _, releases := newRecentNavigationTestSessions(t) | |||
| 293 | + | defer releaseAll(releases) | |||
| 294 | + | recent := d.sessions[domain.SessionID("recent")] | |||
| 295 | + | older := d.sessions[domain.SessionID("older")] | |||
| 296 | + | ||||
| 297 | + | runPaletteCommand(t, d, current, ac, "BSK") | |||
| 298 | + | runPaletteCommand(t, d, recent, ac, "BSK") | |||
| 299 | + | require.Same(t, older, ac.currentSession()) | |||
| 300 | + | ||||
| 301 | + | d.mu.Lock() | |||
| 302 | + | delete(d.sessions, recent.id) | |||
| 303 | + | d.mu.Unlock() | |||
| 304 | + | ||||
| 305 | + | runPaletteCommand(t, d, older, ac, "FSK") | |||
| 306 | + | require.Same(t, current, ac.currentSession()) | |||
| 307 | + | }) | |||
| 308 | + | ||||
| 309 | + | t.Run("back from current refreshes newly recent sessions", func(t *testing.T) { | |||
| 310 | + | d, current, ac, _, releases := newRecentNavigationTestSessions(t) | |||
| 311 | + | defer releaseAll(releases) | |||
| 312 | + | p, release := newBlockingPTY(t) | |||
| 313 | + | defer release() | |||
| 314 | + | newer := &session{id: "newer", name: "newer", ctx: current.ctx, cancel: func() {}, tabs: []*tab{newTab(p, domain.Size{Cols: 80, Rows: 23})}} | |||
| 315 | + | newer.mruAt.Store(25) | |||
| 316 | + | d.mu.Lock() | |||
| 317 | + | d.sessions[newer.id] = newer | |||
| 318 | + | d.mu.Unlock() | |||
| 319 | + | ||||
| 320 | + | runPaletteCommand(t, d, current, ac, "BSK") | |||
| 321 | + | require.Same(t, newer, ac.currentSession()) | |||
| 322 | + | }) | |||
| 323 | + | } | |||
| 324 | + | ||||
| 290 | func newRecentNavigationTestSessions(t *testing.T) (*Daemon, *session, *attachedClient, chan ports.Frame, []func()) { | 325 | func newRecentNavigationTestSessions(t *testing.T) (*Daemon, *session, *attachedClient, chan ports.Frame, []func()) { | ||
| 291 | t.Helper() | 326 | t.Helper() | ||
| 292 | p1, release1 := newBlockingPTY(t) | 327 | p1, release1 := newBlockingPTY(t) | ||
M internal/usecase/daemon/recent_nav.go +30 -21
| @@ -34,11 +34,10 @@ func (d *Daemon) navigateRecentSession(sess *session, ac *attachedClient, delta | |||
| 34 | 34 | return | |
| 35 | 35 | } | |
| 36 | 36 | trail := recentSessionTrail(sess, d.barStateFor(sess, "")) | |
| 37 | - | if len(trail) == 0 { | |
| 38 | - | d.paint(sess, ac, true) | |
| 39 | - | return | |
| 37 | + | if ac.recentNav.index == 0 && !sameSessionTrail(ac.recentNav.ids, trail) { | |
| 38 | + | ac.recentNav.reset(trail) | |
| 40 | 39 | } | |
| 41 | - | if !d.recentNavValid(&ac.recentNav) || ac.recentNav.index < 0 || ac.recentNav.index >= len(ac.recentNav.ids) || ac.recentNav.ids[ac.recentNav.index] != sess.id { | |
| 40 | + | if !d.pruneRecentNav(&ac.recentNav) || ac.recentNav.index < 0 || ac.recentNav.index >= len(ac.recentNav.ids) || ac.recentNav.ids[ac.recentNav.index] != sess.id { | |
| 42 | 41 | ac.recentNav.reset(trail) | |
| 43 | 42 | } | |
| 44 | 43 | ||
| @@ -47,30 +46,40 @@ func (d *Daemon) navigateRecentSession(sess *session, ac *attachedClient, delta | |||
| 47 | 46 | d.paint(sess, ac, true) | |
| 48 | 47 | return | |
| 49 | 48 | } | |
| 50 | - | targetID := ac.recentNav.ids[next] | |
| 51 | - | if d.sessionByID(targetID) == nil { | |
| 52 | - | ac.recentNav.reset(trail) | |
| 53 | - | next = ac.recentNav.index + delta | |
| 54 | - | if next < 0 || next >= len(ac.recentNav.ids) { | |
| 55 | - | d.paint(sess, ac, true) | |
| 56 | - | return | |
| 57 | - | } | |
| 58 | - | targetID = ac.recentNav.ids[next] | |
| 59 | - | if d.sessionByID(targetID) == nil { | |
| 60 | - | d.paint(sess, ac, true) | |
| 61 | - | return | |
| 62 | - | } | |
| 63 | - | } | |
| 64 | 49 | ac.recentNav.index = next | |
| 65 | - | d.switchToTarget(sess, ac, picker.Target{Session: targetID, TabIndex: -1}) | |
| 50 | + | d.switchToTarget(sess, ac, picker.Target{Session: ac.recentNav.ids[next], TabIndex: -1}) | |
| 66 | 51 | } | |
| 67 | 52 | ||
| 68 | - | func (d *Daemon) recentNavValid(n *recentSessionNavigator) bool { | |
| 69 | - | if n == nil || len(n.ids) == 0 { | |
| 53 | + | func (d *Daemon) pruneRecentNav(n *recentSessionNavigator) bool { | |
| 54 | + | if n == nil || len(n.ids) == 0 || n.index < 0 || n.index >= len(n.ids) { | |
| 70 | 55 | return false | |
| 71 | 56 | } | |
| 57 | + | current := n.ids[n.index] | |
| 58 | + | kept := n.ids[:0] | |
| 59 | + | nextIndex := -1 | |
| 72 | 60 | for _, id := range n.ids { | |
| 73 | 61 | if d.sessionByID(id) == nil { | |
| 62 | + | continue | |
| 63 | + | } | |
| 64 | + | if id == current { | |
| 65 | + | nextIndex = len(kept) | |
| 66 | + | } | |
| 67 | + | kept = append(kept, id) | |
| 68 | + | } | |
| 69 | + | n.ids = kept | |
| 70 | + | if nextIndex == -1 { | |
| 71 | + | return false | |
| 72 | + | } | |
| 73 | + | n.index = nextIndex | |
| 74 | + | return true | |
| 75 | + | } | |
| 76 | + | ||
| 77 | + | func sameSessionTrail(a, b []domain.SessionID) bool { | |
| 78 | + | if len(a) != len(b) { | |
| 79 | + | return false | |
| 80 | + | } | |
| 81 | + | for i := range a { | |
| 82 | + | if a[i] != b[i] { | |
| 74 | 83 | return false | |
| 75 | 84 | } | |
| 76 | 85 | } | |
| @@ -34,11 +34,10 @@ func (d *Daemon) navigateRecentSession(sess *session, ac *attachedClient, delta | |||||
| 34 | return | 34 | return | ||
| 35 | } | 35 | } | ||
| 36 | trail := recentSessionTrail(sess, d.barStateFor(sess, "")) | 36 | trail := recentSessionTrail(sess, d.barStateFor(sess, "")) | ||
| 37 | - | if len(trail) == 0 { | 37 | + | if ac.recentNav.index == 0 && !sameSessionTrail(ac.recentNav.ids, trail) { |
| 38 | - | d.paint(sess, ac, true) | 38 | + | ac.recentNav.reset(trail) |
| 39 | - | return | |||
| 40 | } | 39 | } | ||
| 41 | - | if !d.recentNavValid(&ac.recentNav) || ac.recentNav.index < 0 || ac.recentNav.index >= len(ac.recentNav.ids) || ac.recentNav.ids[ac.recentNav.index] != sess.id { | 40 | + | if !d.pruneRecentNav(&ac.recentNav) || ac.recentNav.index < 0 || ac.recentNav.index >= len(ac.recentNav.ids) || ac.recentNav.ids[ac.recentNav.index] != sess.id { |
| 42 | ac.recentNav.reset(trail) | 41 | ac.recentNav.reset(trail) | ||
| 43 | } | 42 | } | ||
| 44 | 43 | ||||
| @@ -47,30 +46,40 @@ func (d *Daemon) navigateRecentSession(sess *session, ac *attachedClient, delta | |||||
| 47 | d.paint(sess, ac, true) | 46 | d.paint(sess, ac, true) | ||
| 48 | return | 47 | return | ||
| 49 | } | 48 | } | ||
| 50 | - | targetID := ac.recentNav.ids[next] | |||
| 51 | - | if d.sessionByID(targetID) == nil { | |||
| 52 | - | ac.recentNav.reset(trail) | |||
| 53 | - | next = ac.recentNav.index + delta | |||
| 54 | - | if next < 0 || next >= len(ac.recentNav.ids) { | |||
| 55 | - | d.paint(sess, ac, true) | |||
| 56 | - | return | |||
| 57 | - | } | |||
| 58 | - | targetID = ac.recentNav.ids[next] | |||
| 59 | - | if d.sessionByID(targetID) == nil { | |||
| 60 | - | d.paint(sess, ac, true) | |||
| 61 | - | return | |||
| 62 | - | } | |||
| 63 | - | } | |||
| 64 | ac.recentNav.index = next | 49 | ac.recentNav.index = next | ||
| 65 | - | d.switchToTarget(sess, ac, picker.Target{Session: targetID, TabIndex: -1}) | 50 | + | d.switchToTarget(sess, ac, picker.Target{Session: ac.recentNav.ids[next], TabIndex: -1}) |
| 66 | } | 51 | } | ||
| 67 | 52 | ||||
| 68 | - | func (d *Daemon) recentNavValid(n *recentSessionNavigator) bool { | 53 | + | func (d *Daemon) pruneRecentNav(n *recentSessionNavigator) bool { |
| 69 | - | if n == nil || len(n.ids) == 0 { | 54 | + | if n == nil || len(n.ids) == 0 || n.index < 0 || n.index >= len(n.ids) { |
| 70 | return false | 55 | return false | ||
| 71 | } | 56 | } | ||
| 57 | + | current := n.ids[n.index] | |||
| 58 | + | kept := n.ids[:0] | |||
| 59 | + | nextIndex := -1 | |||
| 72 | for _, id := range n.ids { | 60 | for _, id := range n.ids { | ||
| 73 | if d.sessionByID(id) == nil { | 61 | if d.sessionByID(id) == nil { | ||
| 62 | + | continue | |||
| 63 | + | } | |||
| 64 | + | if id == current { | |||
| 65 | + | nextIndex = len(kept) | |||
| 66 | + | } | |||
| 67 | + | kept = append(kept, id) | |||
| 68 | + | } | |||
| 69 | + | n.ids = kept | |||
| 70 | + | if nextIndex == -1 { | |||
| 71 | + | return false | |||
| 72 | + | } | |||
| 73 | + | n.index = nextIndex | |||
| 74 | + | return true | |||
| 75 | + | } | |||
| 76 | + | ||||
| 77 | + | func sameSessionTrail(a, b []domain.SessionID) bool { | |||
| 78 | + | if len(a) != len(b) { | |||
| 79 | + | return false | |||
| 80 | + | } | |||
| 81 | + | for i := range a { | |||
| 82 | + | if a[i] != b[i] { | |||
| 74 | return false | 83 | return false | ||
| 75 | } | 84 | } | ||
| 76 | } | 85 | } | ||