fix(app): handle local picker handoffs

commit: 7739dc9ef05c3f237b14bb835e60f43d6bba5b57
author: brice · 2mo ago · 2026-08-03 13:19 UTC
committer: brice · 2mo ago · 2026-08-04 07:16 UTC
parent: dea913c
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

2 files · +101 -37

M internal/app/run.go +59 -35


@@ -821,6 +821,12 @@ func remoteDiscoveryDaemonOption(stateDir string, observer ports.SerializedRunti
821	821		), nil
822	822		}
823	823
	824	+	type attachHandoffKey struct {
	825	+	endpoint string
	826	+	session string
	827	+	intent uint8
	828	+	}
	829	+
824	830		func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, activeSession string, log *slog.Logger, deps runAttachDeps) error {
825	831		if activeSession != "" {
826	832		if remoteTarget == "" && intent == ports.IntentNew {

@@ -833,22 +839,32 @@ func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, ac
833	839		if runClient == nil {
834	840		runClient = runClientWithDeps
835	841		}
	842	+	localDialer := deps.localDialer
	843	+	if localDialer == nil {
	844	+	localDialer = defaultLocalDialer
	845	+	}
	846	+	mode, modeErr := remoteTransportModeFromEnv(deps.selectedRemoteTransport)
	847	+	factory := deps.remoteDialerFactory
	848	+	seenHandoffs := make(map[attachHandoffKey]struct{})
836	849		if remoteTarget != "" {
837		-	mode, err := remoteTransportModeFromEnv(deps.selectedRemoteTransport)
838		-	if err != nil {
839		-	return err
	850	+	if modeErr != nil {
	851	+	return modeErr
840	852		}
841		-	factory := deps.remoteDialerFactory
842	853		if factory == nil {
843	854		factory = defaultRemoteDialerFactory()
844	855		}
845		-	for {
	856	+	seenHandoffs[attachHandoffKey{endpoint: remoteTarget, session: name, intent: intent}] = struct{}{}
	857	+	}
	858	+
	859	+	for {
	860	+	var err error
	861	+	if remoteTarget != "" {
846	862		if log != nil {
847	863		log.Info("attaching to remote session", "target", remoteTarget, "name", name, "transport", string(mode))
848	864		}
849		-	dialer, err := factory.DialerForRemote(remoteTarget, name, mode, log)
850		-	if err != nil {
851		-	return err
	865	+	dialer, dialErr := factory.DialerForRemote(remoteTarget, name, mode, log)
	866	+	if dialErr != nil {
	867	+	return dialErr
852	868		}
853	869		err = runClient(ctx, client.Dependencies{
854	870		Dialer: dialer,

@@ -860,37 +876,45 @@ func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, ac
860	876		RemoteHostLearner: attachRememberLearner(deps, remoteTarget, log),
861	877		Remote: true,
862	878		}, client.AttachRequest{Intent: intent, SessionName: name})
863		-	var handoff *client.AttachTargetError
864		-	if !errors.As(err, &handoff) {
865		-	return err
866		-	}
867		-	if handoff == nil {
868		-	return fmt.Errorf("vev: invalid remote attach handoff")
869		-	}
870		-	if err := validateRemoteAttachHandoff(handoff.Target); err != nil {
871		-	return fmt.Errorf("vev: invalid remote attach handoff: %w", err)
	879	+	} else {
	880	+	if log != nil {
	881	+	log.Info("attaching to local session", "intent", intent, "name", name)
872	882		}
873		-	remoteTarget = handoff.Target.Endpoint
874		-	name = handoff.Target.Session
875		-	intent = handoff.Target.Intent
	883	+	err = runClient(ctx, client.Dependencies{
	884	+	Dialer: localDialer(),
	885	+	Terminal: term.New(),
	886	+	Clock: clock.New(),
	887	+	Logger: log,
	888	+	RuntimeObserver: deps.runtimeObserver,
	889	+	Remote: false,
	890	+	}, client.AttachRequest{Intent: intent, SessionName: name})
876	891		}
877		-	}
878	892
879		-	localDialer := deps.localDialer
880		-	if localDialer == nil {
881		-	localDialer = defaultLocalDialer
	893	+	var handoff *client.AttachTargetError
	894	+	if !errors.As(err, &handoff) {
	895	+	return err
	896	+	}
	897	+	if handoff == nil {
	898	+	return fmt.Errorf("vev: invalid remote attach handoff")
	899	+	}
	900	+	if err := validateRemoteAttachHandoff(handoff.Target); err != nil {
	901	+	return fmt.Errorf("vev: invalid remote attach handoff: %w", err)
	902	+	}
	903	+	next := attachHandoffKey{endpoint: handoff.Target.Endpoint, session: handoff.Target.Session, intent: handoff.Target.Intent}
	904	+	if _, seen := seenHandoffs[next]; seen {
	905	+	return fmt.Errorf("vev: attach handoff loop for %q/%q", next.endpoint, next.session)
	906	+	}
	907	+	seenHandoffs[next] = struct{}{}
	908	+	remoteTarget = handoff.Target.Endpoint
	909	+	name = handoff.Target.Session
	910	+	intent = handoff.Target.Intent
	911	+	if modeErr != nil {
	912	+	return modeErr
	913	+	}
	914	+	if factory == nil {
	915	+	factory = defaultRemoteDialerFactory()
	916	+	}
882	917		}
883		-	if log != nil {
884		-	log.Info("attaching to local session", "intent", intent, "name", name)
885		-	}
886		-	return runClient(ctx, client.Dependencies{
887		-	Dialer: localDialer(),
888		-	Terminal: term.New(),
889		-	Clock: clock.New(),
890		-	Logger: log,
891		-	RuntimeObserver: deps.runtimeObserver,
892		-	Remote: false,
893		-	}, client.AttachRequest{Intent: intent, SessionName: name})
894	918		}
895	919
896	920		const daemonStopTimeout = 2 * time.Second

 	), nil
 }
+type attachHandoffKey struct {
+	endpoint string
+	session  string
+	intent   uint8
+}
 func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, activeSession string, log *slog.Logger, deps runAttachDeps) error {
 	if activeSession != "" {
 		if remoteTarget == "" && intent == ports.IntentNew {
 	if runClient == nil {
 		runClient = runClientWithDeps
 	}
+	localDialer := deps.localDialer
+	if localDialer == nil {
+		localDialer = defaultLocalDialer
+	}
+	mode, modeErr := remoteTransportModeFromEnv(deps.selectedRemoteTransport)
+	factory := deps.remoteDialerFactory
+	seenHandoffs := make(map[attachHandoffKey]struct{})
 	if remoteTarget != "" {
-		mode, err := remoteTransportModeFromEnv(deps.selectedRemoteTransport)
+		if modeErr != nil {
-		if err != nil {
+			return modeErr
-			return err
 		}
-		factory := deps.remoteDialerFactory
 		if factory == nil {
 			factory = defaultRemoteDialerFactory()
 		}
-		for {
+		seenHandoffs[attachHandoffKey{endpoint: remoteTarget, session: name, intent: intent}] = struct{}{}
+	}
+	for {
+		var err error
+		if remoteTarget != "" {
 			if log != nil {
 				log.Info("attaching to remote session", "target", remoteTarget, "name", name, "transport", string(mode))
 			}
-			dialer, err := factory.DialerForRemote(remoteTarget, name, mode, log)
+			dialer, dialErr := factory.DialerForRemote(remoteTarget, name, mode, log)
-			if err != nil {
+			if dialErr != nil {
-				return err
+				return dialErr
 			}
 			err = runClient(ctx, client.Dependencies{
 				Dialer:            dialer,
 				RemoteHostLearner: attachRememberLearner(deps, remoteTarget, log),
 				Remote:            true,
 			}, client.AttachRequest{Intent: intent, SessionName: name})
-			var handoff *client.AttachTargetError
+		} else {
-			if !errors.As(err, &handoff) {
+			if log != nil {
-				return err
+				log.Info("attaching to local session", "intent", intent, "name", name)
-			}
-			if handoff == nil {
-				return fmt.Errorf("vev: invalid remote attach handoff")
-			}
-			if err := validateRemoteAttachHandoff(handoff.Target); err != nil {
-				return fmt.Errorf("vev: invalid remote attach handoff: %w", err)
 			}
-			remoteTarget = handoff.Target.Endpoint
+			err = runClient(ctx, client.Dependencies{
-			name = handoff.Target.Session
+				Dialer:          localDialer(),
-			intent = handoff.Target.Intent
+				Terminal:        term.New(),
+				Clock:           clock.New(),
+				Logger:          log,
+				RuntimeObserver: deps.runtimeObserver,
+				Remote:          false,
+			}, client.AttachRequest{Intent: intent, SessionName: name})
 		}
-	}
-	localDialer := deps.localDialer
+		var handoff *client.AttachTargetError
-	if localDialer == nil {
+		if !errors.As(err, &handoff) {
-		localDialer = defaultLocalDialer
+			return err
+		}
+		if handoff == nil {
+			return fmt.Errorf("vev: invalid remote attach handoff")
+		}
+		if err := validateRemoteAttachHandoff(handoff.Target); err != nil {
+			return fmt.Errorf("vev: invalid remote attach handoff: %w", err)
+		}
+		next := attachHandoffKey{endpoint: handoff.Target.Endpoint, session: handoff.Target.Session, intent: handoff.Target.Intent}
+		if _, seen := seenHandoffs[next]; seen {
+			return fmt.Errorf("vev: attach handoff loop for %q/%q", next.endpoint, next.session)
+		}
+		seenHandoffs[next] = struct{}{}
+		remoteTarget = handoff.Target.Endpoint
+		name = handoff.Target.Session
+		intent = handoff.Target.Intent
+		if modeErr != nil {
+			return modeErr
+		}
+		if factory == nil {
+			factory = defaultRemoteDialerFactory()
+		}
 	}
-	if log != nil {
-		log.Info("attaching to local session", "intent", intent, "name", name)
-	}
-	return runClient(ctx, client.Dependencies{
-		Dialer:          localDialer(),
-		Terminal:        term.New(),
-		Clock:           clock.New(),
-		Logger:          log,
-		RuntimeObserver: deps.runtimeObserver,
-		Remote:          false,
-	}, client.AttachRequest{Intent: intent, SessionName: name})
 }
 const daemonStopTimeout = 2 * time.Second

M internal/app/run_test.go +42 -2


@@ -872,7 +872,7 @@ func TestRunAttachWithDepsSelectsRemoteTransport(t *testing.T) {
872	872		func TestRunAttachWithDepsRemotePickerHandoffReopensDirectConnection(t *testing.T) {
873	873		factory := portsmocks.NewMockRemoteDialerFactory(t)
874	874		factory.EXPECT().DialerForRemote("remote.example", "work", ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote-1"}, nil).Once()
875		-	factory.EXPECT().DialerForRemote("remote.example", "work", ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote-2"}, nil).Once()
	875	+	factory.EXPECT().DialerForRemote("selected.example", "picked", ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote-2"}, nil).Once()
876	876		var calls int
877	877		err := runAttachWithDeps(context.Background(), ports.IntentAttach, "work", "remote.example", "", nil, runAttachDeps{
878	878		remoteDialerFactory: factory,

@@ -880,7 +880,7 @@ func TestRunAttachWithDepsRemotePickerHandoffReopensDirectConnection(t *testing.
880	880		calls++
881	881		require.NotNil(t, deps.Dialer)
882	882		if calls == 1 {
883		-	return &client.AttachTargetError{Target: ports.AttachTarget{Endpoint: "remote.example", Session: "work", Intent: ports.IntentAttach}}
	883	+	return &client.AttachTargetError{Target: ports.AttachTarget{Endpoint: "selected.example", Session: "picked", Intent: ports.IntentAttach}}
884	884		}
885	885		return nil
886	886		},

@@ -889,6 +889,46 @@ func TestRunAttachWithDepsRemotePickerHandoffReopensDirectConnection(t *testing.
889	889		require.Equal(t, 2, calls, "a picker handoff must close the old connection and open a fresh direct connection")
890	890		}
891	891
	892	+	func TestRunAttachWithDepsRejectsHandoffLoop(t *testing.T) {
	893	+	factory := portsmocks.NewMockRemoteDialerFactory(t)
	894	+	factory.EXPECT().DialerForRemote("remote.example", "work", ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote"}, nil).Once()
	895	+
	896	+	calls := 0
	897	+	err := runAttachWithDeps(context.Background(), ports.IntentAttach, "work", "remote.example", "", nil, runAttachDeps{
	898	+	remoteDialerFactory: factory,
	899	+	runClient: func(_ context.Context, _ client.Dependencies, _ client.AttachRequest) error {
	900	+	calls++
	901	+	return &client.AttachTargetError{Target: ports.AttachTarget{Endpoint: "remote.example", Session: "work", Intent: ports.IntentAttach}}
	902	+	},
	903	+	})
	904	+	require.ErrorContains(t, err, "attach handoff loop")
	905	+	require.Equal(t, 1, calls)
	906	+	}
	907	+
	908	+	func TestRunAttachWithDepsLocalPickerHandoffAttachesSelectedRemote(t *testing.T) {
	909	+	factory := portsmocks.NewMockRemoteDialerFactory(t)
	910	+	factory.EXPECT().DialerForRemote("selected.example", "picked", ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote"}, nil).Once()
	911	+
	912	+	localCalls, remoteCalls := 0, 0
	913	+	err := runAttachWithDeps(context.Background(), ports.IntentAttach, "work", "", "", nil, runAttachDeps{
	914	+	localDialer: func() ports.Dialer { return namedDialer{name: "local"} },
	915	+	remoteDialerFactory: factory,
	916	+	runClient: func(_ context.Context, deps client.Dependencies, request client.AttachRequest) error {
	917	+	if deps.Remote {
	918	+	remoteCalls++
	919	+	require.Equal(t, client.AttachRequest{Intent: ports.IntentAttach, SessionName: "picked"}, request)
	920	+	return nil
	921	+	}
	922	+	localCalls++
	923	+	require.Equal(t, client.AttachRequest{Intent: ports.IntentAttach, SessionName: "work"}, request)
	924	+	return &client.AttachTargetError{Target: ports.AttachTarget{Endpoint: "selected.example", Session: "picked", Intent: ports.IntentAttach}}
	925	+	},
	926	+	})
	927	+	require.NoError(t, err)
	928	+	require.Equal(t, 1, localCalls, "the failed local picker attach must not retry locally")
	929	+	require.Equal(t, 1, remoteCalls, "the selected target must be dialed and attached exactly once")
	930	+	}
	931	+
892	932		func TestRunAttachWithDepsRejectsInvalidHandoffBeforeDialing(t *testing.T) {
893	933		tests := []struct {
894	934		name string


@@ -872,7 +872,7 @@ func TestRunAttachWithDepsSelectsRemoteTransport(t *testing.T) {
872		func TestRunAttachWithDepsRemotePickerHandoffReopensDirectConnection(t *testing.T) {	872		func TestRunAttachWithDepsRemotePickerHandoffReopensDirectConnection(t *testing.T) {
873		factory := portsmocks.NewMockRemoteDialerFactory(t)	873		factory := portsmocks.NewMockRemoteDialerFactory(t)
874		factory.EXPECT().DialerForRemote("remote.example", "work", ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote-1"}, nil).Once()	874		factory.EXPECT().DialerForRemote("remote.example", "work", ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote-1"}, nil).Once()
875	-	factory.EXPECT().DialerForRemote("remote.example", "work", ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote-2"}, nil).Once()	875	+	factory.EXPECT().DialerForRemote("selected.example", "picked", ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote-2"}, nil).Once()
876		var calls int	876		var calls int
877		err := runAttachWithDeps(context.Background(), ports.IntentAttach, "work", "remote.example", "", nil, runAttachDeps{	877		err := runAttachWithDeps(context.Background(), ports.IntentAttach, "work", "remote.example", "", nil, runAttachDeps{
878		remoteDialerFactory: factory,	878		remoteDialerFactory: factory,

@@ -880,7 +880,7 @@ func TestRunAttachWithDepsRemotePickerHandoffReopensDirectConnection(t *testing.
880		calls++	880		calls++
881		require.NotNil(t, deps.Dialer)	881		require.NotNil(t, deps.Dialer)
882		if calls == 1 {	882		if calls == 1 {
883	-	return &client.AttachTargetError{Target: ports.AttachTarget{Endpoint: "remote.example", Session: "work", Intent: ports.IntentAttach}}	883	+	return &client.AttachTargetError{Target: ports.AttachTarget{Endpoint: "selected.example", Session: "picked", Intent: ports.IntentAttach}}
884		}	884		}
885		return nil	885		return nil
886		},	886		},

@@ -889,6 +889,46 @@ func TestRunAttachWithDepsRemotePickerHandoffReopensDirectConnection(t *testing.
889		require.Equal(t, 2, calls, "a picker handoff must close the old connection and open a fresh direct connection")	889		require.Equal(t, 2, calls, "a picker handoff must close the old connection and open a fresh direct connection")
890		}	890		}
891			891
			892	+	func TestRunAttachWithDepsRejectsHandoffLoop(t *testing.T) {
			893	+	factory := portsmocks.NewMockRemoteDialerFactory(t)
			894	+	factory.EXPECT().DialerForRemote("remote.example", "work", ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote"}, nil).Once()
			895	+
			896	+	calls := 0
			897	+	err := runAttachWithDeps(context.Background(), ports.IntentAttach, "work", "remote.example", "", nil, runAttachDeps{
			898	+	remoteDialerFactory: factory,
			899	+	runClient: func(_ context.Context, _ client.Dependencies, _ client.AttachRequest) error {
			900	+	calls++
			901	+	return &client.AttachTargetError{Target: ports.AttachTarget{Endpoint: "remote.example", Session: "work", Intent: ports.IntentAttach}}
			902	+	},
			903	+	})
			904	+	require.ErrorContains(t, err, "attach handoff loop")
			905	+	require.Equal(t, 1, calls)
			906	+	}
			907	+
			908	+	func TestRunAttachWithDepsLocalPickerHandoffAttachesSelectedRemote(t *testing.T) {
			909	+	factory := portsmocks.NewMockRemoteDialerFactory(t)
			910	+	factory.EXPECT().DialerForRemote("selected.example", "picked", ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote"}, nil).Once()
			911	+
			912	+	localCalls, remoteCalls := 0, 0
			913	+	err := runAttachWithDeps(context.Background(), ports.IntentAttach, "work", "", "", nil, runAttachDeps{
			914	+	localDialer: func() ports.Dialer { return namedDialer{name: "local"} },
			915	+	remoteDialerFactory: factory,
			916	+	runClient: func(_ context.Context, deps client.Dependencies, request client.AttachRequest) error {
			917	+	if deps.Remote {
			918	+	remoteCalls++
			919	+	require.Equal(t, client.AttachRequest{Intent: ports.IntentAttach, SessionName: "picked"}, request)
			920	+	return nil
			921	+	}
			922	+	localCalls++
			923	+	require.Equal(t, client.AttachRequest{Intent: ports.IntentAttach, SessionName: "work"}, request)
			924	+	return &client.AttachTargetError{Target: ports.AttachTarget{Endpoint: "selected.example", Session: "picked", Intent: ports.IntentAttach}}
			925	+	},
			926	+	})
			927	+	require.NoError(t, err)
			928	+	require.Equal(t, 1, localCalls, "the failed local picker attach must not retry locally")
			929	+	require.Equal(t, 1, remoteCalls, "the selected target must be dialed and attached exactly once")
			930	+	}
			931	+
892		func TestRunAttachWithDepsRejectsInvalidHandoffBeforeDialing(t *testing.T) {	932		func TestRunAttachWithDepsRejectsInvalidHandoffBeforeDialing(t *testing.T) {
893		tests := []struct {	933		tests := []struct {
894		name string	934		name string