From 4abb10c1aa46de1855a281808fea41c0119ee06a Mon Sep 17 00:00:00 2001
From: riccardom <riccardomanfrin@gmail.com>
Date: Wed, 10 Jun 2026 14:30:02 +0200
Subject: [PATCH] Fixes DisableAutoConnect semantics
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

DisableAutoConnect semantics
============================

Scope: governs ONLY the service-Start auto-connect decision. Once the
connection goroutine has been spawned (by any path), the flag is never
consulted again — the retry loop keeps trying to connect until ctx is
cancelled by Down / Stop / Logout.

Cases
-----

1. Service Start + DisableAutoConnect = true
   - No connection goroutine spawned.
   - clientRunning stays false.
   - State set to StatusIdle.
   - Daemon stays passive until an explicit Up RPC.

2. Service Start + DisableAutoConnect = false
   - Spawn connectWithRetryRuns.
   - clientRunning = true.
   - Retry loop runs until ctx cancelled.

3. Up RPC (any value of DisableAutoConnect)
   - Flag ignored. The user / admin explicitly asked to connect — by
     definition not "auto".
   - Spawn connectWithRetryRuns.
   - clientRunning = true.

4. MDM-triggered restart (any value of DisableAutoConnect)
   - Flag ignored. An MDM policy change applies new config to an
     already-running engine; treated as an implicit Up.
   - Spawn connectWithRetryRuns.
   - clientRunning = true.

5. Down / Stop / Logout
   - Cancels ctx → connectWithRetryRuns exits → close(giveUpChan).
   - cleanupConnection clears clientRunning = false.
   - DisableAutoConnect not involved.
Prepare for next step

Collapse error log into s.connect and rename it to more explicit connectOnce

# Conflicts:
#	client/server/server.go
---
 client/server/server.go | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/client/server/server.go b/client/server/server.go
index d60d2dc3b..67a4a065a 100644
--- a/client/server/server.go
+++ b/client/server/server.go
@@ -243,16 +243,20 @@ func (s *Server) Start() error {
 
 // connectWithRetryRuns runs the client connection with a backoff strategy where we retry the operation as additional
 // mechanism to keep the client connected even when the connection is lost.
-// we cancel retry if the client receive a stop or down command, or if disable auto connect is configured.
+// we cancel retry if the client receive a stop or down command.
+//
+// DisableAutoConnect governs ONLY the service-Start auto-connect decision
+// (handled in Start). Once this goroutine is spawned — by Start, Up or an
+// MDM restart — the flag is never consulted again: the retry loop keeps
+// trying until ctx is cancelled by Down / Stop / Logout.
 //
 // The goroutine's exit is signalled to the daemon via close(giveUpChan)
 // — placed in the function-scope defer so every return path (panic,
-// DisableAutoConnect early-exit, backoff exhausted, ctx cancel) closes
-// it. Callers that need to observe "is the goroutine still alive?" use
-// Server.connectionGoroutineRunning() which non-blockingly checks the close state
-// of clientGiveUpChan. The defer does NOT touch s.mutex; the daemon's
-// "intent" (clientRunning) is maintained by the RPC handlers, not by this
-// goroutine.
+// backoff exhausted, ctx cancel) closes it. Callers that need to observe
+// "is the goroutine still alive?" use Server.connectionGoroutineRunning()
+// which non-blockingly checks the close state of clientGiveUpChan. The defer
+// does NOT touch s.mutex; the daemon's "intent" (clientRunning) is maintained
+// by the RPC handlers, not by this goroutine.
 func (s *Server) connectWithRetryRuns(ctx context.Context, client *internal.ConnectClient, profileConfig *profilemanager.Config, statusRecorder *peer.Status, runningChan chan struct{}, giveUpChan chan struct{}) {
 	defer func() {
 		if giveUpChan != nil {
@@ -260,14 +264,6 @@ func (s *Server) connectWithRetryRuns(ctx context.Context, client *internal.Conn
 		}
 	}()
 
-	if s.config.DisableAutoConnect {
-		if err := s.connect(client, s.config, runningChan); err != nil {
-			log.Debugf("run client connection exited with error: %v", err)
-		}
-		log.Tracef("client connection exited")
-		return
-	}
-
 	backOff := getConnectWithBackoff(ctx)
 	go func() {
 		t := time.NewTicker(24 * time.Hour)
@@ -290,9 +286,9 @@ func (s *Server) connectWithRetryRuns(ctx context.Context, client *internal.Conn
 	}()
 
 	runOperation := func() error {
-		err := s.connect(client, profileConfig, runningChan)
+		err := s.connectOnce(client, profileConfig, runningChan)
 		if err != nil {
-			log.Debugf("run client connection exited with error: %v. Will retry in the background", err)
+			log.Debugf("will retry the connection in the background")
 			return err
 		}
 
@@ -1738,9 +1734,13 @@ func (s *Server) GetFeatures(ctx context.Context, msg *proto.GetFeaturesRequest)
 	return features, nil
 }
 
-func (s *Server) connect(client *internal.ConnectClient, config *profilemanager.Config, runningChan chan struct{}) error {
+// connectOnce performs a single client run (Run blocks, retrying its own
+// internal backoff, until the run ends). The outer connectWithRetryRuns
+// backoff re-invokes it only when a run returns an error.
+func (s *Server) connectOnce(client *internal.ConnectClient, config *profilemanager.Config, runningChan chan struct{}) error {
 	log.Tracef("running client connection")
 	if err := client.Run(config, runningChan, s.logFile); err != nil {
+		log.Debugf("run client connection exited with error: %v", err)
 		return err
 	}
 	return nil