From 2530d86f1b4d941fa49212273a1e3068a89c7757 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Fri, 17 Apr 2026 06:51:30 +0000 Subject: [PATCH] change: document PingRequest merge first-wins foot-gun change.Merge keeps the first PingRequest seen when merging, which means a later probe's callback URL is silently dropped if a stale merge is in-flight. Document the contract at Merge and at doPing so callers know to serialise probes. Updates #3157 --- hscontrol/types/change/change.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/hscontrol/types/change/change.go b/hscontrol/types/change/change.go index a1f1f8c4..0e38e498 100644 --- a/hscontrol/types/change/change.go +++ b/hscontrol/types/change/change.go @@ -1,3 +1,7 @@ +// Package change declares the Change type: a compact description of +// what must land in a MapResponse. The mapper reads Change values to +// build responses without inspecting state, and Merge combines +// multiple pending changes for a single tick. package change import ( @@ -99,6 +103,13 @@ func (r Change) Merge(other Change) Change { } // Preserve PingRequest (first wins). + // + // Foot-gun: if two PingRequests to the same target merge in the + // same tick, only the first is emitted. The client-side + // isUniquePingRequest check then suppresses the second when it + // eventually arrives, and the caller waits out the full + // pingTimeout. Call sites must avoid issuing rapid successive + // pings to one node within a single batcher tick. if merged.PingRequest == nil { merged.PingRequest = other.PingRequest } @@ -472,8 +483,9 @@ func UserRemoved() Change { return c } -// PingNode creates a Change that sends a PingRequest to a specific node. -// The node will respond to the PingRequest URL to prove connectivity. +// PingNode creates a Change that sends a PingRequest to a specific +// node. pr must be non-nil and nodeID must be non-zero; the node +// responds to the PingRequest URL to prove connectivity. func PingNode(nodeID types.NodeID, pr *tailcfg.PingRequest) Change { return Change{ Reason: "ping node",