From 73bb865a8a84c9a4414cbebba2569d835f6ea32c Mon Sep 17 00:00:00 2001
From: riccardom <riccardomanfrin@gmail.com>
Date: Tue, 20 Oct 2026 12:24:28 +0200
Subject: [PATCH] [client] Keep a converging map when an older one arrives

The first pass of a new target replaced syncTargetMap before the serial
check in updateNetworkMap ran. An outdated map arriving mid-convergence
took the place of the newer one, was then ignored as stale and reported as
converged, so the newer map's remaining peer batches were never applied
until the next update. A stale map now leaves the pending one in place.
---
 client/internal/engine.go      |  9 +++++++++
 client/internal/engine_test.go | 20 ++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/client/internal/engine.go b/client/internal/engine.go
index 3f244ced7..d73beb400 100644
--- a/client/internal/engine.go
+++ b/client/internal/engine.go
@@ -1075,6 +1075,7 @@ func (e *Engine) applySyncPass(update *mgmProto.SyncResponse, firstPass bool) (b
 // when the update carries one. A config-only update leaves syncTargetMap nil. The
 // caller holds syncMsgMux.
 func (e *Engine) loadSyncTarget(update *mgmProto.SyncResponse) error {
+	pending := e.syncTargetMap
 	e.syncTargetMap = nil
 
 	e.ApplySessionDeadline(update.GetSessionExpiresAt())
@@ -1158,6 +1159,14 @@ func (e *Engine) loadSyncTarget(update *mgmProto.SyncResponse) error {
 		e.latestComponents = components
 	}
 
+	// updateNetworkMap ignores a stale map and reports convergence, which would
+	// strand the remaining peer batches of a newer map still being applied.
+	if pending != nil && nm.GetSerial() < e.networkSerial {
+		log.Debugf("ignoring network map with serial %d while serial %d is converging", nm.GetSerial(), e.networkSerial)
+		e.syncTargetMap = pending
+		return nil
+	}
+
 	e.syncTargetMap = nm
 	return nil
 }
diff --git a/client/internal/engine_test.go b/client/internal/engine_test.go
index 289d6b2bc..6173e99d7 100644
--- a/client/internal/engine_test.go
+++ b/client/internal/engine_test.go
@@ -1798,3 +1798,23 @@ func TestEngine_ApplySyncPassDecodesOncePerTarget(t *testing.T) {
 	_, err = e.applySyncPass(undecodable, true)
 	assert.Error(t, err, "the first pass of a target decodes it")
 }
+
+// A map older than the one still converging must not take its place: the
+// stale guard in updateNetworkMap would ignore it and report convergence,
+// leaving the newer map's remaining peer batches unapplied.
+func TestEngine_LoadSyncTargetKeepsNewerPendingMap(t *testing.T) {
+	pending := &mgmtProto.NetworkMap{Serial: 10}
+	e := &Engine{
+		ctx:           context.Background(),
+		syncMsgMux:    &sync.Mutex{},
+		networkSerial: 10,
+		syncTargetMap: pending,
+	}
+
+	require.NoError(t, e.loadSyncTarget(&mgmtProto.SyncResponse{NetworkMap: &mgmtProto.NetworkMap{Serial: 9}}))
+	assert.Same(t, pending, e.syncTargetMap, "a stale map should leave the pending newer map in place")
+
+	newer := &mgmtProto.NetworkMap{Serial: 11}
+	require.NoError(t, e.loadSyncTarget(&mgmtProto.SyncResponse{NetworkMap: newer}))
+	assert.Same(t, newer, e.syncTargetMap, "a newer map should replace the pending one")
+}