fix(traffic): count local traffic for clients whose shared row is node-owned (#4921)

client_traffics is keyed by email (one shared row per client across every
inbound it is attached to). addClientTraffic filtered with
`inbound_id NOT IN (node inbounds)`, so when a client was attached to both a
node inbound and the mother inbound and the node inbound was attached first,
the shared row carried the node inbound's id (AddClientStat uses OnConflict
DoNothing and never refreshes it) and the local xray's traffic for that client
was dropped entirely. The client showed online but its usage stayed at zero
unless the mother inbound happened to be attached first.

Match purely by email instead. The reported emails come only from the local
xray, which only knows local-attached clients, so the query is still correctly
scoped, and this also repairs already-broken rows that a per-row AddClientStat
fix alone could not.

web/service/inbound.go

@@ -1879,9 +1879,16 @@ func (s *InboundService) addClientTraffic(tx *gorm.DB, traffics []*xray.ClientTr
 		emails = append(emails, traffic.Email)
 	}
 	dbClientTraffics := make([]*xray.ClientTraffic, 0, len(traffics))
+	// Match purely by email. client_traffics is email-keyed (one shared row per
+	// email regardless of how many inbounds the client is attached to), and these
+	// emails come from the local xray's report, so they always belong to a client
+	// attached to a local inbound. The old `inbound_id NOT IN (node inbounds)`
+	// filter dropped the local traffic of a client attached to both a node and the
+	// mother inbound whenever the node inbound happened to be attached first — its
+	// shared row then carried the node inbound's id (AddClientStat uses OnConflict
+	// DoNothing and never refreshes it), so the local poll skipped it entirely.
 	err = tx.Model(xray.ClientTraffic{}).
-		Where("email IN (?) AND inbound_id NOT IN (?)", emails,
-			tx.Model(&model.Inbound{}).Select("id").Where("node_id IS NOT NULL")).
+		Where("email IN (?)", emails).
 		Find(&dbClientTraffics).Error
 	if err != nil {
 		return err

web/service/inbound_client_traffic_test.go

@@ -10,14 +10,20 @@ import (
 	"github.com/mhsanaei/3x-ui/v3/xray"
 )
 
-// TestAddClientTraffic_MatchesDespiteStaleInboundId reproduces the production bug where
-// client_traffics rows survive an inbound delete+recreate with a stale inbound_id (the
-// shared-by-email row keeps the deleted inbound's id, and AddClientStat's OnConflict-
-// DoNothing never refreshes it). The old `inbound_id IN (local inbounds)` filter dropped
-// those rows, so local traffic and online status stopped updating. The fix matches by
-// email and only excludes rows owned by a node inbound, so a stale local row is still
-// updated while a genuine node-owned row is left untouched.
-func TestAddClientTraffic_MatchesDespiteStaleInboundId(t *testing.T) {
+// TestAddClientTraffic_MatchesByEmail covers two scenarios that share one fix:
+// client_traffics is keyed by email (one shared row per email no matter how many
+// inbounds the client is attached to), so local traffic must be applied by email
+// regardless of which inbound_id the row happens to carry.
+//
+//   - staleEmail: the row points at an inbound id that no longer exists (a deleted
+//     earlier incarnation, AddClientStat's OnConflict-DoNothing never refreshes it).
+//   - dualEmail: the client is attached to both a node inbound and the mother inbound,
+//     but the node inbound was attached first, so the shared row carries the node
+//     inbound's id (issue #4921). The old `inbound_id NOT IN (node inbounds)` filter
+//     dropped this client's local traffic, leaving it stuck at zero and offline.
+//
+// Both must have their local traffic counted.
+func TestAddClientTraffic_MatchesByEmail(t *testing.T) {
 	dbDir := t.TempDir()
 	t.Setenv("XUI_DB_FOLDER", dbDir)
 	if err := database.InitDB(filepath.Join(dbDir, "x-ui.db")); err != nil {
@@ -27,11 +33,9 @@ func TestAddClientTraffic_MatchesDespiteStaleInboundId(t *testing.T) {
 
 	db := database.GetDB()
 
-	const localEmail = "local-user"
-	const nodeEmail = "node-user"
+	const staleEmail = "stale-user"
+	const dualEmail = "dual-user"
 
-	// A local inbound exists, but the local client's traffic row points at an inbound id
-	// that no longer exists (a deleted earlier incarnation) — the stale-pointer scenario.
 	localInbound := &model.Inbound{UserId: 1, Tag: "local-in", Enable: true, Port: 40001, Protocol: model.VLESS}
 	if err := db.Create(localInbound).Error; err != nil {
 		t.Fatalf("create local inbound: %v", err)
@@ -42,39 +46,44 @@ func TestAddClientTraffic_MatchesDespiteStaleInboundId(t *testing.T) {
 		t.Fatalf("create node inbound: %v", err)
 	}
 
-	if err := db.Create(&xray.ClientTraffic{InboundId: 9999, Email: localEmail, Enable: true}).Error; err != nil {
-		t.Fatalf("create stale local client_traffics: %v", err)
+	if err := db.Create(&xray.ClientTraffic{InboundId: 9999, Email: staleEmail, Enable: true}).Error; err != nil {
+		t.Fatalf("create stale client_traffics: %v", err)
 	}
-	if err := db.Create(&xray.ClientTraffic{InboundId: nodeInbound.Id, Email: nodeEmail, Enable: true}).Error; err != nil {
-		t.Fatalf("create node client_traffics: %v", err)
+	// Attached to both inbounds, but the node inbound won the OnConflict so the
+	// shared row is owned by the node inbound id.
+	if err := db.Create(&xray.ClientTraffic{InboundId: nodeInbound.Id, Email: dualEmail, Enable: true}).Error; err != nil {
+		t.Fatalf("create dual client_traffics: %v", err)
 	}
 
 	svc := InboundService{}
 	err := svc.addClientTraffic(db, []*xray.ClientTraffic{
-		{Email: localEmail, Up: 10, Down: 20},
-		{Email: nodeEmail, Up: 30, Down: 40},
+		{Email: staleEmail, Up: 10, Down: 20},
+		{Email: dualEmail, Up: 30, Down: 40},
 	})
 	if err != nil {
 		t.Fatalf("addClientTraffic: %v", err)
 	}
 
-	var local xray.ClientTraffic
-	if err := db.Model(xray.ClientTraffic{}).Where("email = ?", localEmail).First(&local).Error; err != nil {
-		t.Fatalf("reload local row: %v", err)
+	var stale xray.ClientTraffic
+	if err := db.Model(xray.ClientTraffic{}).Where("email = ?", staleEmail).First(&stale).Error; err != nil {
+		t.Fatalf("reload stale row: %v", err)
 	}
-	if local.Up != 10 || local.Down != 20 {
-		t.Errorf("stale-pointer local row not updated: up=%d down=%d, want 10/20", local.Up, local.Down)
+	if stale.Up != 10 || stale.Down != 20 {
+		t.Errorf("stale-pointer row not updated: up=%d down=%d, want 10/20", stale.Up, stale.Down)
 	}
-	if local.LastOnline == 0 {
-		t.Errorf("stale-pointer local row LastOnline not set")
+	if stale.LastOnline == 0 {
+		t.Errorf("stale-pointer row LastOnline not set")
 	}
 
-	var node xray.ClientTraffic
-	if err := db.Model(xray.ClientTraffic{}).Where("email = ?", nodeEmail).First(&node).Error; err != nil {
-		t.Fatalf("reload node row: %v", err)
+	var dual xray.ClientTraffic
+	if err := db.Model(xray.ClientTraffic{}).Where("email = ?", dualEmail).First(&dual).Error; err != nil {
+		t.Fatalf("reload dual row: %v", err)
 	}
-	if node.Up != 0 || node.Down != 0 {
-		t.Errorf("node-owned row should not be touched by local traffic: up=%d down=%d, want 0/0", node.Up, node.Down)
+	if dual.Up != 30 || dual.Down != 40 {
+		t.Errorf("node-owned row not updated by local traffic (issue #4921): up=%d down=%d, want 30/40", dual.Up, dual.Down)
+	}
+	if dual.LastOnline == 0 {
+		t.Errorf("node-owned row LastOnline not set (client stayed offline)")
 	}
 }