[management] add activity events for domains (#5548)

* add activity events for domains

* fix test

* update activity codes

* update activity codes

client/server/server.go

@@ -1625,9 +1625,14 @@ func (s *Server) GetFeatures(ctx context.Context, msg *proto.GetFeaturesRequest)
 
 func (s *Server) connect(ctx context.Context, config *profilemanager.Config, statusRecorder *peer.Status, doInitialAutoUpdate bool, runningChan chan struct{}) error {
 	log.Tracef("running client connection")
-	s.connectClient = internal.NewConnectClient(ctx, config, statusRecorder, doInitialAutoUpdate)
-	s.connectClient.SetSyncResponsePersistence(s.persistSyncResponse)
-	if err := s.connectClient.Run(runningChan, s.logFile); err != nil {
+	client := internal.NewConnectClient(ctx, config, statusRecorder, doInitialAutoUpdate)
+	client.SetSyncResponsePersistence(s.persistSyncResponse)
+
+	s.mutex.Lock()
+	s.connectClient = client
+	s.mutex.Unlock()
+
+	if err := client.Run(runningChan, s.logFile); err != nil {
 		return err
 	}
 	return nil

client/server/server_connect_test.go

@@ -0,0 +1,187 @@
+package server
+
+import (
+	"context"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/netbirdio/netbird/client/internal"
+	"github.com/netbirdio/netbird/client/internal/peer"
+	"github.com/netbirdio/netbird/client/proto"
+)
+
+func newTestServer() *Server {
+	return &Server{
+		rootCtx:        context.Background(),
+		statusRecorder: peer.NewRecorder(""),
+	}
+}
+
+func newDummyConnectClient(ctx context.Context) *internal.ConnectClient {
+	return internal.NewConnectClient(ctx, nil, nil, false)
+}
+
+// TestConnectSetsClientWithMutex validates that connect() sets s.connectClient
+// under mutex protection so concurrent readers see a consistent value.
+func TestConnectSetsClientWithMutex(t *testing.T) {
+	s := newTestServer()
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	// Manually simulate what connect() does (without calling Run which panics without full setup)
+	client := newDummyConnectClient(ctx)
+
+	s.mutex.Lock()
+	s.connectClient = client
+	s.mutex.Unlock()
+
+	// Verify the assignment is visible under mutex
+	s.mutex.Lock()
+	assert.Equal(t, client, s.connectClient, "connectClient should be set")
+	s.mutex.Unlock()
+}
+
+// TestConcurrentConnectClientAccess validates that concurrent reads of
+// s.connectClient under mutex don't race with a write.
+func TestConcurrentConnectClientAccess(t *testing.T) {
+	s := newTestServer()
+	ctx := context.Background()
+	client := newDummyConnectClient(ctx)
+
+	var wg sync.WaitGroup
+	nilCount := 0
+	setCount := 0
+	var mu sync.Mutex
+
+	// Start readers
+	for i := 0; i < 50; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			s.mutex.Lock()
+			c := s.connectClient
+			s.mutex.Unlock()
+
+			mu.Lock()
+			defer mu.Unlock()
+			if c == nil {
+				nilCount++
+			} else {
+				setCount++
+			}
+		}()
+	}
+
+	// Simulate connect() writing under mutex
+	time.Sleep(5 * time.Millisecond)
+	s.mutex.Lock()
+	s.connectClient = client
+	s.mutex.Unlock()
+
+	wg.Wait()
+
+	assert.Equal(t, 50, nilCount+setCount, "all goroutines should complete without panic")
+}
+
+// TestCleanupConnection_ClearsConnectClient validates that cleanupConnection
+// properly nils out connectClient.
+func TestCleanupConnection_ClearsConnectClient(t *testing.T) {
+	s := newTestServer()
+	_, cancel := context.WithCancel(context.Background())
+	s.actCancel = cancel
+
+	s.connectClient = newDummyConnectClient(context.Background())
+	s.clientRunning = true
+
+	err := s.cleanupConnection()
+	require.NoError(t, err)
+
+	assert.Nil(t, s.connectClient, "connectClient should be nil after cleanup")
+}
+
+// TestCleanState_NilConnectClient validates that CleanState doesn't panic
+// when connectClient is nil.
+func TestCleanState_NilConnectClient(t *testing.T) {
+	s := newTestServer()
+	s.connectClient = nil
+	s.profileManager = nil // will cause error if it tries to proceed past the nil check
+
+	// Should not panic — the nil check should prevent calling Status() on nil
+	assert.NotPanics(t, func() {
+		_, _ = s.CleanState(context.Background(), &proto.CleanStateRequest{All: true})
+	})
+}
+
+// TestDeleteState_NilConnectClient validates that DeleteState doesn't panic
+// when connectClient is nil.
+func TestDeleteState_NilConnectClient(t *testing.T) {
+	s := newTestServer()
+	s.connectClient = nil
+	s.profileManager = nil
+
+	assert.NotPanics(t, func() {
+		_, _ = s.DeleteState(context.Background(), &proto.DeleteStateRequest{All: true})
+	})
+}
+
+// TestDownThenUp_StaleRunningChan documents the known state issue where
+// clientRunningChan from a previous connection is already closed, causing
+// waitForUp() to return immediately on reconnect.
+func TestDownThenUp_StaleRunningChan(t *testing.T) {
+	s := newTestServer()
+
+	// Simulate state after a successful connection
+	s.clientRunning = true
+	s.clientRunningChan = make(chan struct{})
+	close(s.clientRunningChan) // closed when engine started
+	s.clientGiveUpChan = make(chan struct{})
+	s.connectClient = newDummyConnectClient(context.Background())
+
+	_, cancel := context.WithCancel(context.Background())
+	s.actCancel = cancel
+
+	// Simulate Down(): cleanupConnection sets connectClient = nil
+	s.mutex.Lock()
+	err := s.cleanupConnection()
+	s.mutex.Unlock()
+	require.NoError(t, err)
+
+	// After cleanup: connectClient is nil, clientRunning still true
+	// (goroutine hasn't exited yet)
+	s.mutex.Lock()
+	assert.Nil(t, s.connectClient, "connectClient should be nil after cleanup")
+	assert.True(t, s.clientRunning, "clientRunning still true until goroutine exits")
+	s.mutex.Unlock()
+
+	// waitForUp() returns immediately due to stale closed clientRunningChan
+	ctx, ctxCancel := context.WithTimeout(context.Background(), 2*time.Second)
+	defer ctxCancel()
+
+	waitDone := make(chan error, 1)
+	go func() {
+		_, err := s.waitForUp(ctx)
+		waitDone <- err
+	}()
+
+	select {
+	case err := <-waitDone:
+		assert.NoError(t, err, "waitForUp returns success on stale channel")
+		// But connectClient is still nil — this is the stale state issue
+		s.mutex.Lock()
+		assert.Nil(t, s.connectClient, "connectClient is nil despite waitForUp success")
+		s.mutex.Unlock()
+	case <-time.After(1 * time.Second):
+		t.Fatal("waitForUp should have returned immediately due to stale closed channel")
+	}
+}
+
+// TestConnectClient_EngineNilOnFreshClient validates that a newly created
+// ConnectClient has nil Engine (before Run is called).
+func TestConnectClient_EngineNilOnFreshClient(t *testing.T) {
+	client := newDummyConnectClient(context.Background())
+	assert.Nil(t, client.Engine(), "engine should be nil on fresh ConnectClient")
+}

client/server/state.go

@@ -39,7 +39,7 @@ func (s *Server) ListStates(_ context.Context, _ *proto.ListStatesRequest) (*pro
 
 // CleanState handles cleaning of states (performing cleanup operations)
 func (s *Server) CleanState(ctx context.Context, req *proto.CleanStateRequest) (*proto.CleanStateResponse, error) {
-	if s.connectClient.Status() == internal.StatusConnected || s.connectClient.Status() == internal.StatusConnecting {
+	if s.connectClient != nil && (s.connectClient.Status() == internal.StatusConnected || s.connectClient.Status() == internal.StatusConnecting) {
 		return nil, status.Errorf(codes.FailedPrecondition, "cannot clean state while connecting or connected, run 'netbird down' first.")
 	}
 
@@ -82,7 +82,7 @@ func (s *Server) CleanState(ctx context.Context, req *proto.CleanStateRequest) (
 
 // DeleteState handles deletion of states without cleanup
 func (s *Server) DeleteState(ctx context.Context, req *proto.DeleteStateRequest) (*proto.DeleteStateResponse, error) {
-	if s.connectClient.Status() == internal.StatusConnected || s.connectClient.Status() == internal.StatusConnecting {
+	if s.connectClient != nil && (s.connectClient.Status() == internal.StatusConnected || s.connectClient.Status() == internal.StatusConnecting) {
 		return nil, status.Errorf(codes.FailedPrecondition, "cannot clean state while connecting or connected, run 'netbird down' first.")
 	}
 

client/ui/client_ui.go

@@ -323,7 +323,7 @@ type serviceClient struct {
 
 	exitNodeMu           sync.Mutex
 	mExitNodeItems       []menuHandler
-	exitNodeStates       []exitNodeState
+	exitNodeRetryCancel  context.CancelFunc
 	mExitNodeDeselectAll *systray.MenuItem
 	logFile              string
 	wLoginURL            fyne.Window
@@ -924,7 +924,7 @@ func (s *serviceClient) updateStatus() error {
 			s.mDown.Enable()
 			s.mNetworks.Enable()
 			s.mExitNode.Enable()
-			go s.updateExitNodes()
+			s.startExitNodeRefresh()
 			systrayIconState = true
 		case status.Status == string(internal.StatusConnecting):
 			s.setConnectingStatus()
@@ -985,6 +985,7 @@ func (s *serviceClient) setDisconnectedStatus() {
 	s.mUp.Enable()
 	s.mNetworks.Disable()
 	s.mExitNode.Disable()
+	s.cancelExitNodeRetry()
 	go s.updateExitNodes()
 }
 

client/ui/event_handler.go

@@ -100,8 +100,7 @@ func (h *eventHandler) handleConnectClick() {
 
 func (h *eventHandler) handleDisconnectClick() {
 	h.client.mDown.Disable()
-
-	h.client.exitNodeStates = []exitNodeState{}
+	h.client.cancelExitNodeRetry()
 
 	if h.client.connectCancel != nil {
 		log.Debugf("cancelling ongoing connect operation")

client/ui/network.go

@@ -6,7 +6,6 @@ import (
 	"context"
 	"fmt"
 	"runtime"
-	"slices"
 	"sort"
 	"strings"
 	"time"
@@ -34,11 +33,6 @@ const (
 
 type filter string
 
-type exitNodeState struct {
-	id       string
-	selected bool
-}
-
 func (s *serviceClient) showNetworksUI() {
 	s.wNetworks = s.app.NewWindow("Networks")
 	s.wNetworks.SetOnClosed(s.cancel)
@@ -335,16 +329,75 @@ func (s *serviceClient) updateNetworksBasedOnDisplayTab(tabs *container.AppTabs,
 	s.updateNetworks(grid, f)
 }
 
-func (s *serviceClient) updateExitNodes() {
+// startExitNodeRefresh initiates exit node menu refresh after connecting.
+// On Windows, TrayOpenedCh is not supported by the systray library, so we use
+// a background poller to keep exit nodes in sync while connected.
+// On macOS/Linux, TrayOpenedCh handles refreshes on each tray open.
+func (s *serviceClient) startExitNodeRefresh() {
+	s.cancelExitNodeRetry()
+
+	if runtime.GOOS == "windows" {
+		ctx, cancel := context.WithCancel(s.ctx)
+		s.exitNodeMu.Lock()
+		s.exitNodeRetryCancel = cancel
+		s.exitNodeMu.Unlock()
+
+		go s.pollExitNodes(ctx)
+	} else {
+		go s.updateExitNodes()
+	}
+}
+
+func (s *serviceClient) cancelExitNodeRetry() {
+	s.exitNodeMu.Lock()
+	if s.exitNodeRetryCancel != nil {
+		s.exitNodeRetryCancel()
+		s.exitNodeRetryCancel = nil
+	}
+	s.exitNodeMu.Unlock()
+}
+
+// pollExitNodes periodically refreshes exit nodes while connected.
+// Uses a short initial interval to catch routes from the management sync,
+// then switches to a longer interval for ongoing updates.
+func (s *serviceClient) pollExitNodes(ctx context.Context) {
+	// Initial fast polling to catch routes as they appear after connect.
+	for i := 0; i < 5; i++ {
+		if s.updateExitNodes() {
+			break
+		}
+		select {
+		case <-ctx.Done():
+			return
+		case <-time.After(2 * time.Second):
+		}
+	}
+
+	ticker := time.NewTicker(10 * time.Second)
+	defer ticker.Stop()
+
+	for {
+		select {
+		case <-ctx.Done():
+			return
+		case <-ticker.C:
+			s.updateExitNodes()
+		}
+	}
+}
+
+// updateExitNodes fetches exit nodes from the daemon and recreates the menu.
+// Returns true if exit nodes were found.
+func (s *serviceClient) updateExitNodes() bool {
 	conn, err := s.getSrvClient(defaultFailTimeout)
 	if err != nil {
 		log.Errorf("get client: %v", err)
-		return
+		return false
 	}
 	exitNodes, err := s.getExitNodes(conn)
 	if err != nil {
 		log.Errorf("get exit nodes: %v", err)
-		return
+		return false
 	}
 
 	s.exitNodeMu.Lock()
@@ -354,28 +407,14 @@ func (s *serviceClient) updateExitNodes() {
 
 	if len(s.mExitNodeItems) > 0 {
 		s.mExitNode.Enable()
-	} else {
-		s.mExitNode.Disable()
+		return true
 	}
+
+	s.mExitNode.Disable()
+	return false
 }
 
 func (s *serviceClient) recreateExitNodeMenu(exitNodes []*proto.Network) {
-	var exitNodeIDs []exitNodeState
-	for _, node := range exitNodes {
-		exitNodeIDs = append(exitNodeIDs, exitNodeState{
-			id:       node.ID,
-			selected: node.Selected,
-		})
-	}
-
-	sort.Slice(exitNodeIDs, func(i, j int) bool {
-		return exitNodeIDs[i].id < exitNodeIDs[j].id
-	})
-	if slices.Equal(s.exitNodeStates, exitNodeIDs) {
-		log.Debug("Exit node menu already up to date")
-		return
-	}
-
 	for _, node := range s.mExitNodeItems {
 		node.cancel()
 		node.Hide()
@@ -413,8 +452,6 @@ func (s *serviceClient) recreateExitNodeMenu(exitNodes []*proto.Network) {
 		go s.handleChecked(ctx, node.ID, menuItem)
 	}
 
-	s.exitNodeStates = exitNodeIDs
-
 	if showDeselectAll {
 		s.mExitNode.AddSeparator()
 		deselectAllItem := s.mExitNode.AddSubMenuItem("Deselect All", "Deselect All")

management/internals/shared/grpc/server.go

@@ -330,13 +330,12 @@ func (s *Server) Sync(req *proto.EncryptedMessage, srv proto.ManagementService_S
 
 	s.secretsManager.SetupRefresh(ctx, accountID, peer.ID)
 
-	if s.appMetrics != nil {
-		s.appMetrics.GRPCMetrics().CountSyncRequestDuration(time.Since(reqStart), accountID)
-	}
-
 	unlock()
 	unlock = nil
 
+	if s.appMetrics != nil {
+		s.appMetrics.GRPCMetrics().CountSyncRequestDuration(time.Since(reqStart), accountID)
+	}
 	log.WithContext(ctx).Debugf("Sync took %s", time.Since(reqStart))
 
 	s.syncSem.Add(-1)
@@ -743,13 +742,6 @@ func (s *Server) Login(ctx context.Context, req *proto.EncryptedMessage) (*proto
 
 	log.WithContext(ctx).Debugf("Login request from peer [%s] [%s]", req.WgPubKey, sRealIP)
 
-	defer func() {
-		if s.appMetrics != nil {
-			s.appMetrics.GRPCMetrics().CountLoginRequestDuration(time.Since(reqStart), accountID)
-		}
-		log.WithContext(ctx).Debugf("Login took %s", time.Since(reqStart))
-	}()
-
 	if loginReq.GetMeta() == nil {
 		msg := status.Errorf(codes.FailedPrecondition,
 			"peer system meta has to be provided to log in. Peer %s, remote addr %s", peerKey.String(), realIP)
@@ -799,6 +791,11 @@ func (s *Server) Login(ctx context.Context, req *proto.EncryptedMessage) (*proto
 		return nil, status.Errorf(codes.Internal, "failed logging in peer")
 	}
 
+	if s.appMetrics != nil {
+		s.appMetrics.GRPCMetrics().CountLoginRequestDuration(time.Since(reqStart), accountID)
+	}
+	log.WithContext(ctx).Debugf("Login took %s", time.Since(reqStart))
+
 	return &proto.EncryptedMessage{
 		WgPubKey: key.PublicKey().String(),
 		Body:     encryptedResp,

management/server/activity/codes.go

@@ -221,11 +221,11 @@ const (
 	AccountPeerExposeDisabled Activity = 115
 
 	// DomainAdded indicates that a user added a custom domain
-	DomainAdded Activity = 116
+	DomainAdded Activity = 118
 	// DomainDeleted indicates that a user deleted a custom domain
-	DomainDeleted Activity = 117
+	DomainDeleted Activity = 119
 	// DomainValidated indicates that a custom domain was validated
-	DomainValidated Activity = 118
+	DomainValidated Activity = 120
 
 	AccountDeleted Activity = 99999
 )

proxy/internal/acme/manager.go

@@ -135,45 +135,24 @@ func (mgr *Manager) AddDomain(d domain.Domain, accountID, serviceID string) {
 
 // prefetchCertificate proactively triggers certificate generation for a domain.
 // It acquires a distributed lock to prevent multiple replicas from issuing
-// duplicate ACME requests. If the certificate appears in the cache while waiting
-// for the lock, it cancels the wait and uses the cached certificate.
+// duplicate ACME requests. The second replica will block until the first
+// finishes, then find the certificate in the cache.
 func (mgr *Manager) prefetchCertificate(d domain.Domain) {
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 	defer cancel()
 
 	name := d.PunycodeString()
 
-	if mgr.certExistsInCache(ctx, name) {
-		mgr.logger.Infof("certificate for domain %q already exists in cache before lock attempt", name)
-		mgr.loadAndFinalizeCachedCert(ctx, d, name)
-		return
-	}
-
-	go mgr.pollCacheAndCancel(ctx, name, cancel)
-
-	// Acquire lock
 	mgr.logger.Infof("acquiring cert lock for domain %q", name)
 	lockStart := time.Now()
 	unlock, err := mgr.locker.Lock(ctx, name)
 	if err != nil {
-		if mgr.certExistsInCache(context.Background(), name) {
-			mgr.logger.Infof("certificate for domain %q appeared in cache while waiting for lock", name)
-			mgr.loadAndFinalizeCachedCert(context.Background(), d, name)
-			return
-		}
-		mgr.logger.Warnf("acquire cert lock for domain %q: %v", name, err)
-		// Continue without lock
+		mgr.logger.Warnf("acquire cert lock for domain %q, proceeding without lock: %v", name, err)
 	} else {
 		mgr.logger.Infof("acquired cert lock for domain %q in %s", name, time.Since(lockStart))
 		defer unlock()
 	}
 
-	if mgr.certExistsInCache(ctx, name) {
-		mgr.logger.Infof("certificate for domain %q already exists in cache after lock acquisition, skipping ACME request", name)
-		mgr.loadAndFinalizeCachedCert(ctx, d, name)
-		return
-	}
-
 	hello := &tls.ClientHelloInfo{
 		ServerName: name,
 		Conn:       &dummyConn{ctx: ctx},
@@ -230,78 +209,6 @@ func (mgr *Manager) setDomainState(d domain.Domain, state domainState, errMsg st
 	}
 }
 
-// certExistsInCache checks if a certificate exists on the shared disk for the given domain.
-func (mgr *Manager) certExistsInCache(ctx context.Context, domain string) bool {
-	if mgr.Cache == nil {
-		return false
-	}
-	_, err := mgr.Cache.Get(ctx, domain)
-	return err == nil
-}
-
-// pollCacheAndCancel periodically checks if a certificate appears in the cache.
-// If found, it cancels the context to abort the lock wait.
-func (mgr *Manager) pollCacheAndCancel(ctx context.Context, domain string, cancel context.CancelFunc) {
-	ticker := time.NewTicker(5 * time.Second)
-	defer ticker.Stop()
-
-	for {
-		select {
-		case <-ctx.Done():
-			return
-		case <-ticker.C:
-			if mgr.certExistsInCache(context.Background(), domain) {
-				mgr.logger.Debugf("cert detected in cache for domain %q, cancelling lock wait", domain)
-				cancel()
-				return
-			}
-		}
-	}
-}
-
-// loadAndFinalizeCachedCert loads a certificate from cache and updates domain state.
-func (mgr *Manager) loadAndFinalizeCachedCert(ctx context.Context, d domain.Domain, name string) {
-	hello := &tls.ClientHelloInfo{
-		ServerName: name,
-		Conn:       &dummyConn{ctx: ctx},
-	}
-
-	cert, err := mgr.GetCertificate(hello)
-	if err != nil {
-		mgr.logger.Warnf("load cached certificate for domain %q: %v", name, err)
-		mgr.setDomainState(d, domainFailed, err.Error())
-		return
-	}
-
-	mgr.setDomainState(d, domainReady, "")
-
-	now := time.Now()
-	if cert != nil && cert.Leaf != nil {
-		leaf := cert.Leaf
-		mgr.logger.Infof("loaded cached certificate for domain %q: serial=%s SANs=%v notBefore=%s, notAfter=%s, now=%s",
-			name,
-			leaf.SerialNumber.Text(16),
-			leaf.DNSNames,
-			leaf.NotBefore.UTC().Format(time.RFC3339),
-			leaf.NotAfter.UTC().Format(time.RFC3339),
-			now.UTC().Format(time.RFC3339),
-		)
-		mgr.logCertificateDetails(name, leaf, now)
-	} else {
-		mgr.logger.Infof("loaded cached certificate for domain %q", name)
-	}
-
-	mgr.mu.RLock()
-	info := mgr.domains[d]
-	mgr.mu.RUnlock()
-
-	if info != nil && mgr.certNotifier != nil {
-		if err := mgr.certNotifier.NotifyCertificateIssued(ctx, info.accountID, info.serviceID, name); err != nil {
-			mgr.logger.Warnf("notify certificate ready for domain %q: %v", name, err)
-		}
-	}
-}
-
 // logCertificateDetails logs certificate validity and SCT timestamps.
 func (mgr *Manager) logCertificateDetails(domain string, cert *x509.Certificate, now time.Time) {
 	if cert.NotBefore.After(now) {