fix(daemon): enforce overflow handoff invariants
- commit
- 8953d4264e07c1bb42e9b616fe0fe64748f5c090
- author
- brice · · 2026-07-25 05:29 UTC
- parent
- 777c969
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +78 -1
M internal/usecase/daemon/navigation_overflow.go +27 -0
| @@ -50,6 +50,23 @@ func resolveOverflow(dir layout.Direction, cfg domain.NavConfig, position, count | |||
| 50 | 50 | return step | |
| 51 | 51 | } | |
| 52 | 52 | ||
| 53 | + | // overflowSourceEligible validates that directional overflow still originates | |
| 54 | + | // from the active tiled tab. The session-to-tab lock order matches tab overflow | |
| 55 | + | // commit and leaves no lock held across a session handoff. | |
| 56 | + | func overflowSourceEligible(sess *session, expectedSource *tab) bool { | |
| 57 | + | if sess == nil || expectedSource == nil { | |
| 58 | + | return false | |
| 59 | + | } | |
| 60 | + | sess.mu.Lock() | |
| 61 | + | defer sess.mu.Unlock() | |
| 62 | + | if sess.active < 0 || sess.active >= len(sess.tabs) || sess.tabs[sess.active] != expectedSource { | |
| 63 | + | return false | |
| 64 | + | } | |
| 65 | + | expectedSource.mu.Lock() | |
| 66 | + | defer expectedSource.mu.Unlock() | |
| 67 | + | return expectedSource.floating.state != floatingVisible | |
| 68 | + | } | |
| 69 | + | ||
| 53 | 70 | type sessionOverflowSnapshot struct { | |
| 54 | 71 | session *session | |
| 55 | 72 | id domain.SessionID | |
| @@ -98,6 +115,16 @@ func (d *Daemon) prepareSessionOverflow(current *session, dir layout.Direction, | |||
| 98 | 115 | return picker.Target{Session: target.id, TabIndex: -1}, true | |
| 99 | 116 | } | |
| 100 | 117 | ||
| 118 | + | // commitSessionOverflow revalidates the floating-source exclusion immediately | |
| 119 | + | // before switchToTarget. It deliberately releases session and tab locks first: | |
| 120 | + | // switchToTarget owns the daemon -> routing -> session lock order. | |
| 121 | + | func (d *Daemon) commitSessionOverflow(from *session, ac *attachedClient, expectedSource *tab, target picker.Target) error { | |
| 122 | + | if !overflowSourceEligible(from, expectedSource) { | |
| 123 | + | return errNoNeighbor | |
| 124 | + | } | |
| 125 | + | return d.switchToTarget(from, ac, target) | |
| 126 | + | } | |
| 127 | + | ||
| 101 | 128 | type tabOverflowCandidate struct { | |
| 102 | 129 | source *tab | |
| 103 | 130 | target *tab | |
| @@ -50,6 +50,23 @@ func resolveOverflow(dir layout.Direction, cfg domain.NavConfig, position, count | |||||
| 50 | return step | 50 | return step | ||
| 51 | } | 51 | } | ||
| 52 | 52 | ||||
| 53 | + | // overflowSourceEligible validates that directional overflow still originates | |||
| 54 | + | // from the active tiled tab. The session-to-tab lock order matches tab overflow | |||
| 55 | + | // commit and leaves no lock held across a session handoff. | |||
| 56 | + | func overflowSourceEligible(sess *session, expectedSource *tab) bool { | |||
| 57 | + | if sess == nil || expectedSource == nil { | |||
| 58 | + | return false | |||
| 59 | + | } | |||
| 60 | + | sess.mu.Lock() | |||
| 61 | + | defer sess.mu.Unlock() | |||
| 62 | + | if sess.active < 0 || sess.active >= len(sess.tabs) || sess.tabs[sess.active] != expectedSource { | |||
| 63 | + | return false | |||
| 64 | + | } | |||
| 65 | + | expectedSource.mu.Lock() | |||
| 66 | + | defer expectedSource.mu.Unlock() | |||
| 67 | + | return expectedSource.floating.state != floatingVisible | |||
| 68 | + | } | |||
| 69 | + | ||||
| 53 | type sessionOverflowSnapshot struct { | 70 | type sessionOverflowSnapshot struct { | ||
| 54 | session *session | 71 | session *session | ||
| 55 | id domain.SessionID | 72 | id domain.SessionID | ||
| @@ -98,6 +115,16 @@ func (d *Daemon) prepareSessionOverflow(current *session, dir layout.Direction, | |||||
| 98 | return picker.Target{Session: target.id, TabIndex: -1}, true | 115 | return picker.Target{Session: target.id, TabIndex: -1}, true | ||
| 99 | } | 116 | } | ||
| 100 | 117 | ||||
| 118 | + | // commitSessionOverflow revalidates the floating-source exclusion immediately | |||
| 119 | + | // before switchToTarget. It deliberately releases session and tab locks first: | |||
| 120 | + | // switchToTarget owns the daemon -> routing -> session lock order. | |||
| 121 | + | func (d *Daemon) commitSessionOverflow(from *session, ac *attachedClient, expectedSource *tab, target picker.Target) error { | |||
| 122 | + | if !overflowSourceEligible(from, expectedSource) { | |||
| 123 | + | return errNoNeighbor | |||
| 124 | + | } | |||
| 125 | + | return d.switchToTarget(from, ac, target) | |||
| 126 | + | } | |||
| 127 | + | ||||
| 101 | type tabOverflowCandidate struct { | 128 | type tabOverflowCandidate struct { | ||
| 102 | source *tab | 129 | source *tab | ||
| 103 | target *tab | 130 | target *tab | ||
M internal/usecase/daemon/navigation_overflow_test.go +47 -0
| @@ -149,6 +149,53 @@ func TestKeyboardVerticalOverflowSwitchesOnlyAcrossAlphabeticalLiveSessions(t *t | |||
| 149 | 149 | require.Same(t, ac, attached, "the source session owns the genuinely attached client after returning") | |
| 150 | 150 | } | |
| 151 | 151 | ||
| 152 | + | func TestKeyboardVerticalOverflowRefusesVisibleFloatingSource(t *testing.T) { | |
| 153 | + | d, alpha, ac, _ := newManualSessionWithPTYs(t, nil) | |
| 154 | + | alpha.mu.Lock() | |
| 155 | + | alpha.name = "alpha" | |
| 156 | + | alpha.mu.Unlock() | |
| 157 | + | installTestFloating(alpha.tabs[0], newPane("floating", nil, domain.Size{Cols: 20, Rows: 5}), true) | |
| 158 | + | charlie := &session{id: "live-charlie", name: "charlie", ctx: t.Context(), cancel: func() {}, tabs: []*tab{newTab(nil, domain.Size{Cols: 41, Rows: 10})}} | |
| 159 | + | d.mu.Lock() | |
| 160 | + | d.sessions[charlie.id] = charlie | |
| 161 | + | d.mu.Unlock() | |
| 162 | + | d.ApplyConfig(domain.Config{Nav: domain.NavConfig{OverflowSessions: true}}) | |
| 163 | + | ||
| 164 | + | daemonKeyHandler{d: d, ac: ac}.Action(keys.ActionFocusPaneDown) | |
| 165 | + | ||
| 166 | + | require.Same(t, alpha, ac.currentSession()) | |
| 167 | + | alpha.mu.Lock() | |
| 168 | + | require.Same(t, ac, alpha.client) | |
| 169 | + | alpha.mu.Unlock() | |
| 170 | + | charlie.mu.Lock() | |
| 171 | + | require.Nil(t, charlie.client) | |
| 172 | + | charlie.mu.Unlock() | |
| 173 | + | } | |
| 174 | + | ||
| 175 | + | func TestVerticalOverflowRevalidatesFloatingSourceBeforeHandoff(t *testing.T) { | |
| 176 | + | d, alpha, ac, _ := newManualSessionWithPTYs(t, nil) | |
| 177 | + | alpha.mu.Lock() | |
| 178 | + | alpha.name = "alpha" | |
| 179 | + | alpha.mu.Unlock() | |
| 180 | + | charlie := &session{id: "live-charlie", name: "charlie", ctx: t.Context(), cancel: func() {}, tabs: []*tab{newTab(nil, domain.Size{Cols: 41, Rows: 10})}} | |
| 181 | + | d.mu.Lock() | |
| 182 | + | d.sessions[charlie.id] = charlie | |
| 183 | + | d.mu.Unlock() | |
| 184 | + | ||
| 185 | + | target, ok := d.prepareSessionOverflow(alpha, layout.Down, domain.NavConfig{OverflowSessions: true}) | |
| 186 | + | require.True(t, ok) | |
| 187 | + | installTestFloating(alpha.tabs[0], newPane("floating", nil, domain.Size{Cols: 20, Rows: 5}), true) | |
| 188 | + | ||
| 189 | + | require.ErrorIs(t, d.commitSessionOverflow(alpha, ac, alpha.tabs[0], target), errNoNeighbor) | |
| 190 | + | require.Same(t, alpha, ac.currentSession()) | |
| 191 | + | alpha.mu.Lock() | |
| 192 | + | require.Same(t, ac, alpha.client) | |
| 193 | + | alpha.mu.Unlock() | |
| 194 | + | charlie.mu.Lock() | |
| 195 | + | require.Nil(t, charlie.client) | |
| 196 | + | charlie.mu.Unlock() | |
| 197 | + | } | |
| 198 | + | ||
| 152 | 199 | func TestVerticalOverflowIsRaceFreeDuringSessionRename(t *testing.T) { | |
| 153 | 200 | d, alpha, ac, _ := newManualSessionWithPTYs(t, nil) | |
| 154 | 201 | alpha.mu.Lock() | |
| @@ -149,6 +149,53 @@ func TestKeyboardVerticalOverflowSwitchesOnlyAcrossAlphabeticalLiveSessions(t *t | |||||
| 149 | require.Same(t, ac, attached, "the source session owns the genuinely attached client after returning") | 149 | require.Same(t, ac, attached, "the source session owns the genuinely attached client after returning") | ||
| 150 | } | 150 | } | ||
| 151 | 151 | ||||
| 152 | + | func TestKeyboardVerticalOverflowRefusesVisibleFloatingSource(t *testing.T) { | |||
| 153 | + | d, alpha, ac, _ := newManualSessionWithPTYs(t, nil) | |||
| 154 | + | alpha.mu.Lock() | |||
| 155 | + | alpha.name = "alpha" | |||
| 156 | + | alpha.mu.Unlock() | |||
| 157 | + | installTestFloating(alpha.tabs[0], newPane("floating", nil, domain.Size{Cols: 20, Rows: 5}), true) | |||
| 158 | + | charlie := &session{id: "live-charlie", name: "charlie", ctx: t.Context(), cancel: func() {}, tabs: []*tab{newTab(nil, domain.Size{Cols: 41, Rows: 10})}} | |||
| 159 | + | d.mu.Lock() | |||
| 160 | + | d.sessions[charlie.id] = charlie | |||
| 161 | + | d.mu.Unlock() | |||
| 162 | + | d.ApplyConfig(domain.Config{Nav: domain.NavConfig{OverflowSessions: true}}) | |||
| 163 | + | ||||
| 164 | + | daemonKeyHandler{d: d, ac: ac}.Action(keys.ActionFocusPaneDown) | |||
| 165 | + | ||||
| 166 | + | require.Same(t, alpha, ac.currentSession()) | |||
| 167 | + | alpha.mu.Lock() | |||
| 168 | + | require.Same(t, ac, alpha.client) | |||
| 169 | + | alpha.mu.Unlock() | |||
| 170 | + | charlie.mu.Lock() | |||
| 171 | + | require.Nil(t, charlie.client) | |||
| 172 | + | charlie.mu.Unlock() | |||
| 173 | + | } | |||
| 174 | + | ||||
| 175 | + | func TestVerticalOverflowRevalidatesFloatingSourceBeforeHandoff(t *testing.T) { | |||
| 176 | + | d, alpha, ac, _ := newManualSessionWithPTYs(t, nil) | |||
| 177 | + | alpha.mu.Lock() | |||
| 178 | + | alpha.name = "alpha" | |||
| 179 | + | alpha.mu.Unlock() | |||
| 180 | + | charlie := &session{id: "live-charlie", name: "charlie", ctx: t.Context(), cancel: func() {}, tabs: []*tab{newTab(nil, domain.Size{Cols: 41, Rows: 10})}} | |||
| 181 | + | d.mu.Lock() | |||
| 182 | + | d.sessions[charlie.id] = charlie | |||
| 183 | + | d.mu.Unlock() | |||
| 184 | + | ||||
| 185 | + | target, ok := d.prepareSessionOverflow(alpha, layout.Down, domain.NavConfig{OverflowSessions: true}) | |||
| 186 | + | require.True(t, ok) | |||
| 187 | + | installTestFloating(alpha.tabs[0], newPane("floating", nil, domain.Size{Cols: 20, Rows: 5}), true) | |||
| 188 | + | ||||
| 189 | + | require.ErrorIs(t, d.commitSessionOverflow(alpha, ac, alpha.tabs[0], target), errNoNeighbor) | |||
| 190 | + | require.Same(t, alpha, ac.currentSession()) | |||
| 191 | + | alpha.mu.Lock() | |||
| 192 | + | require.Same(t, ac, alpha.client) | |||
| 193 | + | alpha.mu.Unlock() | |||
| 194 | + | charlie.mu.Lock() | |||
| 195 | + | require.Nil(t, charlie.client) | |||
| 196 | + | charlie.mu.Unlock() | |||
| 197 | + | } | |||
| 198 | + | ||||
| 152 | func TestVerticalOverflowIsRaceFreeDuringSessionRename(t *testing.T) { | 199 | func TestVerticalOverflowIsRaceFreeDuringSessionRename(t *testing.T) { | ||
| 153 | d, alpha, ac, _ := newManualSessionWithPTYs(t, nil) | 200 | d, alpha, ac, _ := newManualSessionWithPTYs(t, nil) | ||
| 154 | alpha.mu.Lock() | 201 | alpha.mu.Lock() | ||
M internal/usecase/daemon/pane_actions.go +4 -1
| @@ -319,13 +319,16 @@ func (d *Daemon) focusDir(sess *session, ac *attachedClient, dir layout.Directio | |||
| 319 | 319 | if !errors.Is(err, errNoNeighbor) || target.tab == nil { | |
| 320 | 320 | return err | |
| 321 | 321 | } | |
| 322 | + | if !overflowSourceEligible(sess, target.tab) { | |
| 323 | + | return errNoNeighbor | |
| 324 | + | } | |
| 322 | 325 | ||
| 323 | 326 | cfg := d.currentNavConfig() | |
| 324 | 327 | if sessionTarget, ok := d.prepareSessionOverflow(sess, dir, cfg); ok { | |
| 325 | 328 | if ac == nil { | |
| 326 | 329 | return errNoNeighbor | |
| 327 | 330 | } | |
| 328 | - | return d.switchToTarget(sess, ac, sessionTarget) | |
| 331 | + | return d.commitSessionOverflow(sess, ac, target.tab, sessionTarget) | |
| 329 | 332 | } | |
| 330 | 333 | ||
| 331 | 334 | sess.mu.Lock() | |
| @@ -319,13 +319,16 @@ func (d *Daemon) focusDir(sess *session, ac *attachedClient, dir layout.Directio | |||||
| 319 | if !errors.Is(err, errNoNeighbor) || target.tab == nil { | 319 | if !errors.Is(err, errNoNeighbor) || target.tab == nil { | ||
| 320 | return err | 320 | return err | ||
| 321 | } | 321 | } | ||
| 322 | + | if !overflowSourceEligible(sess, target.tab) { | |||
| 323 | + | return errNoNeighbor | |||
| 324 | + | } | |||
| 322 | 325 | ||||
| 323 | cfg := d.currentNavConfig() | 326 | cfg := d.currentNavConfig() | ||
| 324 | if sessionTarget, ok := d.prepareSessionOverflow(sess, dir, cfg); ok { | 327 | if sessionTarget, ok := d.prepareSessionOverflow(sess, dir, cfg); ok { | ||
| 325 | if ac == nil { | 328 | if ac == nil { | ||
| 326 | return errNoNeighbor | 329 | return errNoNeighbor | ||
| 327 | } | 330 | } | ||
| 328 | - | return d.switchToTarget(sess, ac, sessionTarget) | 331 | + | return d.commitSessionOverflow(sess, ac, target.tab, sessionTarget) |
| 329 | } | 332 | } | ||
| 330 | 333 | ||||
| 331 | sess.mu.Lock() | 334 | sess.mu.Lock() | ||