change: fold a tick's node deletions into one removal

Under delete churn each removal still rendered its own frame and split
the coalesced peer runs.

hscontrol/types/change/change.go

@@ -327,16 +327,45 @@ func (r Change) isPeerDelta(nodeID types.NodeID) bool {
 	return len(r.PeersChanged) > 0 || len(r.PeerPatches) > 0
 }
 
-// CoalescePeerDeltas folds each run of adjacent peer deltas in nodeID's
+// isDeletion reports whether r only announces deleted nodes, as
-// changes for one tick into a single change, so a burst of joins renders as
+// [NodeRemoved] does: [CoalescePeerDeltas] can hoist and fold such changes.
-// one [tailcfg.MapResponse] instead of one per change. Every change renders
+func (r Change) isDeletion() bool {
-// from live state after the drain, so a peer in the run's PeersChanged is sent
+	rest := r
-// whole and newer than any patch queued for it, and those patches are
+	rest.Reason, rest.TargetNode, rest.PeersRemoved, rest.DeletedNodes = "", 0, nil, nil
-// dropped. Runs never move across other changes, which keeps removals and
+
-// policy recomputes in order.
+	return len(r.DeletedNodes) > 0 && rest.IsEmpty() && rest.OriginNode == 0
+}
+
+// CoalescePeerDeltas cuts the responses nodeID's changes for one tick render
+// to, so a burst of joins or deletes renders as a few [tailcfg.MapResponse]
+// values instead of one per change. Every change renders from live state after
+// the drain:
+//   - A deleted node can no longer render anywhere and its ID is never reused,
+//     so all deletions fold into one removal ahead of the tick.
+//   - A peer in a run's PeersChanged is sent whole and newer than any patch
+//     queued for it, so each run of adjacent peer deltas folds into one change
+//     and those patches are dropped. Runs never move across other changes,
+//     which keeps other removals and policy recomputes in order.
 func CoalescePeerDeltas(nodeID types.NodeID, changes []Change) []Change {
 	out := make([]Change, 0, len(changes))
 
+	var deleted Change
+
+	for _, c := range changes {
+		if c.isDeletion() {
+			deleted.PeersRemoved = append(deleted.PeersRemoved, c.PeersRemoved...)
+			deleted.DeletedNodes = append(deleted.DeletedNodes, c.DeletedNodes...)
+		}
+	}
+
+	if deleted.DeletedNodes != nil {
+		deleted.Reason = "node removed"
+		deleted.PeersRemoved = uniqueNodeIDs(deleted.PeersRemoved)
+		deleted.DeletedNodes = uniqueNodeIDs(deleted.DeletedNodes)
+		out = append(out, deleted)
+		changes = slices.DeleteFunc(slices.Clone(changes), Change.isDeletion)
+	}
+
 	for i := 0; i < len(changes); {
 		j := i
 		for j < len(changes) && changes[j].isPeerDelta(nodeID) {

hscontrol/types/change/change_test.go

@@ -519,10 +519,20 @@ func TestCoalescePeerDeltas(t *testing.T) {
 		{
 			name: "runs do not cross other changes",
 			changes: []Change{
-				NodeAdded(2), NodeOnline(2), PolicyChange(), NodeAdded(3), NodeRemoved(4), NodeOnline(5), ping, NodeOnline(6),
+				NodeAdded(2), NodeOnline(2), PolicyChange(), NodeAdded(3), PeersRemoved(4), NodeOnline(5), ping, NodeOnline(6),
 			},
 			want: []Change{
-				merged([]types.NodeID{2}), PolicyChange(), NodeAdded(3), NodeRemoved(4), NodeOnline(5), ping, NodeOnline(6),
+				merged([]types.NodeID{2}), PolicyChange(), NodeAdded(3), PeersRemoved(4), NodeOnline(5), ping, NodeOnline(6),
+			},
+		},
+		{
+			name: "deletions fold into one removal ahead of the tick",
+			changes: []Change{
+				NodeAdded(2), NodeRemoved(5), NodeOnline(3), PolicyChange(), NodeRemoved(4), NodeAdded(6),
+			},
+			want: []Change{
+				{Reason: "node removed", PeersRemoved: []types.NodeID{4, 5}, DeletedNodes: []types.NodeID{4, 5}},
+				merged([]types.NodeID{2}, patch(NodeOnline(3))), PolicyChange(), NodeAdded(6),
 			},
 		},
 		{
@@ -579,6 +589,46 @@ func TestCoalescePeerDeltasClassifiesEveryField(t *testing.T) {
 	}
 }
 
+// TestCoalescePeerDeltasHoistsOnlyDeletions classifies every [Change] field
+// by whether a deletion setting it may still be hoisted ahead of the tick. A
+// new field fails here until it is classified.
+func TestCoalescePeerDeltasHoistsOnlyDeletions(t *testing.T) {
+	const self = types.NodeID(1)
+
+	hoists := map[string]bool{
+		"Reason":                         true,
+		"TargetNode":                     true,
+		"OriginNode":                     false,
+		"IncludeSelf":                    false,
+		"IncludeDERPMap":                 false,
+		"IncludeDNS":                     false,
+		"IncludeDomain":                  false,
+		"IncludePolicy":                  false,
+		"PeersChanged":                   false,
+		"PeersRemoved":                   true,
+		"PeerPatches":                    false,
+		"SendAllPeers":                   false,
+		"DeletedNodes":                   true,
+		"RequiresRuntimePeerComputation": false,
+		"PingRequest":                    false,
+	}
+
+	typ := reflect.TypeFor[Change]()
+	require.Len(t, hoists, typ.NumField(), "classify every Change field")
+
+	for field := range typ.Fields() {
+		t.Run(field.Name, func(t *testing.T) {
+			want, ok := hoists[field.Name]
+			require.True(t, ok, "field %s is not classified", field.Name)
+
+			c := NodeRemoved(2)
+			setNonZero(t, reflect.ValueOf(&c).Elem().FieldByIndex(field.Index), self)
+
+			assert.Equal(t, want, c.isDeletion())
+		})
+	}
+}
+
 func TestChange_Constructors(t *testing.T) {
 	tests := []struct {
 		name        string