mapper: record initial-map peers only after delivery

A timed-out initial map left phantom lastSentPeers, skewing later diffs.

hscontrol/mapper/batcher.go

@@ -310,7 +310,12 @@ func (b *Batcher) AddNode(
 	// and we want to avoid the race condition where the receiver isn't ready yet
 	select {
 	case c <- initialMap:
-		// Success
+		// Record sent peers only after confirmed delivery, mirroring the async
+		// path, and under workMu so a concurrent async bundle for this node
+		// cannot interleave its own lastSentPeers update.
+		nodeConn.workMu.Lock()
+		nodeConn.updateSentPeers(initialMap)
+		nodeConn.workMu.Unlock()
 	case <-time.After(5 * time.Second): //nolint:mnd
 		nlog.Error().Err(ErrInitialMapSendTimeout).Msg("initial map send timeout")
 		nlog.Debug().Caller().Dur("timeout.duration", 5*time.Second). //nolint:mnd
@@ -483,9 +488,11 @@ func (b *Batcher) worker(workerID int) {
 							Uint64(zf.NodeID, w.nodeID.Uint64()).
 							Str(zf.Reason, w.changes[0].Reason).
 							Msg("failed to generate map response for synchronous work")
-					} else if result.mapResponse != nil {
-						nc.updateSentPeers(result.mapResponse)
 					}
+					// Peer tracking is recorded by the caller (AddNode) only
+					// after the initial map is actually delivered; recording it
+					// here, before delivery, would leave phantom lastSentPeers
+					// if the send times out.
 
 					nc.workMu.Unlock()
 				} else {

hscontrol/mapper/initialmap_phantom_test.go

@@ -0,0 +1,67 @@
+package mapper
+
+import (
+	"testing"
+
+	"tailscale.com/tailcfg"
+)
+
+// TestSyncInitialMapNoPhantomPeersOnTimeout ensures the synchronous initial-map
+// path does not record peers as sent until the map is actually delivered. When
+// the AddNode channel send times out, the client received nothing, so
+// lastSentPeers must stay empty; otherwise future computePeerDiff calculations
+// miss peer additions or removals after reconnect.
+func TestSyncInitialMapNoPhantomPeersOnTimeout(t *testing.T) {
+	testData, cleanup := setupBatcherWithTestData(t, NewBatcherAndMapper, 1, 2, normalBufferSize)
+	defer cleanup()
+
+	batcher := testData.Batcher.Batcher
+	state := testData.State
+
+	peerNode := &testData.Nodes[0]
+	targetNode := &testData.Nodes[1]
+
+	state.Connect(peerNode.n.ID)
+
+	err := batcher.AddNode(peerNode.n.ID, peerNode.ch, tailcfg.CapabilityVersion(100), nil)
+	if err != nil {
+		t.Fatalf("adding peer node: %v", err)
+	}
+
+	go func() {
+		for range peerNode.ch {
+		}
+	}()
+
+	state.Connect(targetNode.n.ID)
+
+	// Unbuffered channel that nobody reads: AddNode blocks on the initial-map
+	// send and times out.
+	unreadCh := make(chan *tailcfg.MapResponse)
+
+	err = batcher.AddNode(targetNode.n.ID, unreadCh, tailcfg.CapabilityVersion(100), nil)
+	if err == nil {
+		t.Fatal("expected initial-map send timeout error, got nil")
+	}
+
+	nc, exists := batcher.nodes.Load(targetNode.n.ID)
+	if !exists || nc == nil {
+		t.Fatalf("expected node %d to be retained in b.nodes", targetNode.n.ID)
+	}
+
+	if nc.hasActiveConnections() {
+		t.Fatalf("expected node %d to have no active connections after timeout", targetNode.n.ID)
+	}
+
+	var phantom []tailcfg.NodeID
+
+	nc.lastSentPeers.Range(func(id tailcfg.NodeID, _ struct{}) bool {
+		phantom = append(phantom, id)
+		return true
+	})
+
+	if len(phantom) != 0 {
+		t.Errorf("lastSentPeers must be empty after a failed initial-map delivery, got %d: %v",
+			len(phantom), phantom)
+	}
+}