17667bd fix(remote): fence remote picker transitions · vev-vt

M internal/usecase/daemon/coderabbit_regression_test.go +1 -1


@@ -270,7 +270,7 @@ func TestShutdownWithoutDeadlineBoundsUncooperativeRemoteConstruction(t *testing
270	270		go func() { result <- d.shutdownAll(ports.ReasonServerShutdown) }()
271	271		timer := awaitTestValue(t, clock.timers, "remote construction shutdown timer")
272	272		require.Equal(t, remoteConstructionShutdownGrace, timer.duration)
273		-	timer.ch <- time.Now()
	273	+	timer.ch <- time.Time{}
274	274		require.True(t, awaitTestValue(t, result, "shutdown blocked on uncooperative remote construction"))
275	275		}
276	276


@@ -270,7 +270,7 @@ func TestShutdownWithoutDeadlineBoundsUncooperativeRemoteConstruction(t *testing
270		go func() { result <- d.shutdownAll(ports.ReasonServerShutdown) }()	270		go func() { result <- d.shutdownAll(ports.ReasonServerShutdown) }()
271		timer := awaitTestValue(t, clock.timers, "remote construction shutdown timer")	271		timer := awaitTestValue(t, clock.timers, "remote construction shutdown timer")
272		require.Equal(t, remoteConstructionShutdownGrace, timer.duration)	272		require.Equal(t, remoteConstructionShutdownGrace, timer.duration)
273	-	timer.ch <- time.Now()	273	+	timer.ch <- time.Time{}
274		require.True(t, awaitTestValue(t, result, "shutdown blocked on uncooperative remote construction"))	274		require.True(t, awaitTestValue(t, result, "shutdown blocked on uncooperative remote construction"))
275		}	275		}
276			276

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


@@ -841,6 +841,7 @@ func (d Daemon) shutdownAllWithSnapshotDeadline(reason uint8, deadline snapsho
841	841		// observes d.closing and retires its unpublished candidate independently.
842	842		var constructionTimer ports.Timer
843	843		var constructionTimeout <-chan time.Time
	844	+	graceExpired := false
844	845		if deadline == nil && len(constructions) != 0 {
845	846		clock := d.clock
846	847		if clock == nil {

@@ -858,10 +859,14 @@ func (d Daemon) shutdownAllWithSnapshotDeadline(reason uint8, deadline snapsho
858	859		continue
859	860		}
860	861		if deadline == nil {
	862	+	if graceExpired {
	863	+	continue
	864	+	}
861	865		select {
862	866		case <-construction.done:
863	867		case <-constructionTimeout:
864	868		checkpointIncomplete = true
	869	+	graceExpired = true
865	870		}
866	871		continue
867	872		}


@@ -841,6 +841,7 @@ func (d Daemon) shutdownAllWithSnapshotDeadline(reason uint8, deadline snapsho
841	// observes d.closing and retires its unpublished candidate independently.	841		// observes d.closing and retires its unpublished candidate independently.
842	var constructionTimer ports.Timer	842		var constructionTimer ports.Timer
843	var constructionTimeout <-chan time.Time	843		var constructionTimeout <-chan time.Time
		844	+	graceExpired := false
844	if deadline == nil && len(constructions) != 0 {	845		if deadline == nil && len(constructions) != 0 {
845	clock := d.clock	846		clock := d.clock
846	if clock == nil {	847		if clock == nil {

@@ -858,10 +859,14 @@ func (d Daemon) shutdownAllWithSnapshotDeadline(reason uint8, deadline snapsho
858	continue	859		continue
859	}	860		}
860	if deadline == nil {	861		if deadline == nil {
		862	+	if graceExpired {
		863	+	continue
		864	+	}
861	select {	865		select {
862	case <-construction.done:	866		case <-construction.done:
863	case <-constructionTimeout:	867		case <-constructionTimeout:
864	checkpointIncomplete = true	868		checkpointIncomplete = true
		869	+	graceExpired = true
865	}	870		}
866	continue	871		continue
867	}	872		}

M internal/usecase/daemon/picker.go +5 -2


@@ -423,7 +423,7 @@ func (d Daemon) handlePickerInput(ac attachedClient, data []byte, effects ...*
423	423		if len(effects) != 0 && effects[0] != nil {
424	424		effect = effects[0]
425	425		} else {
426		-	token := sess.attachmentToken(ac, ac.transport())
	426	+	token := attachmentOwnerToken(ac.currentAttachmentOwner(), ac, ac.transport())
427	427		var admitted bool
428	428		effect, admitted = ac.beginAttachmentEffect(token)
429	429		if !admitted {

@@ -946,9 +946,12 @@ func (d *Daemon) switchToTargetForAttachment(token attachmentConnectionToken, ta
946	946		}
947	947		return nil
948	948		}
949		-	if token.localSession() == nil \|\| token.effect == nil {
	949	+	if token.localSession() == nil {
950	950		return nil
951	951		}
	952	+	if token.effect == nil {
	953	+	return errAttachmentTransition
	954	+	}
952	955		if target.RemoteTarget != nil {
953	956		return d.startRemotePickerSelection(token, target, guard, action)
954	957		}

 			if len(effects) != 0 && effects[0] != nil {
 				effect = effects[0]
 			} else {
-				token := sess.attachmentToken(ac, ac.transport())
+				token := attachmentOwnerToken(ac.currentAttachmentOwner(), ac, ac.transport())
 				var admitted bool
 				effect, admitted = ac.beginAttachmentEffect(token)
 				if !admitted {
 		}
 		return nil
 	}
-	if token.localSession() == nil || token.effect == nil {
+	if token.localSession() == nil {
 		return nil
 	}
+	if token.effect == nil {
+		return errAttachmentTransition
+	}
 	if target.RemoteTarget != nil {
 		return d.startRemotePickerSelection(token, target, guard, action)
 	}

M internal/usecase/daemon/remote_attachment_transition_back_test.go +13 -0


@@ -39,6 +39,19 @@ func TestBackSessionForAttachmentReturnsFromRemoteViewToPreviousLocal(t *testing
39	39		require.NotNil(t, current.lease, "returning to a local owner restores its coordinator lease")
40	40		}
41	41
	42	+	func TestBackSessionForAttachmentReturnsToPreviousRemoteView(t *testing.T) {
	43	+	d, local, ac, view, remoteToken := remoteBackFixture(t)
	44	+
	45	+	require.NoError(t, d.backSessionForAttachment(remoteToken))
	46	+	localToken := local.attachmentToken(ac, ac.transport())
	47	+	require.NoError(t, d.backSessionForAttachment(localToken))
	48	+
	49	+	require.Same(t, view, ac.currentAttachmentOwner())
	50	+	require.True(t, view.attachmentRegistered(ac))
	51	+	require.False(t, local.attachmentRegistered(ac))
	52	+	require.Same(t, local, ac.previousOwner.Get())
	53	+	}
	54	+
42	55		func TestRemoteRecentAppearsInLocalStatusMRUAfterReturn(t *testing.T) {
43	56		d, local, ac, _, token := remoteBackFixture(t)
44	57


@@ -39,6 +39,19 @@ func TestBackSessionForAttachmentReturnsFromRemoteViewToPreviousLocal(t *testing
39	require.NotNil(t, current.lease, "returning to a local owner restores its coordinator lease")	39		require.NotNil(t, current.lease, "returning to a local owner restores its coordinator lease")
40	}	40		}
41		41
		42	+	func TestBackSessionForAttachmentReturnsToPreviousRemoteView(t *testing.T) {
		43	+	d, local, ac, view, remoteToken := remoteBackFixture(t)
		44	+
		45	+	require.NoError(t, d.backSessionForAttachment(remoteToken))
		46	+	localToken := local.attachmentToken(ac, ac.transport())
		47	+	require.NoError(t, d.backSessionForAttachment(localToken))
		48	+
		49	+	require.Same(t, view, ac.currentAttachmentOwner())
		50	+	require.True(t, view.attachmentRegistered(ac))
		51	+	require.False(t, local.attachmentRegistered(ac))
		52	+	require.Same(t, local, ac.previousOwner.Get())
		53	+	}
		54	+
42	func TestRemoteRecentAppearsInLocalStatusMRUAfterReturn(t *testing.T) {	55		func TestRemoteRecentAppearsInLocalStatusMRUAfterReturn(t *testing.T) {
43	d, local, ac, _, token := remoteBackFixture(t)	56		d, local, ac, _, token := remoteBackFixture(t)
44		57

M internal/usecase/daemon/remote_picker_selection.go +1 -1


@@ -79,7 +79,7 @@ func (d *Daemon) startRemotePickerSelection(token attachmentConnectionToken, tar
79	79		token.effect.bindActionEnd(d, action)
80	80		}
81	81		d.invalidateAttachedOwner(token.ac, true, "remote_picker_selection.go")
82		-	go d.completeRemotePickerSelection(selection)
	82	+	d.sessWg.Go(func() { d.completeRemotePickerSelection(selection) })
83	83		return nil
84	84		}
85	85


@@ -79,7 +79,7 @@ func (d *Daemon) startRemotePickerSelection(token attachmentConnectionToken, tar
79		token.effect.bindActionEnd(d, action)	79		token.effect.bindActionEnd(d, action)
80		}	80		}
81		d.invalidateAttachedOwner(token.ac, true, "remote_picker_selection.go")	81		d.invalidateAttachedOwner(token.ac, true, "remote_picker_selection.go")
82	-	go d.completeRemotePickerSelection(selection)	82	+	d.sessWg.Go(func() { d.completeRemotePickerSelection(selection) })
83		return nil	83		return nil
84		}	84		}
85			85

M internal/usecase/daemon/session_back.go +11 -2


@@ -45,10 +45,19 @@ func (d *Daemon) backSessionForAttachment(token attachmentConnectionToken, after
45	45		return nil
46	46		}
47	47		previous := token.ac.previousOwner.Get()
	48	+	if view, remote := normalizeAttachmentOwner(previous).(*remoteView); remote {
	49	+	published, err := d.transitionToRemoteView(token, view)
	50	+	if err != nil {
	51	+	if !token.current() {
	52	+	return nil
	53	+	}
	54	+	return err
	55	+	}
	56	+	d.firstPaintForTransition(published)
	57	+	return nil
	58	+	}
48	59		target := localSession(previous)
49	60		if target == nil {
50		-	// A remote predecessor remains stable history for the remote transition
51		-	// path; the local-only back path must not discard it.
52	61		return nil
53	62		}
54	63		if target == current \|\| target.core() == nil \|\| d.sessionByID(target.core().id) != target {


@@ -45,10 +45,19 @@ func (d *Daemon) backSessionForAttachment(token attachmentConnectionToken, after
45		return nil	45		return nil
46		}	46		}
47		previous := token.ac.previousOwner.Get()	47		previous := token.ac.previousOwner.Get()
			48	+	if view, remote := normalizeAttachmentOwner(previous).(*remoteView); remote {
			49	+	published, err := d.transitionToRemoteView(token, view)
			50	+	if err != nil {
			51	+	if !token.current() {
			52	+	return nil
			53	+	}
			54	+	return err
			55	+	}
			56	+	d.firstPaintForTransition(published)
			57	+	return nil
			58	+	}
48		target := localSession(previous)	59		target := localSession(previous)
49		if target == nil {	60		if target == nil {
50	-	// A remote predecessor remains stable history for the remote transition
51	-	// path; the local-only back path must not discard it.
52		return nil	61		return nil
53		}	62		}
54		if target == current \|\| target.core() == nil \|\| d.sessionByID(target.core().id) != target {	63		if target == current \|\| target.core() == nil \|\| d.sessionByID(target.core().id) != target {