67576a2 fix(remote): fence proxy picker lifecycle · vev

M internal/usecase/daemon/attachment_transition.go +14 -1

@@ -1,9 +1,21 @@
1	1		package daemon
2	2
3		-	import "errors"
	3	+	import (
	4	+	"errors"
	5	+
	6	+	"github.com/bnema/vev/internal/domain"
	7	+	)
4	8
5	9		var errAttachmentTransition = errors.New("attachment transition is no longer valid")
6	10
	11	+	type attachmentLifecycleFence struct {
	12	+	name string
	13	+	createdAt int64
	14	+	checkCreatedAt bool
	15	+	incarnation domain.IncarnationID
	16	+	checkIncarnation bool
	17	+	}
	18	+
7	19		type attachmentTransitionRequest struct {
8	20		source attachmentSession
9	21		target attachmentSession

@@ -29,6 +41,7 @@ type attachmentTransitionRequest struct {
29	41		ready bool
30	42		expectedTargetCurrent *attachedClient
31	43		expectedTargetTransport transportSnapshot
	44	+	expectedTargetLifecycle *attachmentLifecycleFence
32	45		preflighted bool
33	46		roleEffectsFrozen bool
34	47		expectedTargetTransportInterrupted bool

 package daemon
-import "errors"
+import (
+	"errors"
+	"github.com/bnema/vev/internal/domain"
+)
 var errAttachmentTransition = errors.New("attachment transition is no longer valid")
+type attachmentLifecycleFence struct {
+	name             string
+	createdAt        int64
+	checkCreatedAt   bool
+	incarnation      domain.IncarnationID
+	checkIncarnation bool
+}
 type attachmentTransitionRequest struct {
 	source                attachmentSession
 	target                attachmentSession
 	ready                              bool
 	expectedTargetCurrent              *attachedClient
 	expectedTargetTransport            transportSnapshot
+	expectedTargetLifecycle            *attachmentLifecycleFence
 	preflighted                        bool
 	roleEffectsFrozen                  bool
 	expectedTargetTransportInterrupted bool

M internal/usecase/daemon/attachment_transition_publish.go +15 -0


@@ -94,6 +94,20 @@ func (p *attachmentPublication) unlockCoordinators() {
94	94		p.releaseCoordinators()
95	95		}
96	96
	97	+	func attachmentLifecycleCurrentLocked(entry attachmentSession, fence *attachmentLifecycleFence) bool {
	98	+	if fence == nil {
	99	+	return true
	100	+	}
	101	+	target, ok := localSession(entry)
	102	+	if !ok {
	103	+	return false
	104	+	}
	105	+	if fence.checkCreatedAt && (target.name != fence.name \|\| target.createdAt != fence.createdAt) {
	106	+	return false
	107	+	}
	108	+	return !fence.checkIncarnation \|\| target.incarnation == fence.incarnation
	109	+	}
	110	+
97	111		// validateAttachmentTransitionPrelocked performs every fallible transition
98	112		// check without acquiring d.mu, notices.routingMu, or either session lock.
99	113		// The caller holds those locks in that order, with session locks ordered by ID.

@@ -143,6 +157,7 @@ func (d *Daemon) validateAttachmentTransitionPrelocked(req attachmentTransitionR
143	157		invalid = invalid \|\| targetCoordinator != nil && !req.preserveRole && !targetCoordinator.canReplaceLocked(old, req.next)
144	158		invalid = invalid \|\| req.sourceToken == nil && source != req.target && req.expectedRole == attachmentActive && sourceCoordinator != nil &&
145	159		(sourceCoordinator.lease == nil \|\| !sourceCoordinator.lease.active \|\| sourceCoordinator.lease.attachment != req.next)
	160	+	invalid = invalid \|\| !attachmentLifecycleCurrentLocked(req.target, req.expectedTargetLifecycle)
146	161		invalid = invalid \|\| req.activateTargetTab && !req.target.activateTargetLocked(req.targetTabIndex)
147	162		if invalid {
148	163		publication.unlockCoordinators()


@@ -94,6 +94,20 @@ func (p *attachmentPublication) unlockCoordinators() {
94	p.releaseCoordinators()	94		p.releaseCoordinators()
95	}	95		}
96		96
		97	+	func attachmentLifecycleCurrentLocked(entry attachmentSession, fence *attachmentLifecycleFence) bool {
		98	+	if fence == nil {
		99	+	return true
		100	+	}
		101	+	target, ok := localSession(entry)
		102	+	if !ok {
		103	+	return false
		104	+	}
		105	+	if fence.checkCreatedAt && (target.name != fence.name \|\| target.createdAt != fence.createdAt) {
		106	+	return false
		107	+	}
		108	+	return !fence.checkIncarnation \|\| target.incarnation == fence.incarnation
		109	+	}
		110	+
97	// validateAttachmentTransitionPrelocked performs every fallible transition	111		// validateAttachmentTransitionPrelocked performs every fallible transition
98	// check without acquiring d.mu, notices.routingMu, or either session lock.	112		// check without acquiring d.mu, notices.routingMu, or either session lock.
99	// The caller holds those locks in that order, with session locks ordered by ID.	113		// The caller holds those locks in that order, with session locks ordered by ID.

@@ -143,6 +157,7 @@ func (d *Daemon) validateAttachmentTransitionPrelocked(req attachmentTransitionR
143	invalid = invalid \|\| targetCoordinator != nil && !req.preserveRole && !targetCoordinator.canReplaceLocked(old, req.next)	157		invalid = invalid \|\| targetCoordinator != nil && !req.preserveRole && !targetCoordinator.canReplaceLocked(old, req.next)
144	invalid = invalid \|\| req.sourceToken == nil && source != req.target && req.expectedRole == attachmentActive && sourceCoordinator != nil &&	158		invalid = invalid \|\| req.sourceToken == nil && source != req.target && req.expectedRole == attachmentActive && sourceCoordinator != nil &&
145	(sourceCoordinator.lease == nil \|\| !sourceCoordinator.lease.active \|\| sourceCoordinator.lease.attachment != req.next)	159		(sourceCoordinator.lease == nil \|\| !sourceCoordinator.lease.active \|\| sourceCoordinator.lease.attachment != req.next)
		160	+	invalid = invalid \|\| !attachmentLifecycleCurrentLocked(req.target, req.expectedTargetLifecycle)
146	invalid = invalid \|\| req.activateTargetTab && !req.target.activateTargetLocked(req.targetTabIndex)	161		invalid = invalid \|\| req.activateTargetTab && !req.target.activateTargetLocked(req.targetTabIndex)
147	if invalid {	162		if invalid {
148	publication.unlockCoordinators()	163		publication.unlockCoordinators()

M internal/usecase/daemon/picker.go +19 -7


@@ -581,18 +581,29 @@ func (d *Daemon) switchActiveTargetForRole(token attachmentRoleToken, target pic
581	581		return d.switchActiveTargetForRoleGuarded(token, target, sessionHandoffGuard{}, "jump-attention")
582	582		}
583	583
	584	+	func pickerTargetLifecycleFence(target picker.Target) *attachmentLifecycleFence {
	585	+	fence := &attachmentLifecycleFence{}
	586	+	if target.ExpectedCreatedAt != nil {
	587	+	fence.name = target.Name
	588	+	fence.createdAt = *target.ExpectedCreatedAt
	589	+	fence.checkCreatedAt = true
	590	+	}
	591	+	if target.Incarnation != (domain.IncarnationID{}) {
	592	+	fence.incarnation = target.Incarnation
	593	+	fence.checkIncarnation = true
	594	+	}
	595	+	if !fence.checkCreatedAt && !fence.checkIncarnation {
	596	+	return nil
	597	+	}
	598	+	return fence
	599	+	}
	600	+
584	601		func (d *Daemon) switchActiveTargetForRoleGuarded(token attachmentRoleToken, target picker.Target, guard sessionHandoffGuard, action string) error {
585	602		if token.sess == nil \|\| token.ac == nil \|\| token.role != attachmentActive {
586	603		return nil
587	604		}
588	605		d.mu.Lock()
589	606		targetSess, ok := localSession(d.sessions[target.Session])
590		-	if ok {
591		-	targetSess.mu.Lock()
592		-	ok = targetMatchesLifecycle(target, targetSess.name, targetSess.createdAt) &&
593		-	(target.Incarnation == (domain.IncarnationID{}) \|\| target.Incarnation == targetSess.incarnation)
594		-	targetSess.mu.Unlock()
595		-	}
596	607		d.mu.Unlock()
597	608		if !ok \|\| targetSess == token.sess {
598	609		return nil

@@ -602,7 +613,8 @@ func (d *Daemon) switchActiveTargetForRoleGuarded(token attachmentRoleToken, tar
602	613		source: token.sess, target: targetSess, next: token.ac,
603	614		expectedRole: attachmentActive, targetRole: attachmentActive,
604	615		expectedTransport: token.transport, sourceToken: &token, action: action,
605		-	activateTargetTab: true, targetTabIndex: target.TabIndex, copySourceEnvironment: true, ready: true,
	616	+	expectedTargetLifecycle: pickerTargetLifecycleFence(target),
	617	+	activateTargetTab: true, targetTabIndex: target.TabIndex, copySourceEnvironment: true, ready: true,
606	618		})
607	619		if err != nil {
608	620		// Losing the exact source role is a benign stale action, not a notice for

 	return d.switchActiveTargetForRoleGuarded(token, target, sessionHandoffGuard{}, "jump-attention")
 }
+func pickerTargetLifecycleFence(target picker.Target) *attachmentLifecycleFence {
+	fence := &attachmentLifecycleFence{}
+	if target.ExpectedCreatedAt != nil {
+		fence.name = target.Name
+		fence.createdAt = *target.ExpectedCreatedAt
+		fence.checkCreatedAt = true
+	}
+	if target.Incarnation != (domain.IncarnationID{}) {
+		fence.incarnation = target.Incarnation
+		fence.checkIncarnation = true
+	}
+	if !fence.checkCreatedAt && !fence.checkIncarnation {
+		return nil
+	}
+	return fence
+}
 func (d *Daemon) switchActiveTargetForRoleGuarded(token attachmentRoleToken, target picker.Target, guard sessionHandoffGuard, action string) error {
 	if token.sess == nil || token.ac == nil || token.role != attachmentActive {
 		return nil
 	}
 	d.mu.Lock()
 	targetSess, ok := localSession(d.sessions[target.Session])
-	if ok {
-		targetSess.mu.Lock()
-		ok = targetMatchesLifecycle(target, targetSess.name, targetSess.createdAt) &&
-			(target.Incarnation == (domain.IncarnationID{}) || target.Incarnation == targetSess.incarnation)
-		targetSess.mu.Unlock()
-	}
 	d.mu.Unlock()
 	if !ok || targetSess == token.sess {
 		return nil
 		source: token.sess, target: targetSess, next: token.ac,
 		expectedRole: attachmentActive, targetRole: attachmentActive,
 		expectedTransport: token.transport, sourceToken: &token, action: action,
-		activateTargetTab: true, targetTabIndex: target.TabIndex, copySourceEnvironment: true, ready: true,
+		expectedTargetLifecycle: pickerTargetLifecycleFence(target),
+		activateTargetTab:       true, targetTabIndex: target.TabIndex, copySourceEnvironment: true, ready: true,
 	})
 	if err != nil {
 		// Losing the exact source role is a benign stale action, not a notice for

M internal/usecase/daemon/proxy_input_test.go +8 -5


@@ -722,12 +722,15 @@ func TestRemotePickerRejectsReplacedLocalLifecycle(t *testing.T) {
722	722		model := d.newPickerModel(proxy, pickerNavigate, moveSourceLocator{}, picker.SourceFilter{Session: local.id, TabID: domain.TabStableID(tb.stableID)})
723	723		target, selectable := model.Selected()
724	724		require.True(t, selectable)
725		-	local.mu.Lock()
726		-	local.name = "replacement"
727		-	local.createdAt++
728		-	local.mu.Unlock()
	725	+	d.afterRoleEffectsFrozen = func() {
	726	+	d.afterRoleEffectsFrozen = nil
	727	+	local.mu.Lock()
	728	+	local.name = "replacement"
	729	+	local.createdAt++
	730	+	local.mu.Unlock()
	731	+	}
729	732
730		-	require.NoError(t, d.switchToTargetForRole(handler.roleToken, target, sessionHandoffGuard{}, "picker-select"))
	733	+	require.Error(t, d.switchToTargetForRole(handler.roleToken, target, sessionHandoffGuard{}, "picker-select"))
731	734		require.Same(t, proxy, ac.currentAttachmentSession(), "stale picker lifecycle must not redirect the attachment")
732	735		}
733	736

 	model := d.newPickerModel(proxy, pickerNavigate, moveSourceLocator{}, picker.SourceFilter{Session: local.id, TabID: domain.TabStableID(tb.stableID)})
 	target, selectable := model.Selected()
 	require.True(t, selectable)
-	local.mu.Lock()
+	d.afterRoleEffectsFrozen = func() {
-	local.name = "replacement"
+		d.afterRoleEffectsFrozen = nil
-	local.createdAt++
+		local.mu.Lock()
-	local.mu.Unlock()
+		local.name = "replacement"
+		local.createdAt++
+		local.mu.Unlock()
+	}
-	require.NoError(t, d.switchToTargetForRole(handler.roleToken, target, sessionHandoffGuard{}, "picker-select"))
+	require.Error(t, d.switchToTargetForRole(handler.roleToken, target, sessionHandoffGuard{}, "picker-select"))
 	require.Same(t, proxy, ac.currentAttachmentSession(), "stale picker lifecycle must not redirect the attachment")
 }