fix(daemon): guard session overflow handoff atomically

commit: 05d468760e2b22624f6507d0a50e3bb88b8767f1
author: brice · 3mo ago · 2026-07-25 05:39 UTC
parent: 8953d42
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

3 files · +106 -9

M internal/usecase/daemon/navigation_overflow.go +5 -5


@@ -115,14 +115,14 @@ func (d Daemon) prepareSessionOverflow(current session, dir layout.Direction,
115	115		return picker.Target{Session: target.id, TabIndex: -1}, true
116	116		}
117	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.
	118	+	// commitSessionOverflow carries its source constraint into the handoff so tab
	119	+	// identity and floating visibility are revalidated while source ownership is
	120	+	// transferred under the daemon -> routing -> session -> tab lock order.
121	121		func (d Daemon) commitSessionOverflow(from session, ac attachedClient, expectedSource tab, target picker.Target) error {
122		-	if !overflowSourceEligible(from, expectedSource) {
	122	+	if from == nil \|\| ac == nil \|\| expectedSource == nil {
123	123		return errNoNeighbor
124	124		}
125		-	return d.switchToTarget(from, ac, target)
	125	+	return d.switchToTargetGuarded(from, ac, target, sessionHandoffGuard{expectedSource: expectedSource})
126	126		}
127	127
128	128		type tabOverflowCandidate struct {


@@ -115,14 +115,14 @@ func (d Daemon) prepareSessionOverflow(current session, dir layout.Direction,
115		return picker.Target{Session: target.id, TabIndex: -1}, true	115		return picker.Target{Session: target.id, TabIndex: -1}, true
116		}	116		}
117			117
118	-	// commitSessionOverflow revalidates the floating-source exclusion immediately	118	+	// commitSessionOverflow carries its source constraint into the handoff so tab
119	-	// before switchToTarget. It deliberately releases session and tab locks first:	119	+	// identity and floating visibility are revalidated while source ownership is
120	-	// switchToTarget owns the daemon -> routing -> session lock order.	120	+	// transferred under the daemon -> routing -> session -> tab lock order.
121		func (d Daemon) commitSessionOverflow(from session, ac attachedClient, expectedSource tab, target picker.Target) error {	121		func (d Daemon) commitSessionOverflow(from session, ac attachedClient, expectedSource tab, target picker.Target) error {
122	-	if !overflowSourceEligible(from, expectedSource) {	122	+	if from == nil \|\| ac == nil \|\| expectedSource == nil {
123		return errNoNeighbor	123		return errNoNeighbor
124		}	124		}
125	-	return d.switchToTarget(from, ac, target)	125	+	return d.switchToTargetGuarded(from, ac, target, sessionHandoffGuard{expectedSource: expectedSource})
126		}	126		}
127			127
128		type tabOverflowCandidate struct {	128		type tabOverflowCandidate struct {

M internal/usecase/daemon/navigation_overflow_test.go +62 -0


@@ -9,6 +9,7 @@ import (
9	9		"github.com/bnema/vev/internal/domain"
10	10		"github.com/bnema/vev/internal/usecase/keys"
11	11		"github.com/bnema/vev/internal/usecase/layout"
	12	+	"github.com/bnema/vev/internal/usecase/picker"
12	13		)
13	14
14	15		func TestResolveOverflowObeysAxisConfigurationAndWalls(t *testing.T) {

@@ -196,6 +197,67 @@ func TestVerticalOverflowRevalidatesFloatingSourceBeforeHandoff(t *testing.T) {
196	197		charlie.mu.Unlock()
197	198		}
198	199
	200	+	func TestVerticalOverflowRejectsStaleSourceTab(t *testing.T) {
	201	+	d, alpha, ac, _ := newManualSessionWithPTYs(t, nil, nil)
	202	+	alpha.mu.Lock()
	203	+	alpha.name = "alpha"
	204	+	expectedSource := alpha.tabs[0]
	205	+	alpha.active = 1
	206	+	alpha.mu.Unlock()
	207	+	charlie := &session{id: "live-charlie", name: "charlie", ctx: t.Context(), cancel: func() {}, tabs: []*tab{newTab(nil, domain.Size{Cols: 41, Rows: 10})}}
	208	+	d.mu.Lock()
	209	+	d.sessions[charlie.id] = charlie
	210	+	d.mu.Unlock()
	211	+
	212	+	require.ErrorIs(t, d.commitSessionOverflow(alpha, ac, expectedSource, picker.Target{Session: charlie.id, TabIndex: -1}), errNoNeighbor)
	213	+	require.Same(t, alpha, ac.currentSession())
	214	+	alpha.mu.Lock()
	215	+	require.Same(t, ac, alpha.client)
	216	+	alpha.mu.Unlock()
	217	+	charlie.mu.Lock()
	218	+	require.Nil(t, charlie.client)
	219	+	charlie.mu.Unlock()
	220	+	}
	221	+
	222	+	func TestOrdinarySessionSwitchDoesNotApplyOverflowGuard(t *testing.T) {
	223	+	d, alpha, ac, _ := newManualSessionWithPTYs(t, nil)
	224	+	installTestFloating(alpha.tabs[0], newPane("floating", nil, domain.Size{Cols: 20, Rows: 5}), true)
	225	+	charlie := &session{id: "live-charlie", name: "charlie", ctx: t.Context(), cancel: func() {}, tabs: []*tab{newTab(nil, domain.Size{Cols: 41, Rows: 10})}}
	226	+	d.mu.Lock()
	227	+	d.sessions[charlie.id] = charlie
	228	+	d.mu.Unlock()
	229	+
	230	+	require.NoError(t, d.switchToTarget(alpha, ac, picker.Target{Session: charlie.id, TabIndex: -1}))
	231	+	require.Same(t, charlie, ac.currentSession())
	232	+	}
	233	+
	234	+	func TestVerticalOverflowTreatsDisplacedSourceClientAsNoNeighbor(t *testing.T) {
	235	+	d, alpha, ac, _ := newManualSessionWithPTYs(t, nil)
	236	+	alpha.mu.Lock()
	237	+	alpha.name = "alpha"
	238	+	alpha.mu.Unlock()
	239	+	charlie := &session{id: "live-charlie", name: "charlie", ctx: t.Context(), cancel: func() {}, tabs: []*tab{newTab(nil, domain.Size{Cols: 41, Rows: 10})}}
	240	+	d.mu.Lock()
	241	+	d.sessions[charlie.id] = charlie
	242	+	d.mu.Unlock()
	243	+
	244	+	target, ok := d.prepareSessionOverflow(alpha, layout.Down, domain.NavConfig{OverflowSessions: true})
	245	+	require.True(t, ok)
	246	+	replacement := &attachedClient{}
	247	+	alpha.mu.Lock()
	248	+	alpha.client = replacement
	249	+	alpha.mu.Unlock()
	250	+
	251	+	require.ErrorIs(t, d.commitSessionOverflow(alpha, ac, alpha.tabs[0], target), errNoNeighbor)
	252	+	require.Same(t, alpha, ac.currentSession())
	253	+	alpha.mu.Lock()
	254	+	require.Same(t, replacement, alpha.client)
	255	+	alpha.mu.Unlock()
	256	+	charlie.mu.Lock()
	257	+	require.Nil(t, charlie.client)
	258	+	charlie.mu.Unlock()
	259	+	}
	260	+
199	261		func TestVerticalOverflowIsRaceFreeDuringSessionRename(t *testing.T) {
200	262		d, alpha, ac, _ := newManualSessionWithPTYs(t, nil)
201	263		alpha.mu.Lock()


@@ -9,6 +9,7 @@ import (
9	"github.com/bnema/vev/internal/domain"	9		"github.com/bnema/vev/internal/domain"
10	"github.com/bnema/vev/internal/usecase/keys"	10		"github.com/bnema/vev/internal/usecase/keys"
11	"github.com/bnema/vev/internal/usecase/layout"	11		"github.com/bnema/vev/internal/usecase/layout"
		12	+	"github.com/bnema/vev/internal/usecase/picker"
12	)	13		)
13		14
14	func TestResolveOverflowObeysAxisConfigurationAndWalls(t *testing.T) {	15		func TestResolveOverflowObeysAxisConfigurationAndWalls(t *testing.T) {

@@ -196,6 +197,67 @@ func TestVerticalOverflowRevalidatesFloatingSourceBeforeHandoff(t *testing.T) {
196	charlie.mu.Unlock()	197		charlie.mu.Unlock()
197	}	198		}
198		199
		200	+	func TestVerticalOverflowRejectsStaleSourceTab(t *testing.T) {
		201	+	d, alpha, ac, _ := newManualSessionWithPTYs(t, nil, nil)
		202	+	alpha.mu.Lock()
		203	+	alpha.name = "alpha"
		204	+	expectedSource := alpha.tabs[0]
		205	+	alpha.active = 1
		206	+	alpha.mu.Unlock()
		207	+	charlie := &session{id: "live-charlie", name: "charlie", ctx: t.Context(), cancel: func() {}, tabs: []*tab{newTab(nil, domain.Size{Cols: 41, Rows: 10})}}
		208	+	d.mu.Lock()
		209	+	d.sessions[charlie.id] = charlie
		210	+	d.mu.Unlock()
		211	+
		212	+	require.ErrorIs(t, d.commitSessionOverflow(alpha, ac, expectedSource, picker.Target{Session: charlie.id, TabIndex: -1}), errNoNeighbor)
		213	+	require.Same(t, alpha, ac.currentSession())
		214	+	alpha.mu.Lock()
		215	+	require.Same(t, ac, alpha.client)
		216	+	alpha.mu.Unlock()
		217	+	charlie.mu.Lock()
		218	+	require.Nil(t, charlie.client)
		219	+	charlie.mu.Unlock()
		220	+	}
		221	+
		222	+	func TestOrdinarySessionSwitchDoesNotApplyOverflowGuard(t *testing.T) {
		223	+	d, alpha, ac, _ := newManualSessionWithPTYs(t, nil)
		224	+	installTestFloating(alpha.tabs[0], newPane("floating", nil, domain.Size{Cols: 20, Rows: 5}), true)
		225	+	charlie := &session{id: "live-charlie", name: "charlie", ctx: t.Context(), cancel: func() {}, tabs: []*tab{newTab(nil, domain.Size{Cols: 41, Rows: 10})}}
		226	+	d.mu.Lock()
		227	+	d.sessions[charlie.id] = charlie
		228	+	d.mu.Unlock()
		229	+
		230	+	require.NoError(t, d.switchToTarget(alpha, ac, picker.Target{Session: charlie.id, TabIndex: -1}))
		231	+	require.Same(t, charlie, ac.currentSession())
		232	+	}
		233	+
		234	+	func TestVerticalOverflowTreatsDisplacedSourceClientAsNoNeighbor(t *testing.T) {
		235	+	d, alpha, ac, _ := newManualSessionWithPTYs(t, nil)
		236	+	alpha.mu.Lock()
		237	+	alpha.name = "alpha"
		238	+	alpha.mu.Unlock()
		239	+	charlie := &session{id: "live-charlie", name: "charlie", ctx: t.Context(), cancel: func() {}, tabs: []*tab{newTab(nil, domain.Size{Cols: 41, Rows: 10})}}
		240	+	d.mu.Lock()
		241	+	d.sessions[charlie.id] = charlie
		242	+	d.mu.Unlock()
		243	+
		244	+	target, ok := d.prepareSessionOverflow(alpha, layout.Down, domain.NavConfig{OverflowSessions: true})
		245	+	require.True(t, ok)
		246	+	replacement := &attachedClient{}
		247	+	alpha.mu.Lock()
		248	+	alpha.client = replacement
		249	+	alpha.mu.Unlock()
		250	+
		251	+	require.ErrorIs(t, d.commitSessionOverflow(alpha, ac, alpha.tabs[0], target), errNoNeighbor)
		252	+	require.Same(t, alpha, ac.currentSession())
		253	+	alpha.mu.Lock()
		254	+	require.Same(t, replacement, alpha.client)
		255	+	alpha.mu.Unlock()
		256	+	charlie.mu.Lock()
		257	+	require.Nil(t, charlie.client)
		258	+	charlie.mu.Unlock()
		259	+	}
		260	+
199	func TestVerticalOverflowIsRaceFreeDuringSessionRename(t *testing.T) {	261		func TestVerticalOverflowIsRaceFreeDuringSessionRename(t *testing.T) {
200	d, alpha, ac, _ := newManualSessionWithPTYs(t, nil)	262		d, alpha, ac, _ := newManualSessionWithPTYs(t, nil)
201	alpha.mu.Lock()	263		alpha.mu.Lock()

M internal/usecase/daemon/picker.go +39 -4


@@ -341,10 +341,23 @@ func (d Daemon) sessionByID(id domain.SessionID) session {
341	341		return d.sessions[id]
342	342		}
343	343
	344	+	// sessionHandoffGuard optionally constrains an active-session handoff to the
	345	+	// tab that initiated navigation. Its zero value preserves ordinary picker and
	346	+	// command switching behavior.
	347	+	type sessionHandoffGuard struct {
	348	+	expectedSource *tab
	349	+	}
	350	+
344	351		// switchToTarget resolves named lifecycle targets and commits their transition
345	352		// while d.mu is held. A named palette result is allowed to cross an
346	353		// active/stopped transition, but it never follows a same-name replacement.
347	354		func (d Daemon) switchToTarget(from session, ac *attachedClient, target picker.Target) error {
	355	+	return d.switchToTargetGuarded(from, ac, target, sessionHandoffGuard{})
	356	+	}
	357	+
	358	+	// switchToTargetGuarded is the navigation-only variant whose guard is checked
	359	+	// as part of the source ownership transfer.
	360	+	func (d Daemon) switchToTargetGuarded(from session, ac *attachedClient, target picker.Target, guard sessionHandoffGuard) error {
348	361		d.mu.Lock()
349	362		var (
350	363		targetSess *session

@@ -363,15 +376,18 @@ func (d Daemon) switchToTarget(from session, ac *attachedClient, target picker
363	376		// locked name/lifecycle resolver chose this active incarnation.
364	377		resolvedTarget := target
365	378		resolvedTarget.Session = active.id
366		-	targetSess, old, cleanups, switched = d.switchToActiveTargetLocked(from, ac, active, resolvedTarget)
	379	+	targetSess, old, cleanups, switched = d.switchToActiveTargetLocked(from, ac, active, resolvedTarget, guard)
367	380		}
368	381		}
369	382		} else if active := d.sessions[target.Session]; active != nil {
370		-	targetSess, old, cleanups, switched = d.switchToActiveTargetLocked(from, ac, active, target)
	383	+	targetSess, old, cleanups, switched = d.switchToActiveTargetLocked(from, ac, active, target, guard)
371	384		}
372	385		d.mu.Unlock()
373	386
374	387		if !switched {
	388	+	if guard.expectedSource != nil {
	389	+	return errNoNeighbor
	390	+	}
375	391		d.invalidateRender(from, ac, true, "picker.go")
376	392		return domain.UserErr(domain.NoticeSessionUnavailable, "couldn't switch to that session", cause)
377	393		}

@@ -413,7 +429,7 @@ func (d Daemon) resolveNamedLifecycleTargetLocked(target picker.Target) (sessi
413	429
414	430		// switchToActiveTargetLocked commits an active target handoff. Caller holds
415	431		// d.mu and releases it before completing render cleanup or first paint.
416		-	func (d Daemon) switchToActiveTargetLocked(from session, ac attachedClient, targetSess session, target picker.Target) (session, attachedClient, []renderLifecycleCleanup, bool) {
	432	+	func (d Daemon) switchToActiveTargetLocked(from session, ac attachedClient, targetSess session, target picker.Target, guard sessionHandoffGuard) (session, attachedClient, []renderLifecycleCleanup, bool) {
417	433		// The caller already holds d.mu. Keep handoff ownership atomic with global
418	434		// routing so it cannot select an attachment midway between sessions.
419	435		d.notices.routingMu.Lock()

@@ -443,10 +459,29 @@ func (d Daemon) switchToActiveTargetLocked(from session, ac *attachedClient, t
443	459		targetSess.mu.Unlock()
444	460		return nil, nil, nil, false
445	461		}
	462	+	var guardedSource *tab
	463	+	if guard.expectedSource != nil {
	464	+	if from.active < 0 \|\| from.active >= len(from.tabs) \|\| from.tabs[from.active] != guard.expectedSource {
	465	+	from.mu.Unlock()
	466	+	targetSess.mu.Unlock()
	467	+	return nil, nil, nil, false
	468	+	}
	469	+	guardedSource = guard.expectedSource
	470	+	guardedSource.mu.Lock()
	471	+	if guardedSource.floating.state == floatingVisible {
	472	+	guardedSource.mu.Unlock()
	473	+	from.mu.Unlock()
	474	+	targetSess.mu.Unlock()
	475	+	return nil, nil, nil, false
	476	+	}
	477	+	}
446	478		term := from.terminal
447	479		env := copyEnvironment(from.env)
448	480		from.client = nil
449	481		ac.setSession(nil)
	482	+	if guardedSource != nil {
	483	+	guardedSource.mu.Unlock()
	484	+	}
450	485		from.mu.Unlock()
451	486
452	487		old := targetSess.client

@@ -513,7 +548,7 @@ func targetMatchesLifecycle(target picker.Target, name string, createdAt int64)
513	548		// targets must use switchToTarget so resolution and commit share d.mu.
514	549		func (d Daemon) stealClientForTarget(from session, ac attachedClient, targetSess session, target picker.Target) *attachedClient {
515	550		d.mu.Lock()
516		-	_, old, cleanups, switched := d.switchToActiveTargetLocked(from, ac, targetSess, target)
	551	+	_, old, cleanups, switched := d.switchToActiveTargetLocked(from, ac, targetSess, target, sessionHandoffGuard{})
517	552		d.mu.Unlock()
518	553		if !switched {
519	554		return nil

 	return d.sessions[id]
 }
+// sessionHandoffGuard optionally constrains an active-session handoff to the
+// tab that initiated navigation. Its zero value preserves ordinary picker and
+// command switching behavior.
+type sessionHandoffGuard struct {
+	expectedSource *tab
+}
 // switchToTarget resolves named lifecycle targets and commits their transition
 // while d.mu is held. A named palette result is allowed to cross an
 // active/stopped transition, but it never follows a same-name replacement.
 func (d *Daemon) switchToTarget(from *session, ac *attachedClient, target picker.Target) error {
+	return d.switchToTargetGuarded(from, ac, target, sessionHandoffGuard{})
+}
+// switchToTargetGuarded is the navigation-only variant whose guard is checked
+// as part of the source ownership transfer.
+func (d *Daemon) switchToTargetGuarded(from *session, ac *attachedClient, target picker.Target, guard sessionHandoffGuard) error {
 	d.mu.Lock()
 	var (
 		targetSess *session
 				// locked name/lifecycle resolver chose this active incarnation.
 				resolvedTarget := target
 				resolvedTarget.Session = active.id
-				targetSess, old, cleanups, switched = d.switchToActiveTargetLocked(from, ac, active, resolvedTarget)
+				targetSess, old, cleanups, switched = d.switchToActiveTargetLocked(from, ac, active, resolvedTarget, guard)
 			}
 		}
 	} else if active := d.sessions[target.Session]; active != nil {
-		targetSess, old, cleanups, switched = d.switchToActiveTargetLocked(from, ac, active, target)
+		targetSess, old, cleanups, switched = d.switchToActiveTargetLocked(from, ac, active, target, guard)
 	}
 	d.mu.Unlock()
 	if !switched {
+		if guard.expectedSource != nil {
+			return errNoNeighbor
+		}
 		d.invalidateRender(from, ac, true, "picker.go")
 		return domain.UserErr(domain.NoticeSessionUnavailable, "couldn't switch to that session", cause)
 	}
 // switchToActiveTargetLocked commits an active target handoff. Caller holds
 // d.mu and releases it before completing render cleanup or first paint.
-func (d *Daemon) switchToActiveTargetLocked(from *session, ac *attachedClient, targetSess *session, target picker.Target) (*session, *attachedClient, []renderLifecycleCleanup, bool) {
+func (d *Daemon) switchToActiveTargetLocked(from *session, ac *attachedClient, targetSess *session, target picker.Target, guard sessionHandoffGuard) (*session, *attachedClient, []renderLifecycleCleanup, bool) {
 	// The caller already holds d.mu. Keep handoff ownership atomic with global
 	// routing so it cannot select an attachment midway between sessions.
 	d.notices.routingMu.Lock()
 		targetSess.mu.Unlock()
 		return nil, nil, nil, false
 	}
+	var guardedSource *tab
+	if guard.expectedSource != nil {
+		if from.active < 0 || from.active >= len(from.tabs) || from.tabs[from.active] != guard.expectedSource {
+			from.mu.Unlock()
+			targetSess.mu.Unlock()
+			return nil, nil, nil, false
+		}
+		guardedSource = guard.expectedSource
+		guardedSource.mu.Lock()
+		if guardedSource.floating.state == floatingVisible {
+			guardedSource.mu.Unlock()
+			from.mu.Unlock()
+			targetSess.mu.Unlock()
+			return nil, nil, nil, false
+		}
+	}
 	term := from.terminal
 	env := copyEnvironment(from.env)
 	from.client = nil
 	ac.setSession(nil)
+	if guardedSource != nil {
+		guardedSource.mu.Unlock()
+	}
 	from.mu.Unlock()
 	old := targetSess.client
 // targets must use switchToTarget so resolution and commit share d.mu.
 func (d *Daemon) stealClientForTarget(from *session, ac *attachedClient, targetSess *session, target picker.Target) *attachedClient {
 	d.mu.Lock()
-	_, old, cleanups, switched := d.switchToActiveTargetLocked(from, ac, targetSess, target)
+	_, old, cleanups, switched := d.switchToActiveTargetLocked(from, ac, targetSess, target, sessionHandoffGuard{})
 	d.mu.Unlock()
 	if !switched {
 		return nil