fix(outbounds): preserve stable subscription tags (#5)

A newly inserted subscription link could reuse the positional tag of an existing server before that server's own entry was processed later in the same refresh loop. The existing server was then bumped to a suffixed tag instead, silently repointing any routing or balancer rule that referenced the original stable tag at a different physical server.

Ported from upstream MHSanaei#6345. Reserves historical tags for identities still present in the current fetch before assigning tags to new/renamed links, so neither the positional-fallback path nor a fresh-tag collision suffix can steal a tag that rightfully belongs to another identity still in the batch.

Verified: all 8 TestAssignStableTags subtests pass with the fix; the 3 new ones confirmed to fail without it (reproduces the exact wrong-tag assignment). Reviewed by the repo's automated Claude Bot (verdict: Approve); one doc-comment suggestion applied, one pre-existing orthogonal bug (duplicate identities within a subscription) tracked separately per the bot's own recommendation not to bolt it onto this PR, one upgrade caveat noted in the PR description instead of a code change.

Upgrade note: this fix prevents the mis-assignment from recurring, it does not repair a subscription that already hit it -- see PR discussion for the recovery steps if a routing/balancer rule seems to be pointing at the wrong server on a subscription that predates this fix.

Co-Authored-By: Claude <noreply@anthropic.com>

Author: kuzzrus

internal/web/service/outbound_subscription.go

@@ -455,13 +455,21 @@ func (s *OutboundSubscriptionService) recordError(sub *model.OutboundSubscriptio
 
 // assignStableTags assigns a tag to each parsed outbound, preferring stability:
 //  1. reuse the tag previously mapped to the link's identity (prev),
-//  2. else reuse the tag at the same position from the last fetch (prevTagByIndex),
+//  2. else reuse the tag at the same position from the last fetch, unless it
+//     belongs to another identity still present in this batch (prevTagByIndex),
 //  3. else allocate a fresh tag from the prefix + remark (link.SuggestTag).
 //
 // Tags are kept unique within the batch by appending "-N" on collision, and are
 // written back into parsed[i]["tag"]. The returned slice holds the assigned tags
 // in order. When tagPrefix is empty a "sub<subID>-" prefix is used for fresh tags.
 func assignStableTags(parsed []link.Outbound, identities []string, prev map[string]string, prevTagByIndex map[int]string, subID int, tagPrefix string) []string {
+	reservedStableTags := map[string]bool{}
+	for i := range parsed {
+		if i < len(identities) && prev[identities[i]] != "" {
+			reservedStableTags[prev[identities[i]]] = true
+		}
+	}
+
 	used := map[string]bool{} // uniqueness within this refresh batch
 	assigned := make([]string, len(parsed))
 	for i := range parsed {
@@ -470,12 +478,14 @@ func assignStableTags(parsed []link.Outbound, identities []string, prev map[stri
 			id = identities[i]
 		}
 		candidate := ""
+		identityTag := ""
 		if old, ok := prev[id]; ok && old != "" {
 			candidate = old
+			identityTag = old
 		}
 		if candidate == "" {
 			// try to reuse by rough positional match from previous fetch (best effort)
-			if old, ok := prevTagByIndex[i]; ok && old != "" {
+			if old, ok := prevTagByIndex[i]; ok && old != "" && !reservedStableTags[old] {
 				candidate = old
 			}
 		}
@@ -493,7 +503,7 @@ func assignStableTags(parsed []link.Outbound, identities []string, prev map[stri
 		}
 		// ensure local uniqueness inside this batch
 		final := candidate
-		for k := 1; used[final]; k++ {
+		for k := 1; used[final] || (reservedStableTags[final] && final != identityTag); k++ {
 			final = fmt.Sprintf("%s-%d", candidate, k)
 		}
 		used[final] = true

internal/web/service/outbound_subscription_test.go

@@ -3,6 +3,7 @@ package service
 import (
 	"bytes"
 	"errors"
+	"slices"
 	"testing"
 
 	"gorm.io/gorm"
@@ -166,12 +167,53 @@ func TestAssignStableTags(t *testing.T) {
 
 	t.Run("falls back to the previous tag at the same position", func(t *testing.T) {
 		parsed := []link.Outbound{{"tag": "JP-Tokyo"}}
-		got := assignStableTags(parsed, []string{"id-new"}, map[string]string{}, map[int]string{0: "sub1-oldpos"}, 1, "")
+		prev := map[string]string{"id-gone": "sub1-oldpos"}
+		got := assignStableTags(parsed, []string{"id-new"}, prev, map[int]string{0: "sub1-oldpos"}, 1, "")
 		if got[0] != "sub1-oldpos" {
 			t.Fatalf("got %q, want sub1-oldpos", got[0])
 		}
 	})
 
+	t.Run("does not let an inserted link steal a stable tag", func(t *testing.T) {
+		parsed := []link.Outbound{{"tag": "Poland"}, {"tag": "NewServer"}, {"tag": "Netherlands"}}
+		prev := map[string]string{
+			"id-poland":      "sub1-poland",
+			"id-netherlands": "sub1-netherlands",
+		}
+		prevTagByIndex := map[int]string{0: "sub1-poland", 1: "sub1-netherlands"}
+
+		got := assignStableTags(parsed, []string{"id-poland", "id-new", "id-netherlands"}, prev, prevTagByIndex, 1, "")
+		want := []string{"sub1-poland", "sub1-newserver", "sub1-netherlands"}
+		if !slices.Equal(got, want) {
+			t.Fatalf("got %v, want %v", got, want)
+		}
+	})
+
+	t.Run("does not let a fresh tag steal a stable tag", func(t *testing.T) {
+		parsed := []link.Outbound{{"tag": "Netherlands"}, {"tag": "Renamed"}}
+		prev := map[string]string{"id-netherlands": "sub1-netherlands"}
+
+		got := assignStableTags(parsed, []string{"id-new", "id-netherlands"}, prev, nil, 1, "")
+		want := []string{"sub1-netherlands-1", "sub1-netherlands"}
+		if !slices.Equal(got, want) {
+			t.Fatalf("got %v, want %v", got, want)
+		}
+	})
+
+	t.Run("skips reserved tags while adding a suffix", func(t *testing.T) {
+		parsed := []link.Outbound{{"tag": "Netherlands"}, {"tag": "First"}, {"tag": "Second"}}
+		prev := map[string]string{
+			"id-first":  "sub1-netherlands",
+			"id-second": "sub1-netherlands-1",
+		}
+
+		got := assignStableTags(parsed, []string{"id-new", "id-first", "id-second"}, prev, nil, 1, "")
+		want := []string{"sub1-netherlands-2", "sub1-netherlands", "sub1-netherlands-1"}
+		if !slices.Equal(got, want) {
+			t.Fatalf("got %v, want %v", got, want)
+		}
+	})
+
 	t.Run("allocates a fresh tag with the default sub<id>- prefix", func(t *testing.T) {
 		parsed := []link.Outbound{{"tag": "Tokyo"}}
 		got := assignStableTags(parsed, []string{"id-x"}, nil, nil, 7, "")