hscontrol: name policy, registration and map delivery decisions

hscontrol/api/v2/devices.go

@@ -476,7 +476,8 @@ func deviceFromView(view types.NodeView, allFields bool, baseDomain string) Devi
 	}
 
 	// LastSeen is reported only when the device is offline, matching tailcfg.
-	if view.LastSeen().Valid() && view.IsOnline().Valid() && !view.IsOnline().Get() {
+	hasOfflineLastSeen := view.LastSeen().Valid() && view.IsOnline().Valid() && !view.IsOnline().Get()
+	if hasOfflineLastSeen {
 		ls := view.LastSeen().Get()
 		d.LastSeen = &ls
 	}

hscontrol/api/v2/keys.go

@@ -3,7 +3,6 @@ package apiv2
 import (
 	"context"
 	"net/http"
-	"slices"
 	"strconv"
 	"time"
 
@@ -204,7 +203,7 @@ func registerKeys(api huma.API, b Backend) {
 			// As Tailscale lists them: never an invalid key, and to a token
 			// without all=true only the keys tokens made.
 			for i := range keys {
-				if keys[i].ValidAt(now) != nil || isOAuth && !in.All && !keys[i].CreatedByToken {
+				if !keyVisibleInList(&keys[i], now, isOAuth, in.All) {
 					continue
 				}
 
@@ -326,6 +325,14 @@ func registerKeys(api huma.API, b Backend) {
 	})
 }
 
+func keyVisibleInList(key *types.PreAuthKey, now time.Time, isOAuth, all bool) bool {
+	if key.ValidAt(now) != nil {
+		return false
+	}
+
+	return !isOAuth || all || key.CreatedByToken
+}
+
 // requireKeyScope authorizes a keys operation for an OAuth access token. An admin
 // API key carries no OAuth scopes and is all-access, so it always passes.
 func requireKeyScope(ctx context.Context, need scope.Scope) error {
@@ -483,8 +490,7 @@ func createOAuthClient(ctx context.Context, b Backend, body CreateKeyRequest) (*
 
 		// An untagged "all" client may mint tokens for any tag (narrowTags), so a
 		// tag-limited token would escape its tags one client away.
-		if len(tokenTags) > 0 && len(body.Tags) == 0 &&
-			slices.Contains(body.Scopes, string(scope.All)) {
+		if len(tokenTags) > 0 && canAssignAnyTag(body.Tags, body.Scopes) {
 			return nil, huma.Error403Forbidden(
 				"client may not be granted scope all without tags by a tagged token",
 			)

hscontrol/api/v2/oauth.go

@@ -121,8 +121,7 @@ func oauthTokenHandler(b Backend) http.HandlerFunc {
 		}
 
 		granted := scope.Parse(token.Scopes)
-		if scope.Grants(granted, scope.AuthKeys) || scope.Grants(granted, scope.DevicesCore) ||
-			scope.Grants(granted, scope.OAuthKeys) {
+		if tokenCanAssignTags(granted) {
 			resp.Tags = strings.Join(token.Tags, " ")
 		}
 
@@ -130,6 +129,16 @@ func oauthTokenHandler(b Backend) http.HandlerFunc {
 	}
 }
 
+func tokenCanAssignTags(granted []scope.Scope) bool {
+	return scope.Grants(granted, scope.AuthKeys) ||
+		scope.Grants(granted, scope.DevicesCore) ||
+		scope.Grants(granted, scope.OAuthKeys)
+}
+
+func canAssignAnyTag(tags, scopes []string) bool {
+	return len(tags) == 0 && slices.Contains(scopes, string(scope.All))
+}
+
 // narrowScopes returns the requested scopes if each is granted by the client (an
 // empty request means "the client's full grant"), otherwise the offending scope
 // and false. A client holding a broad scope (e.g. "all") may mint a token limited
@@ -157,7 +166,7 @@ func narrowTags(client *types.OAuthClient, requested []string) ([]string, string
 		return client.Tags, "", true
 	}
 
-	anyTag := len(client.Tags) == 0 && slices.Contains(client.Scopes, string(scope.All))
+	anyTag := canAssignAnyTag(client.Tags, client.Scopes)
 
 	for _, req := range requested {
 		// Reject malformed tags at the trust boundary. A client's own tags are

hscontrol/debug.go

@@ -104,8 +104,7 @@ func (h *Headscale) debugHostGuard(next http.Handler) http.Handler {
 
 		host := normalize(r.Host)
 
-		_, err := netip.ParseAddr(host)
-		if host != "" && (err == nil || slices.Contains(allowed, host)) {
+		if isAllowedDebugHost(host, allowed) {
 			next.ServeHTTP(w, r)
 
 			return
@@ -120,6 +119,15 @@ func (h *Headscale) debugHostGuard(next http.Handler) http.Handler {
 	})
 }
 
+func isAllowedDebugHost(host string, allowed []string) bool {
+	_, err := netip.ParseAddr(host)
+	if host == "" {
+		return false
+	}
+
+	return err == nil || slices.Contains(allowed, host)
+}
+
 func (h *Headscale) debugHTTPServer() *http.Server {
 	debugMux := http.NewServeMux()
 	debug := tsweb.Debugger(debugMux)

hscontrol/mapper/batcher.go

@@ -848,7 +848,7 @@ func (b *Batcher) cleanupOfflineNodes() {
 	// Find nodes that have been offline for too long by scanning b.nodes
 	// and checking each node's disconnectedAt timestamp.
 	b.nodes.Range(func(nodeID types.NodeID, nc *multiChannelNodeConn) bool {
-		if nc != nil && !nc.hasActiveConnections() && nc.offlineDuration() > offlineNodeCleanupThreshold {
+		if nc.isOffline() && nc.offlineDuration() > offlineNodeCleanupThreshold {
 			nodesToCleanup = append(nodesToCleanup, nodeID)
 		}
 
@@ -864,7 +864,7 @@ func (b *Batcher) cleanupOfflineNodes() {
 		b.nodes.Compute(
 			nodeID,
 			func(conn *multiChannelNodeConn, loaded bool) (*multiChannelNodeConn, xsync.ComputeOp) {
-				if !loaded || conn == nil || conn.hasActiveConnections() {
+				if !loaded || !conn.isOffline() {
 					return conn, xsync.CancelOp
 				}
 

hscontrol/mapper/node_conn.go

@@ -340,6 +340,10 @@ func (mc *multiChannelNodeConn) isConnected() bool {
 	return mc.disconnectedAt.Load() == nil
 }
 
+func (mc *multiChannelNodeConn) isOffline() bool {
+	return mc != nil && !mc.hasActiveConnections()
+}
+
 // offlineDuration returns how long the node has been disconnected.
 // Returns 0 if the node is connected or has never been marked as disconnected.
 func (mc *multiChannelNodeConn) offlineDuration() time.Duration {
@@ -358,7 +362,7 @@ func (mc *multiChannelNodeConn) appendPending(changes ...change.Change) {
 
 	mc.pending = append(mc.pending, changes...)
 	mc.pendingSize += changesSize(changes)
-	fullAtHead := len(mc.pending) > 0 && mc.pending[0].IsFull()
+	fullAtHead := startsWithFullUpdate(mc.pending)
 
 	// Pending drains only once the bundle in flight is delivered, so behind
 	// workers it grew by every change; it renders folded anyway, and
@@ -382,8 +386,7 @@ func (mc *multiChannelNodeConn) appendPending(changes ...change.Change) {
 // online/offline.
 func (mc *multiChannelNodeConn) prependPending(changes ...change.Change) {
 	mc.pendingMu.Lock()
-	full := (len(mc.pending) > 0 && mc.pending[0].IsFull()) ||
-		(len(changes) > 0 && changes[0].IsFull())
+	full := startsWithFullUpdate(mc.pending) || startsWithFullUpdate(changes)
 
 	mc.pending = append(changes, mc.pending...)
 	mc.pendingSize += changesSize(changes)
@@ -394,6 +397,10 @@ func (mc *multiChannelNodeConn) prependPending(changes ...change.Change) {
 	mc.pendingMu.Unlock()
 }
 
+func startsWithFullUpdate(changes []change.Change) bool {
+	return len(changes) > 0 && changes[0].IsFull()
+}
+
 // collapsePendingToFull replaces pending, together with incoming, by a
 // single full update and the pings it cannot carry ([change.CollapseToFull]).
 func (mc *multiChannelNodeConn) collapsePendingToFull(incoming []change.Change) {

hscontrol/oidc.go

@@ -349,17 +349,16 @@ func (a *AuthProviderOIDC) OIDCCallbackHandler(
 	// The [oidc.UserInfo] type only decodes some fields (Subject, Profile, Email, EmailVerified).
 	// We are interested in other fields too (e.g. groups are required for allowedGroups) so we
 	// decode into our own [types.OIDCUserInfo] type using the underlying claims struct.
-	var userinfo2 types.OIDCUserInfo
-	if userinfo != nil && userinfo.Claims(&userinfo2) == nil && userinfo2.Sub == claims.Sub {
+	if userinfoClaims, ok := userInfoForSubject(userinfo, claims.Sub); ok {
 		// Update the user with the userinfo claims (with id token claims as fallback).
 		// TODO(kradalby): there might be more interesting fields here that we have not found yet.
-		claims.Email, claims.EmailVerified = mergeEmailClaims(&claims, &userinfo2)
-		claims.Username = cmp.Or(userinfo2.PreferredUsername, claims.Username)
-		claims.Name = cmp.Or(userinfo2.Name, claims.Name)
+		claims.Email, claims.EmailVerified = mergeEmailClaims(&claims, &userinfoClaims)
+		claims.Username = cmp.Or(userinfoClaims.PreferredUsername, claims.Username)
+		claims.Name = cmp.Or(userinfoClaims.Name, claims.Name)
 
-		claims.ProfilePictureURL = cmp.Or(userinfo2.Picture, claims.ProfilePictureURL)
-		if userinfo2.Groups != nil {
-			claims.Groups = userinfo2.Groups
+		claims.ProfilePictureURL = cmp.Or(userinfoClaims.Picture, claims.ProfilePictureURL)
+		if userinfoClaims.Groups != nil {
+			claims.Groups = userinfoClaims.Groups
 		}
 	}
 
@@ -394,6 +393,15 @@ func (a *AuthProviderOIDC) OIDCCallbackHandler(
 	a.beginConfirmation(writer, req, authInfo.AuthID, authInfo.Registration, user, nodeExpiry)
 }
 
+func userInfoForSubject(userinfo *oidc.UserInfo, subject string) (types.OIDCUserInfo, bool) {
+	var claims types.OIDCUserInfo
+	if userinfo == nil || userinfo.Claims(&claims) != nil {
+		return claims, false
+	}
+
+	return claims, claims.Sub == subject
+}
+
 // mergeEmailClaims keeps each verification flag with the address it describes.
 func mergeEmailClaims(
 	claims *types.OIDCClaims,
@@ -425,7 +433,8 @@ func extractCodeAndStateParamFromRequest(
 	code := query.Get("code")
 	state := query.Get("state")
 
-	if (code == "" && query.Get("error") == "") || state == "" {
+	hasAuthenticationResult := code != "" || query.Get("error") != ""
+	if !hasAuthenticationResult || state == "" {
 		return "", "", NewHTTPError(http.StatusBadRequest, "missing code or state parameter", errEmptyOIDCCallbackParams)
 	}
 

hscontrol/policy/policyutil/reduce.go

@@ -102,9 +102,7 @@ func appendOwnDests(
 	nodeAddrs *netipx.IPSet,
 ) []tailcfg.NetPortRange {
 	pfx, err := netip.ParsePrefix(dest.IP)
-	if err != nil || pfx.IsSingleIP() || nodeAddrs == nil || !nodeAddrs.ContainsPrefix(pfx) ||
-		slices.ContainsFunc(subnetRoutes, pfx.Overlaps) ||
-		slices.ContainsFunc(exitRoutes, pfx.Overlaps) {
+	if err != nil || !destinationNeedsOwnAddressExpansion(pfx, subnetRoutes, exitRoutes, nodeAddrs) {
 		return append(dests, dest)
 	}
 
@@ -119,6 +117,16 @@ func appendOwnDests(
 	return dests
 }
 
+func destinationNeedsOwnAddressExpansion(
+	pfx netip.Prefix,
+	subnetRoutes, exitRoutes []netip.Prefix,
+	nodeAddrs *netipx.IPSet,
+) bool {
+	return !pfx.IsSingleIP() && nodeAddrs != nil && nodeAddrs.ContainsPrefix(pfx) &&
+		!slices.ContainsFunc(subnetRoutes, pfx.Overlaps) &&
+		!slices.ContainsFunc(exitRoutes, pfx.Overlaps)
+}
+
 // reduceCapGrantRule filters a [tailcfg.CapGrant] rule to only include
 // [tailcfg.CapGrant] entries whose Dsts match the given node's IPs. When a
 // broad prefix (e.g. 100.64.0.0/10 from dst:*) contains a node's IP, it is

hscontrol/policy/v2/canaccess_equiv_test.go

@@ -188,10 +188,12 @@ func peerMapPrefixReference(
 	for i, a := range nodes {
 		for _, b := range nodes[i+1:] {
 			mi, mj := nodeMatchers[a.ID], nodeMatchers[b.ID]
-			if canAccessPrefixReference(a, b, mi) ||
+
+			linkedByEitherPolicy := canAccessPrefixReference(a, b, mi) ||
 				canAccessPrefixReference(b, a, mi) ||
 				canAccessPrefixReference(a, b, mj) ||
-				canAccessPrefixReference(b, a, mj) {
+				canAccessPrefixReference(b, a, mj)
+			if linkedByEitherPolicy {
 				peers[a.ID] = append(peers[a.ID], b.ID)
 				peers[b.ID] = append(peers[b.ID], a.ID)
 			}

hscontrol/policy/v2/compiled.go

@@ -130,7 +130,7 @@ func buildUserNodeIndex(
 	idx := make(userNodeIndex)
 
 	for _, n := range nodes.All() {
-		if !n.IsTagged() && n.User().Valid() {
+		if nodeHasUserIdentity(n) {
 			uid := n.User().ID()
 			idx[uid] = append(idx[uid], n)
 		}
@@ -139,6 +139,11 @@ func buildUserNodeIndex(
 	return idx
 }
 
+// A user association alone does not establish ownership: tagged nodes may retain their creator.
+func nodeHasUserIdentity(node types.NodeView) bool {
+	return !node.IsTagged() && node.User().Valid()
+}
+
 // nodeAddrSet is the addresses of every node.
 func nodeAddrSet(nodes views.Slice[types.NodeView]) *netipx.IPSet {
 	var b netipx.IPSetBuilder
@@ -852,7 +857,7 @@ func exitNodeSelfRules(
 
 		for _, uid := range slices.Sorted(maps.Keys(userIdx)) {
 			// compileAutogroupSelf already covers an untagged node's own user.
-			if !node.IsTagged() && node.User().Valid() && node.User().ID() == uid {
+			if nodeHasUserIdentity(node) && node.User().ID() == uid {
 				continue
 			}
 

hscontrol/policy/v2/filter.go

@@ -512,7 +512,7 @@ func sshPolicyForNode(
 
 		// Handle autogroup:self destinations.
 		// Tagged nodes can't match autogroup:self.
-		if cr.hasSelfDest && !node.IsTagged() && node.User().Valid() {
+		if cr.hasSelfDest && nodeHasUserIdentity(node) {
 			rules = append(rules, cr.userRules[node.User().ID()]...)
 		}
 

hscontrol/policy/v2/policy.go

@@ -695,7 +695,7 @@ func (pm *PolicyManager) sshRuleAllowsUser(rule SSH, srcNode types.NodeView, loc
 		return true
 	}
 
-	if srcNode.IsTagged() || !srcNode.User().Valid() || !rule.Users.ContainsLocalpart() {
+	if !nodeHasUserIdentity(srcNode) || !rule.Users.ContainsLocalpart() {
 		return false
 	}
 
@@ -863,12 +863,11 @@ func (pm *PolicyManager) BuildPeerMap(nodes views.Slice[types.NodeView]) map[typ
 			}
 
 			ips, ri := n.IPs(), routeInfo[n.ID()]
-			routed := len(ri.subnet) > 0
 
 			for k := range pm.matchers {
 				m := &pm.matchers[k]
 
-				if m.SrcsContainsIPs(ips...) || (routed && m.SrcsOverlapsRanges(ri.subnet)) {
+				if matcherReachesSource(m, ips, ri.subnet) {
 					if srcOf[i] == nil {
 						srcOf[i] = make([]uint64, words)
 					}
@@ -876,8 +875,7 @@ func (pm *PolicyManager) BuildPeerMap(nodes views.Slice[types.NodeView]) map[typ
 					srcOf[i][k/64] |= 1 << (k % 64) //nolint:mnd
 				}
 
-				if m.DestsContainsIP(ips...) || (routed && m.DestsOverlapsRanges(ri.subnet)) ||
-					(ri.isExit && m.DestsIsTheInternet()) {
+				if matcherReachesDestination(m, ips, ri.subnet, ri.isExit) {
 					if dstOf[i] == nil {
 						dstOf[i] = make([]uint64, words)
 					}
@@ -971,6 +969,20 @@ func (pm *PolicyManager) BuildPeerMap(nodes views.Slice[types.NodeView]) map[typ
 	return ret
 }
 
+func matcherReachesSource(m *matcher.Match, ips []netip.Addr, routes []netipx.IPRange) bool {
+	return m.SrcsContainsIPs(ips...) || (len(routes) > 0 && m.SrcsOverlapsRanges(routes))
+}
+
+func matcherReachesDestination(
+	m *matcher.Match,
+	ips []netip.Addr,
+	routes []netipx.IPRange,
+	isExit bool,
+) bool {
+	return m.DestsContainsIP(ips...) || (len(routes) > 0 && m.DestsOverlapsRanges(routes)) ||
+		(isExit && m.DestsIsTheInternet())
+}
+
 // filterRulesForNodeLocked returns the unreduced compiled filter rules
 // for a node, combining pre-compiled global rules with per-node self
 // and via rules from the stored compiled grants.
@@ -1128,14 +1140,18 @@ func (pm *PolicyManager) SetUsers(users []types.User) (bool, bool, error) {
 
 	// SSH rules and per-node filters embed user identity outside the filter
 	// hash, so a user change needs a client refresh even when it did not move.
-	if hadPerNodeFilter || pm.needsPerNodeFilter ||
-		(pm.pol != nil && len(pm.pol.SSHs) > 0) {
+	if pm.userChangeNeedsPolicyRefreshLocked(hadPerNodeFilter) {
 		policyChanged = true
 	}
 
 	return policyChanged, true, nil
 }
 
+func (pm *PolicyManager) userChangeNeedsPolicyRefreshLocked(hadPerNodeFilter bool) bool {
+	return hadPerNodeFilter || pm.needsPerNodeFilter ||
+		(pm.pol != nil && len(pm.pol.SSHs) > 0)
+}
+
 // equalUsers compares user lists ignoring order and every field the policy
 // does not read, so a row touch such as an OIDC login is not a change.
 func equalUsers(a, b []types.User) bool {
@@ -1736,8 +1752,7 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via
 								}
 							}
 
-							if viaPrimaryID == 0 || since.Before(viaPrimarySince) ||
-								(since.Equal(viaPrimarySince) && node.ID() < viaPrimaryID) {
+							if isPreferredViaPrimary(node, since, viaPrimaryID, viaPrimarySince) {
 								viaPrimaryID, viaPrimarySince = node.ID(), since
 							}
 						}
@@ -1784,11 +1799,7 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via
 			// routes the via grant added to Include.
 			for _, dstPrefix := range resolvedDstPrefixes[i] {
 				for _, p := range result.Include {
-					// Only a grant to the internet covers an exit route,
-					// as [types.Node.CanAccessRoutes] decides.
-					covers := dstPrefix.Overlaps(p) &&
-						(reachesInternet || !tsaddr.IsExitRoute(p))
-					if covers && !slices.Contains(result.UsePrimary, p) {
+					if grantCoversRoute(dstPrefix, p, reachesInternet) && !slices.Contains(result.UsePrimary, p) {
 						result.UsePrimary = append(result.UsePrimary, p)
 					}
 				}
@@ -1811,6 +1822,22 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via
 	return result
 }
 
+func isPreferredViaPrimary(
+	node types.NodeView,
+	since time.Time,
+	primaryID types.NodeID,
+	primarySince time.Time,
+) bool {
+	return primaryID == 0 || since.Before(primarySince) ||
+		(since.Equal(primarySince) && node.ID() < primaryID)
+}
+
+// Only a grant to the internet covers an exit route,
+// as [types.Node.CanAccessRoutes] decides.
+func grantCoversRoute(dst, route netip.Prefix, reachesInternet bool) bool {
+	return dst.Overlaps(route) && (reachesInternet || !tsaddr.IsExitRoute(route))
+}
+
 // grantReachesInternet reports whether a grant's destinations, resolved
 // to dsts, reach the internet by the test the grant's filter rule gets
 // ([matcher.Match.DestsIsTheInternet]). Neither the wildcard nor

hscontrol/policy/v2/sshtest.go

@@ -575,7 +575,7 @@ func sshTestDstNodes(
 		}
 
 		for _, n := range nodes.All() {
-			if !n.IsTagged() && n.User().Valid() && n.User().ID() == srcUserID {
+			if nodeHasUserIdentity(n) && n.User().ID() == srcUserID {
 				out = append(out, n)
 			}
 		}

hscontrol/policy/v2/types.go

@@ -2635,12 +2635,7 @@ func (p *Policy) validate() error {
 
 		// A check asks the source's user to log in again, and a tagged
 		// node has none; Tailscale SaaS refuses this too.
-		if ssh.Action == SSHActionCheck && slices.ContainsFunc(ssh.Sources, func(a Alias) bool {
-			_, isTag := a.(*Tag)
-			ag, _ := a.(*AutoGroup)
-
-			return isTag || ag.Is(AutoGroupTagged)
-		}) {
+		if sshCheckHasTaggedSource(ssh) {
 			errs = append(errs, ErrSSHCheckTagSource)
 		}
 
@@ -3055,6 +3050,15 @@ type SSH struct {
 	AcceptEnv    []string        `json:"acceptEnv,omitempty"`
 }
 
+func sshCheckHasTaggedSource(ssh SSH) bool {
+	return ssh.Action == SSHActionCheck && slices.ContainsFunc(ssh.Sources, func(a Alias) bool {
+		_, isTag := a.(*Tag)
+		ag, _ := a.(*AutoGroup)
+
+		return isTag || ag.Is(AutoGroupTagged)
+	})
+}
+
 // SSHSrcAliases is a list of aliases that can be used as sources in an [SSH] rule.
 // It can be a list of usernames, groups, tags or autogroups.
 type SSHSrcAliases []Alias

hscontrol/servertest/ha_property_test.go

@@ -296,8 +296,8 @@ func checkHAInvariants(
 			)
 		}
 
-		if !prevStillApproves && hasPrimary && owner != prevOwner &&
-			!st.IsNodeHealthy(owner) {
+		selectedReplacement := !prevStillApproves && hasPrimary && owner != prevOwner
+		if selectedReplacement && !st.IsNodeHealthy(owner) {
 			rt.Fatalf(
 				"prefix %s with no router available moved to %d, "+
 					"which the prober declared unreachable",

hscontrol/state/node_store.go

@@ -742,10 +742,15 @@ func resolveGivenName(
 		taken[strings.ToLower(n.GivenName)] = struct{}{}
 	}
 
+	labelAvailable := func(candidate string) bool {
+		_, busy := taken[strings.ToLower(candidate)]
+
+		return !busy && (reservedLabel == nil || !reservedLabel(candidate))
+	}
+
 	candidate := base
 	for i := 1; ; i++ {
-		_, busy := taken[strings.ToLower(candidate)]
-		if !busy && (reservedLabel == nil || !reservedLabel(candidate)) {
+		if labelAvailable(candidate) {
 			return candidate
 		}
 
@@ -907,6 +912,10 @@ func electPrimaryRoutes(
 		return servesRoutes(&n) && !n.Unhealthy
 	}
 	healthy := func(id types.NodeID) bool { return !nodes[id].Unhealthy }
+	hasOlderAvailableSession := func(id, oldest types.NodeID) bool {
+		return available(id) && (oldest == 0 ||
+			nodes[id].SessionStart.Before(nodes[oldest].SessionStart))
+	}
 
 	routes := make(map[netip.Prefix]types.NodeID, len(approvers))
 	for prefix, ids := range approvers {
@@ -914,8 +923,7 @@ func electPrimaryRoutes(
 
 		// ids ascend, so a tie keeps the lower ID.
 		for _, id := range ids {
-			if available(id) && (oldest == 0 ||
-				nodes[id].SessionStart.Before(nodes[oldest].SessionStart)) {
+			if hasOlderAvailableSession(id, oldest) {
 				oldest = id
 			}
 		}

hscontrol/state/state.go

@@ -632,8 +632,7 @@ func (s *State) UpdateUser(userID types.UserID, updateFn func(*types.User) error
 			owner := node.User()
 			credential := node.AuthKey()
 			owns := owner.Valid() && owner.ID() == user.ID
-			createdKey := credential.Valid() && credential.User().Valid() &&
-				credential.User().ID() == user.ID
+			createdKey := credentialCreatedByUser(credential, user.ID)
 
 			if !owns && !createdKey {
 				continue
@@ -647,7 +646,7 @@ func (s *State) UpdateUser(userID types.UserID, updateFn func(*types.User) error
 					ownerRefreshed = ownerRefreshed || !n.IsTagged()
 				}
 
-				if n.AuthKey != nil && n.AuthKey.User != nil && n.AuthKey.User.ID == user.ID {
+				if credentialCreatedByUser(n.AuthKey.View(), user.ID) {
 					n.AuthKey = n.AuthKey.Clone()
 					n.AuthKey.User = user.Clone()
 				}
@@ -675,6 +674,15 @@ func (s *State) UpdateUser(userID types.UserID, updateFn func(*types.User) error
 	return user, c, nil
 }
 
+func credentialCreatedByUser(credential types.CredentialView, userID uint) bool {
+	return credential.Valid() && credential.User().Valid() && credential.User().ID() == userID
+}
+
+func taggedCredentialCreatedByUser(credential types.CredentialView, userID types.UserID) bool {
+	return credential.Valid() && credential.Tags().Len() > 0 && credential.UserID().Valid() &&
+		types.UserID(credential.UserID().Get()) == userID
+}
+
 // DeleteUser permanently removes a user and all associated data (nodes, API keys, etc).
 // This operation is irreversible.
 // It also updates the policy manager to ensure ACL policies referencing the deleted
@@ -689,7 +697,7 @@ func (s *State) DeleteUser(userID types.UserID) (change.Change, error) {
 	// must stop naming it, as the database already does.
 	dropCreator := func(n *types.Node) {
 		k := n.AuthKey
-		if k == nil || !k.IsTagged() || k.UserID == nil || types.UserID(*k.UserID) != userID {
+		if !taggedCredentialCreatedByUser(k.View(), userID) {
 			return
 		}
 
@@ -701,8 +709,7 @@ func (s *State) DeleteUser(userID types.UserID) (change.Change, error) {
 	updates := make(map[types.NodeID]UpdateNodeFunc)
 
 	for _, n := range s.nodeStore.ListNodes().All() {
-		if k := n.AuthKey(); k.Valid() && k.Tags().Len() > 0 && k.UserID().Valid() &&
-			types.UserID(k.UserID().Get()) == userID {
+		if taggedCredentialCreatedByUser(n.AuthKey(), userID) {
 			updates[n.ID()] = dropCreator
 		}
 	}
@@ -1206,11 +1213,15 @@ func (s *State) BringNodeExpiryForward(
 	expiry time.Time,
 ) (types.NodeView, change.Change, error) {
 	return s.setNodeExpiry(nodeID, &expiry, func(node *types.Node) bool {
-		return node.NodeKey == nodeKey &&
-			(node.Expiry == nil || node.Expiry.IsZero() || expiry.Before(*node.Expiry))
+		return canBringExpiryForward(node, nodeKey, expiry)
 	})
 }
 
+func canBringExpiryForward(node *types.Node, nodeKey key.NodePublic, expiry time.Time) bool {
+	return node.NodeKey == nodeKey &&
+		(node.Expiry == nil || node.Expiry.IsZero() || expiry.Before(*node.Expiry))
+}
+
 // setNodeExpiry sets the node's expiry. A non-nil allow decides on the node
 // as the writer holds it; a refusal writes nothing.
 func (s *State) setNodeExpiry(
@@ -2010,7 +2021,7 @@ func (s *State) BatchSetNodeHealth(updates map[types.NodeID]bool) bool {
 
 	for id := range fns {
 		nv, ok := s.nodeStore.GetNode(id)
-		if ok && nv.Unhealthy() == !updates[id] && s.polMan.NodeIsViaTarget(nv) {
+		if ok && s.viaNodeHasHealth(nv, updates[id]) {
 			return true
 		}
 	}
@@ -2018,6 +2029,10 @@ func (s *State) BatchSetNodeHealth(updates map[types.NodeID]bool) bool {
 	return false
 }
 
+func (s *State) viaNodeHasHealth(node types.NodeView, healthy bool) bool {
+	return node.Unhealthy() == !healthy && s.polMan.NodeIsViaTarget(node)
+}
+
 // healthSetter returns an UpdateNodeFunc that flips n.Unhealthy to
 // the inverse of healthy, with the same gate as [State.SetNodeHealth]:
 // an unhealthy mark only sticks when the node is still online and
@@ -2319,8 +2334,7 @@ func (s *State) SetAuthCacheEntry(id types.AuthID, entry *types.AuthRequest) err
 		s.authCache.Remove(oldest)
 	}
 
-	if (!entry.IsSSHCheck() && s.authCacheRegularEntries.Load() >= int64(s.authCacheMaxEntries)) ||
-		s.authCache.Len() >= s.authCacheMaxEntries+authCacheSSHReserve {
+	if s.authCacheFullFor(entry) {
 		return ErrAuthCacheFull
 	}
 
@@ -2338,6 +2352,12 @@ func (s *State) SetAuthCacheEntry(id types.AuthID, entry *types.AuthRequest) err
 	return nil
 }
 
+// authCacheFullFor is called with authCacheMu held.
+func (s *State) authCacheFullFor(entry *types.AuthRequest) bool {
+	return (!entry.IsSSHCheck() && s.authCacheRegularEntries.Load() >= int64(s.authCacheMaxEntries)) ||
+		s.authCache.Len() >= s.authCacheMaxEntries+authCacheSSHReserve
+}
+
 // DeleteAuthCacheEntry releases a consumed authentication transaction.
 func (s *State) DeleteAuthCacheEntry(id types.AuthID) {
 	s.authCache.Remove(id)
@@ -2588,10 +2608,7 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView
 		// Apply default node expiry for non-tagged nodes when the
 		// resolved expiry is still nil or zero (e.g., CLI registration
 		// where the client did not request a specific expiry).
-		needsDefaultExpiry := !node.IsTagged() &&
-			(node.Expiry == nil || node.Expiry.IsZero()) &&
-			s.cfg.Node.Expiry > 0
-		if needsDefaultExpiry {
+		if s.needsDefaultNodeExpiry(node) {
 			exp := time.Now().Add(s.cfg.Node.Expiry)
 			node.Expiry = &exp
 		}
@@ -2747,10 +2764,7 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro
 	// Apply default node expiry for non-tagged nodes when the client
 	// did not request a specific expiry.
 	// Tagged nodes are exempt — they never expire.
-	needsDefaultExpiry := !nodeToRegister.IsTagged() &&
-		(nodeToRegister.Expiry == nil || nodeToRegister.Expiry.IsZero()) &&
-		s.cfg.Node.Expiry > 0
-	if needsDefaultExpiry {
+	if s.needsDefaultNodeExpiry(&nodeToRegister) {
 		exp := time.Now().Add(s.cfg.Node.Expiry)
 		nodeToRegister.Expiry = &exp
 	}
@@ -2839,6 +2853,15 @@ func (s *State) validateRequestTags(node types.NodeView, requestTags []string) [
 	return rejectedTags
 }
 
+func (s *State) needsDefaultNodeExpiry(node *types.Node) bool {
+	return !node.IsTagged() && (node.Expiry == nil || node.Expiry.IsZero()) && s.cfg.Node.Expiry > 0
+}
+
+func (s *State) canApplyReauthTag(node types.NodeView, authUser types.UserView, tag string) bool {
+	return s.polMan.NodeCanHaveTag(node, tag) ||
+		(authUser.Valid() && s.polMan.UserCanHaveTag(authUser, tag))
+}
+
 // validateRequestTagsForReauth authorises re-auth request tags against the
 // existing node OR the authenticating user. A tag-owned node (#3374) has no
 // user and its IP is in no owner set, so NodeCanHaveTag alone rejects every
@@ -2853,11 +2876,7 @@ func (s *State) validateRequestTagsForReauth(node types.NodeView, authUser types
 	var rejectedTags []string
 
 	for _, tag := range requestTags {
-		if s.polMan.NodeCanHaveTag(node, tag) {
-			continue
-		}
-
-		if authUser.Valid() && s.polMan.UserCanHaveTag(authUser, tag) {
+		if s.canApplyReauthTag(node, authUser, tag) {
 			continue
 		}
 
@@ -2926,8 +2945,7 @@ func (s *State) processReauthTags(
 	var approvedTags, rejectedTags []string
 
 	for _, tag := range requestTags {
-		if s.polMan.NodeCanHaveTag(node.View(), tag) ||
-			(authUser.Valid() && s.polMan.UserCanHaveTag(authUser, tag)) {
+		if s.canApplyReauthTag(node.View(), authUser, tag) {
 			approvedTags = append(approvedTags, tag)
 		} else {
 			rejectedTags = append(rejectedTags, tag)
@@ -3038,7 +3056,7 @@ func (s *State) HandleNodeFromAuthPath(
 	existingNodeOwnedByOtherUser := false
 
 	for uid, n := range all {
-		if uid != 0 && uid != types.UserID(user.ID) && !n.IsTagged() {
+		if isOtherUserOwnedNode(n, uid, types.UserID(user.ID)) {
 			existingNodeOtherUser = n
 			existingNodeOwnedByOtherUser = true
 		}
@@ -3226,7 +3244,7 @@ func (s *State) findExistingNodeForPAK(
 			// key under its own node key is the device restarting (#3312);
 			// pakReregister lets that key only wake it.
 			node, ok = all[0]
-			ok = ok && node.IsTagged() && (pak.CanReauthenticate(node) || node.NodeKey() == nodeKey)
+			ok = ok && canReuseTaggedNodeWithKey(node, pak, nodeKey)
 		}
 
 		return node, ok, nil
@@ -3252,14 +3270,25 @@ func loginNode(all map[types.UserID]types.NodeView, userID types.UserID) (types.
 		return node, true
 	}
 
-	if node, ok := all[0]; ok && node.IsTagged() &&
-		(node.ClaimableBy(userID) || node.RegisterMethod() != util.RegisterMethodAuthKey) {
+	if node, ok := all[0]; ok && canClaimTaggedNode(node, userID) {
 		return node, true
 	}
 
 	return types.NodeView{}, false
 }
 
+func isOtherUserOwnedNode(node types.NodeView, ownerID, userID types.UserID) bool {
+	return ownerID != 0 && ownerID != userID && !node.IsTagged()
+}
+
+func canReuseTaggedNodeWithKey(node types.NodeView, pak *types.PreAuthKey, nodeKey key.NodePublic) bool {
+	return node.IsTagged() && (pak.CanReauthenticate(node) || node.NodeKey() == nodeKey)
+}
+
+func canClaimTaggedNode(node types.NodeView, userID types.UserID) bool {
+	return node.IsTagged() && (node.ClaimableBy(userID) || node.RegisterMethod() != util.RegisterMethodAuthKey)
+}
+
 // pakSkipsValidation reports whether re-registering n with pak may skip key
 // validation at now. The machine key proves identity and a pre-auth key only
 // authorises the initial join, so a plain restart (container "tailscale up
@@ -3315,6 +3344,50 @@ func pakReregister(n types.NodeView, pak *types.PreAuthKey, nodeKey key.NodePubl
 	return false, err
 }
 
+// requestedTagsForPreAuthKey returns an authorised replacement set, or nil
+// when the key's tags should be kept.
+// Advertising a subset of the key's tags is redundant, not an escalation,
+// and the tailscale client's OAuth authkey flow always does it. Other
+// requested tags replace the key's when a tagged key's tags own each one,
+// or a user's key's user owns it directly, as on Tailscale SaaS; otherwise
+// they are refused, ahead of the key's own error, so new nodes and
+// re-registrations are held to the same rule.
+func (s *State) requestedTagsForPreAuthKey(hostinfo *tailcfg.Hostinfo, pak *types.PreAuthKey) ([]string, error) {
+	if !requestsTagsOutsideKey(hostinfo, pak.Tags) {
+		return nil, nil
+	}
+
+	var refused []string
+
+	for _, tag := range hostinfo.RequestTags {
+		if !s.preAuthKeyCanOwnTag(pak, tag) {
+			refused = append(refused, tag)
+		}
+	}
+
+	if len(refused) > 0 {
+		return nil, fmt.Errorf("%w %v are invalid or not permitted", ErrRequestedTagsInvalidOrNotPermitted, refused)
+	}
+
+	// Sort a copy: RequestTags is the node's reported Hostinfo.
+	return slices.Compact(slices.Sorted(slices.Values(hostinfo.RequestTags))), nil
+}
+
+func requestsTagsOutsideKey(hostinfo *tailcfg.Hostinfo, keyTags []string) bool {
+	return hostinfo != nil && slices.ContainsFunc(hostinfo.RequestTags, func(tag string) bool {
+		return !slices.Contains(keyTags, tag)
+	})
+}
+
+func (s *State) preAuthKeyCanOwnTag(pak *types.PreAuthKey, tag string) bool {
+	return s.polMan.TagOwnedByTags(tag, pak.Tags) ||
+		(!pak.IsTagged() && pak.User != nil && s.polMan.UserCanHaveTag(pak.User.View(), tag))
+}
+
+func requestedTagsErrorTakesPrecedence(tagsErr, keyErr error, reauth bool) bool {
+	return tagsErr != nil && !errors.Is(keyErr, ErrNodeKeyExpired) && (reauth || keyErr != nil)
+}
+
 //nolint:gocyclo // sequential validation/update/create paths with security-sensitive ordering
 func (s *State) HandleNodeFromPreAuthKey(
 	regReq tailcfg.RegisterRequest,
@@ -3332,7 +3405,7 @@ func (s *State) HandleNodeFromPreAuthKey(
 		return types.NodeView{}, s.policyChangeSince(genBefore), err
 	}
 
-	if !pak.Reusable && !pak.Used {
+	if shouldConsumePreAuthKey(pak) {
 		mu := &s.singleUseKeyLocks[pak.ID%uint64(len(s.singleUseKeyLocks))]
 		mu.Lock()
 		defer mu.Unlock()
@@ -3344,37 +3417,7 @@ func (s *State) HandleNodeFromPreAuthKey(
 		}
 	}
 
-	// Advertising a subset of the key's tags is redundant, not an escalation,
-	// and the tailscale client's OAuth authkey flow always does it. Other
-	// requested tags replace the key's when a tagged key's tags own each one,
-	// or a user's key's user owns it directly, as on Tailscale SaaS; otherwise
-	// they are refused, ahead of the key's own error, so new nodes and
-	// re-registrations are held to the same rule.
-	var (
-		requestedTags []string
-		tagsErr       error
-	)
-
-	if regReq.Hostinfo != nil && slices.ContainsFunc(regReq.Hostinfo.RequestTags, func(tag string) bool {
-		return !slices.Contains(pak.Tags, tag)
-	}) {
-		var refused []string
-
-		for _, tag := range regReq.Hostinfo.RequestTags {
-			owned := s.polMan.TagOwnedByTags(tag, pak.Tags) ||
-				(!pak.IsTagged() && pak.User != nil && s.polMan.UserCanHaveTag(pak.User.View(), tag))
-			if !owned {
-				refused = append(refused, tag)
-			}
-		}
-
-		if len(refused) > 0 {
-			tagsErr = fmt.Errorf("%w %v are invalid or not permitted", ErrRequestedTagsInvalidOrNotPermitted, refused)
-		} else {
-			// Sort a copy: RequestTags is the node's reported Hostinfo.
-			requestedTags = slices.Compact(slices.Sorted(slices.Values(regReq.Hostinfo.RequestTags)))
-		}
-	}
+	requestedTags, tagsErr := s.requestedTagsForPreAuthKey(regReq.Hostinfo, pak)
 
 	existingNodeSameUser, existsSameUser, err := s.findExistingNodeForPAK(machineKey, pak, regReq.NodeKey)
 	if err != nil {
@@ -3392,7 +3435,7 @@ func (s *State) HandleNodeFromPreAuthKey(
 	}
 
 	// A wake-up writes no tags, so the tags it asks for are not refused.
-	if tagsErr != nil && !errors.Is(err, ErrNodeKeyExpired) && (reauth || err != nil) {
+	if requestedTagsErrorTakesPrecedence(tagsErr, err, reauth) {
 		return types.NodeView{}, s.policyChangeSince(genBefore), tagsErr
 	}
 
@@ -3450,7 +3493,7 @@ func (s *State) HandleNodeFromPreAuthKey(
 
 		if pak.User != nil {
 			for uid, node := range s.nodeStore.GetNodesByMachineKeyAllUsers(machineKey) {
-				if uid != 0 && !node.IsTagged() && uid != types.UserID(pak.User.ID) {
+				if isOtherUserOwnedNode(node, uid, types.UserID(pak.User.ID)) {
 					differentUserNode = node
 					belongsToDifferentUser = true
 				}
@@ -3551,8 +3594,7 @@ func (s *State) approveRoutesAfterRegistration(
 	before, node types.NodeView,
 	c change.Change,
 ) (types.NodeView, change.Change, error) {
-	if before.Valid() && before.NodeKey() == node.NodeKey() &&
-		slices.Equal(sortedPrefixes(before.AnnouncedRoutes()), sortedPrefixes(node.AnnouncedRoutes())) {
+	if registrationPreservesRouteInputs(before, node) {
 		return node, c, nil
 	}
 
@@ -3566,6 +3608,19 @@ func (s *State) approveRoutesAfterRegistration(
 	return node, c, nil
 }
 
+func registrationPreservesRouteInputs(before, node types.NodeView) bool {
+	return before.Valid() && before.NodeKey() == node.NodeKey() &&
+		slices.Equal(sortedPrefixes(before.AnnouncedRoutes()), sortedPrefixes(node.AnnouncedRoutes()))
+}
+
+func preAuthKeyClaimsTaggedNode(node *types.Node, pak *types.PreAuthKey, requestedTags []string, previousNodeKey key.NodePublic) bool {
+	return requestedTags == nil && node.IsTagged() && !pak.IsTagged() && previousNodeKey != node.NodeKey
+}
+
+func shouldConsumePreAuthKey(pak *types.PreAuthKey) bool {
+	return !pak.Reusable && !pak.Used
+}
+
 // reregisterNodeWithPAK updates an existing node in place for a pre-auth key
 // re-registration: NodeStore first, then the database.
 //
@@ -3686,8 +3741,7 @@ func (s *State) reregisterNodeWithPAK(
 			if wasUserOwned || node.IsExpiredAt(now) {
 				node.Expiry = nil
 			}
-		} else if requestedTags == nil && node.IsTagged() && !pak.IsTagged() &&
-			prior.NodeKey != regReq.NodeKey {
+		} else if preAuthKeyClaimsTaggedNode(node, pak, requestedTags, prior.NodeKey) {
 			// A user key that speaks for a tagged node, with a new node key,
 			// takes it back as that user's device, as on Tailscale SaaS. A
 			// relogin under the same node key keeps it tagged. As at a login
@@ -3703,7 +3757,7 @@ func (s *State) reregisterNodeWithPAK(
 		// If this registration will consume a single-use key (the tx below
 		// calls UsePreAuthKey under the same condition), reflect that in the
 		// cached AuthKey so the NodeStore copy matches the database.
-		if !pak.Reusable && !pak.Used {
+		if shouldConsumePreAuthKey(pak) {
 			node.AuthKey.Used = true
 		}
 
@@ -3766,7 +3820,7 @@ func (s *State) reregisterNodeWithPAK(
 		// atomic compare-and-set in [hsdb.UsePreAuthKey] would otherwise
 		// reject it as "authkey already used", locking out containers
 		// that restart with the same one-shot key.
-		if reauth && !pak.Reusable && !pak.Used {
+		if reauth && shouldConsumePreAuthKey(pak) {
 			err := hsdb.UsePreAuthKey(tx, pak)
 			if err != nil {
 				return fmt.Errorf("using pre auth key: %w", err)
@@ -3793,6 +3847,11 @@ func (s *State) reregisterNodeWithPAK(
 	return updatedNodeView, before, nil
 }
 
+func registrationOwnershipStillMatches(node, written *types.Node) bool {
+	return slices.Equal(node.Tags, written.Tags) &&
+		reflect.DeepEqual(node.UserID, written.UserID) && reflect.DeepEqual(node.User, written.User)
+}
+
 // undoReregistration puts back the fields of a failed re-registration that
 // still hold what it wrote, so later admin and map writes survive, and saves
 // the result. LastSeen stays because the node did contact us; sessions,
@@ -3815,9 +3874,7 @@ func (s *State) undoReregistration(prior, written *types.Node) {
 			n.RegisterMethod = prior.RegisterMethod
 		}
 
-		if slices.Equal(n.Tags, written.Tags) &&
-			reflect.DeepEqual(n.UserID, written.UserID) &&
-			reflect.DeepEqual(n.User, written.User) {
+		if registrationOwnershipStillMatches(n, written) {
 			n.Tags = prior.Tags
 			n.UserID = prior.UserID
 			n.User = prior.User
@@ -4061,6 +4118,14 @@ func isAutoDerivedGivenName(given, hostname string) bool {
 	return err == nil && given == bumpedLabel(base, n)
 }
 
+func dnsMetadataChanged(previous, reported *tailcfg.Hostinfo) bool {
+	return reported != nil && (previous == nil || previous.Hostname != reported.Hostname || previous.OS != reported.OS)
+}
+
+func hasReportedHostnameChange(hostname string, reported *tailcfg.Hostinfo) bool {
+	return reported != nil && reported.Hostname != "" && reported.Hostname != hostname
+}
+
 // UpdateNodeFromMapRequest is the sync point where Hostinfo changes,
 // endpoint updates, and route advertisements from a [tailcfg.MapRequest]
 // land in the [NodeStore]. It produces a [change.Change] summarising
@@ -4154,10 +4219,7 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest
 			!hostinfoEqual(currentNode.Hostinfo, newHostinfo)
 		delta.peerHostinfoChanged = newHostinfo != nil &&
 			!peerHostinfoEqual(currentNode.Hostinfo.View(), newHostinfo.View())
-		delta.dnsMetadataChanged = newHostinfo != nil &&
-			(currentNode.Hostinfo == nil ||
-				currentNode.Hostinfo.Hostname != newHostinfo.Hostname ||
-				currentNode.Hostinfo.OS != newHostinfo.OS)
+		delta.dnsMetadataChanged = dnsMetadataChanged(currentNode.Hostinfo, newHostinfo)
 
 		// A change carrying only an updated LastSeen is not worth a
 		// full-row database UPDATE plus the O(n) policy rescan
@@ -4181,8 +4243,7 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest
 
 		// Only a real hostname change may re-derive GivenName: it is peer
 		// visible, so the whole node is resent and peers learn the name.
-		if newHostinfo != nil && newHostinfo.Hostname != "" &&
-			newHostinfo.Hostname != currentNode.Hostname {
+		if hasReportedHostnameChange(currentNode.Hostname, newHostinfo) {
 			// Preserve an admin-renamed GivenName: only auto-derive
 			// when the current GivenName is still what
 			// SanitizeHostname of the old Hostname would produce

hscontrol/types/change/change.go

@@ -131,8 +131,7 @@ func (r Change) Merge(other Change) Change {
 }
 
 func (r Change) IsEmpty() bool {
-	if r.IncludeSelf || r.IncludeDERPMap || r.IncludeDNS ||
-		r.IncludeDomain || r.IncludePolicy || r.SendAllPeers {
+	if r.IncludeSelf || r.hasConfigUpdate() || r.SendAllPeers {
 		return false
 	}
 
@@ -144,10 +143,7 @@ func (r Change) IsEmpty() bool {
 		return false
 	}
 
-	return len(r.PeersChanged) == 0 &&
-		len(r.PeersRemoved) == 0 &&
-		len(r.DeletedNodes) == 0 &&
-		len(r.PeerPatches) == 0
+	return !r.hasPeerChanges() && len(r.PeerPatches) == 0
 }
 
 func (r Change) IsSelfOnly() bool {
@@ -155,8 +151,7 @@ func (r Change) IsSelfOnly() bool {
 		return false
 	}
 
-	if r.SendAllPeers || len(r.PeersChanged) > 0 || len(r.PeersRemoved) > 0 ||
-		len(r.DeletedNodes) > 0 || len(r.PeerPatches) > 0 {
+	if r.SendAllPeers || r.hasPeerChanges() || len(r.PeerPatches) > 0 {
 		return false
 	}
 
@@ -195,16 +190,15 @@ func (r Change) Type() string {
 		return "policy"
 	}
 
-	if len(r.PeerPatches) > 0 && len(r.PeersChanged) == 0 && len(r.PeersRemoved) == 0 && !r.SendAllPeers {
+	if len(r.PeerPatches) > 0 && !r.hasPeerRefresh() {
 		return "patch"
 	}
 
-	if len(r.PeersChanged) > 0 || len(r.PeersRemoved) > 0 ||
-		len(r.DeletedNodes) > 0 || r.SendAllPeers {
+	if r.hasPeerChanges() || r.SendAllPeers {
 		return "peers"
 	}
 
-	if r.IncludeDERPMap || r.IncludeDNS || r.IncludeDomain || r.IncludePolicy {
+	if r.hasConfigUpdate() {
 		return "config"
 	}
 
@@ -215,6 +209,18 @@ func (r Change) Type() string {
 	return "unknown"
 }
 
+func (r Change) hasConfigUpdate() bool {
+	return r.IncludeDERPMap || r.IncludeDNS || r.IncludeDomain || r.IncludePolicy
+}
+
+func (r Change) hasPeerChanges() bool {
+	return len(r.PeersChanged) > 0 || len(r.PeersRemoved) > 0 || len(r.DeletedNodes) > 0
+}
+
+func (r Change) hasPeerRefresh() bool {
+	return len(r.PeersChanged) > 0 || len(r.PeersRemoved) > 0 || r.SendAllPeers
+}
+
 // ShouldSendToNode determines if this response should be sent to nodeID.
 // It handles self-only targeting and filtering out self-updates for non-origin nodes.
 func (r Change) ShouldSendToNode(nodeID types.NodeID) bool {
@@ -315,8 +321,7 @@ func DedupePolicyChanges(changes []Change) []Change {
 // isPeerDelta reports whether r only adds, refreshes or patches peers other
 // than nodeID: [CoalescePeerDeltas] can fold such changes together.
 func (r Change) isPeerDelta(nodeID types.NodeID) bool {
-	if r.IncludeSelf || r.IncludeDERPMap || r.IncludeDNS || r.IncludeDomain ||
-		r.IncludePolicy || r.SendAllPeers || r.RequiresRuntimePeerComputation {
+	if r.IncludeSelf || r.hasConfigUpdate() || r.SendAllPeers || r.RequiresRuntimePeerComputation {
 		return false
 	}