fix: stop deleting client_traffics for detached-but-alive clients (#6110)

* fix: stop deleting client_traffics for detached-but-alive clients

MigrationRemoveOrphanedTraffics keyed "orphaned" off presence in some
inbound's settings.clients[] JSON, a definition that predates #4469's
standalone clients table. ClientService.Detach intentionally keeps a
client's traffic row when it drops its last inbound attachment (so it
can be re-attached later without losing stats/expiry), but that client
has no entry in any inbound's JSON anymore - so every x-ui migrate run
or backup restore deleted its traffic row anyway, even though the
client itself was untouched and still listed. Scope the query to the
clients table instead, which is the function's actual intent.

Separately, frontend/src/hooks/useClients.ts recomputed the clients
summary from the client_stats WS snapshot as soon as it arrived, even
when that snapshot held fewer rows than the server's own total (e.g.
exactly the gap above, or any other client with no client_traffics
row). The recompute can only bucket the clients it was given, so the
missing ones silently fell out of every bucket while the headline
total still counted them - the Ended/Disabled cards read 0 and their
hover lists were empty even though the table below listed those rows,
leaving the Filter drawer as the only way to reach them. Extracted the
decision into pickClientsSummary and added the guard: fall back to the
server summary (built from the clients table, always sums to total)
whenever the snapshot doesn't cover every client.

Fixes #6102.

* fix: union both keep-sets instead of replacing (review feedback)

Address the automated review on this PR: switching
MigrationRemoveOrphanedTraffics to key solely off the clients table
traded the original bug for a worse one. The one-shot ClientsTable
seeder (internal/database/db.go) skips a client it fails to unmarshal
and never retries, so a client still live in an inbound's
settings.clients[] JSON can have no clients row at all - the new
predicate deleted its traffic row too, and an empty clients table
would have emptied client_traffics outright. Union both keep-sets: a
row survives if it's referenced by either the clients table or any
inbound's JSON, and is removed only when it's in neither.

Log the delete's outcome instead of discarding it silently, since a
whole-table wipe would otherwise leave no trace.

Rewrote the migration test as a table of all four combinations, driven
through real ClientService calls (SyncInbound, Detach) rather than
hand-built rows wherever a real path produces the state, so it tracks
actual behavior instead of an assumption about it. Added the missing
case the review flagged: a client live in JSON only, with no clients
row, must survive.

Also stripped the // comments this PR had added - CLAUDE.md states
committed Go/TS carries none, which the review separately flagged.

frontend/src/hooks/useClients.ts

@@ -117,6 +117,19 @@ export function computeClientsSummary(
   return { total: stats.length, active, online, depleted, expiring, deactive };
 }
 
+export function pickClientsSummary(
+  serverSummary: ClientsSummary,
+  allClientStats: ClientStatRow[],
+  onlineSet: Set<string>,
+  expireDiffMs: number,
+  trafficDiffBytes: number,
+): ClientsSummary {
+  if (allClientStats.length === 0) return serverSummary;
+  if (serverSummary.total > allClientStats.length) return serverSummary;
+  const live = computeClientsSummary(allClientStats, onlineSet, expireDiffMs, trafficDiffBytes);
+  return { ...live, total: serverSummary.total || live.total };
+}
+
 function buildQS(p: ClientQueryParams): string {
   const sp = new URLSearchParams();
   sp.set('page', String(p.page || 1));
@@ -265,18 +278,12 @@ export function useClients() {
   const trafficDiff = ((defaults.trafficDiff as number) ?? 0) * 1073741824;
   const pageSize = (defaults.pageSize as number) ?? 0;
 
-  // Live summary: the client_stats WS event refreshes allClientStats every few
-  // seconds, so the top counters track reality without a page refresh. Falls
-  // back to the server-computed summary until the first event lands, and keeps
-  // the server's authoritative total for the headline count.
   const [allClientStats, setAllClientStats] = useState<ClientStatRow[]>([]);
   const [clientSpeed, setClientSpeed] = useState<Record<string, ClientSpeedEntry>>({});
-  const summary = useMemo<ClientsSummary>(() => {
-    const serverSummary = listQuery.data?.summary ?? DEFAULT_SUMMARY;
-    if (allClientStats.length === 0) return serverSummary;
-    const live = computeClientsSummary(allClientStats, new Set(onlines), expireDiff, trafficDiff);
-    return { ...live, total: serverSummary.total || live.total };
-  }, [allClientStats, onlines, expireDiff, trafficDiff, listQuery.data?.summary]);
+  const summary = useMemo<ClientsSummary>(
+    () => pickClientsSummary(listQuery.data?.summary ?? DEFAULT_SUMMARY, allClientStats, new Set(onlines), expireDiff, trafficDiff),
+    [allClientStats, onlines, expireDiff, trafficDiff, listQuery.data?.summary],
+  );
 
   const invalidateAll = useCallback(
     () => {

frontend/src/pages/clients/ClientsPage.tsx

@@ -205,7 +205,7 @@ export default function ClientsPage() {
 
   const {
     clients, total, filtered,
-    summary: serverSummary,
+    summary,
     allGroups,
     setQuery,
     inbounds, onlines, loading, transitioning, fetched, fetchError, subSettings,
@@ -385,9 +385,6 @@ export default function ClientsPage() {
   // a rename.
   const filteredClients = clients;
 
-  // Server-computed counts that stay stable as the user paginates/filters.
-  const summary = serverSummary;
-
   // Sort is server-side now; the page already arrives in the requested
   // order, so we just hand it through.
   const sortedClients = filteredClients;

frontend/src/test/clients-summary.test.ts

@@ -1,7 +1,7 @@
 import { describe, it, expect } from 'vitest';
 
-import { computeClientsSummary } from '@/hooks/useClients';
-import type { ClientTraffic } from '@/schemas/client';
+import { computeClientsSummary, pickClientsSummary } from '@/hooks/useClients';
+import type { ClientTraffic, ClientsSummary } from '@/schemas/client';
 
 // Parity with web/service/client.go buildClientsSummary: the same client must
 // land in the same bucket whether the count comes from the server (list fetch)
@@ -60,3 +60,27 @@ describe('computeClientsSummary', () => {
     expect(s.depleted).toEqual([]);
   });
 });
+
+describe('pickClientsSummary', () => {
+  const serverSummary: ClientsSummary = {
+    total: 67, active: 58, online: [], depleted: [], expiring: [], deactive: [],
+  };
+
+  it('keeps the server summary when the snapshot is short of the server total (#6102)', () => {
+    const shortSnapshot: Row[] = Array.from({ length: 58 }, (_, i) => row({ email: `c${i}@x`, enable: true }));
+    const s = pickClientsSummary(serverSummary, shortSnapshot, new Set(), 3 * DAY, 1 * GB);
+    expect(s).toEqual(serverSummary);
+  });
+
+  it('uses the live recompute when the snapshot covers every client', () => {
+    const fullSnapshot: Row[] = Array.from({ length: 67 }, (_, i) => row({ email: `c${i}@x`, enable: true }));
+    const s = pickClientsSummary(serverSummary, fullSnapshot, new Set(), 3 * DAY, 1 * GB);
+    expect(s.total).toBe(67);
+    expect(s.active).toBe(67);
+  });
+
+  it('falls back to the server summary before the first WS snapshot arrives', () => {
+    const s = pickClientsSummary(serverSummary, [], new Set(), 3 * DAY, 1 * GB);
+    expect(s).toEqual(serverSummary);
+  });
+});

internal/web/service/inbound_migration.go

@@ -19,11 +19,18 @@ import (
 func (s *InboundService) MigrationRemoveOrphanedTraffics() {
 	db := database.GetDB()
 	query := fmt.Sprintf(
-		"DELETE FROM client_traffics WHERE email NOT IN (SELECT %s %s)",
+		"DELETE FROM client_traffics WHERE email NOT IN (SELECT email FROM clients) AND email NOT IN (SELECT %s %s)",
 		database.JSONFieldText("client.value", "email"),
 		database.JSONClientsFromInbound(),
 	)
-	db.Exec(query)
+	result := db.Exec(query)
+	if result.Error != nil {
+		logger.Warning("MigrationRemoveOrphanedTraffics failed:", result.Error)
+		return
+	}
+	if result.RowsAffected > 0 {
+		logger.Infof("MigrationRemoveOrphanedTraffics: removed %d orphaned client_traffics row(s)", result.RowsAffected)
+	}
 }
 
 func (s *InboundService) MigrationRequirements() {

internal/web/service/inbound_migration_test.go

@@ -129,6 +129,65 @@ func TestMigrationRequirements_CleansLegacyZeroAddrTag(t *testing.T) {
 	}
 }
 
+func TestMigrationRemoveOrphanedTraffics(t *testing.T) {
+	setupConflictDB(t)
+	db := database.GetDB()
+	clientSvc := &ClientService{}
+	inboundSvc := &InboundService{}
+
+	const attachedEmail = "[email protected]"
+	attachedClient := model.Client{Email: attachedEmail, ID: "11111111-1111-1111-1111-111111111111", SubID: attachedEmail, Enable: true}
+	attachedIb := mkInbound(t, 30003, model.VLESS, clientsSettings(t, []model.Client{attachedClient}))
+	if err := clientSvc.SyncInbound(nil, attachedIb.Id, []model.Client{attachedClient}); err != nil {
+		t.Fatalf("seed attached client: %v", err)
+	}
+	mkTraffic(t, attachedIb.Id, attachedEmail, 0, 0, 0, 0, true)
+
+	const detachedEmail = "[email protected]"
+	detachedClient := model.Client{Email: detachedEmail, ID: "22222222-2222-2222-2222-222222222222", SubID: detachedEmail, Enable: true}
+	detachedIb := mkInbound(t, 30004, model.VLESS, clientsSettings(t, []model.Client{detachedClient}))
+	if err := clientSvc.SyncInbound(nil, detachedIb.Id, []model.Client{detachedClient}); err != nil {
+		t.Fatalf("seed detached client: %v", err)
+	}
+	mkTraffic(t, detachedIb.Id, detachedEmail, 123, 456, 0, 0, true)
+	detachedRec := lookupClientRecord(t, detachedEmail)
+	if _, err := clientSvc.Detach(inboundSvc, detachedRec.Id, []int{detachedIb.Id}); err != nil {
+		t.Fatalf("Detach: %v", err)
+	}
+
+	const jsonOnlyEmail = "[email protected]"
+	jsonOnlyClient := model.Client{Email: jsonOnlyEmail, ID: "33333333-3333-3333-3333-333333333333", SubID: jsonOnlyEmail, Enable: true}
+	jsonOnlyIb := mkInbound(t, 30005, model.VLESS, clientsSettings(t, []model.Client{jsonOnlyClient}))
+	mkTraffic(t, jsonOnlyIb.Id, jsonOnlyEmail, 0, 0, 0, 0, true)
+
+	const trulyOrphanedEmail = "[email protected]"
+	mkTraffic(t, attachedIb.Id, trulyOrphanedEmail, 0, 0, 0, 0, true)
+
+	inboundSvc.MigrationRemoveOrphanedTraffics()
+
+	cases := []struct {
+		name  string
+		email string
+		want  int64
+	}{
+		{"attached, in clients table and JSON", attachedEmail, 1},
+		{"detached-but-alive, in clients table only", detachedEmail, 1},
+		{"seeder-skipped-but-live, in JSON only", jsonOnlyEmail, 1},
+		{"truly orphaned, in neither", trulyOrphanedEmail, 0},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			var got int64
+			if err := db.Model(xray.ClientTraffic{}).Where("email = ?", c.email).Count(&got).Error; err != nil {
+				t.Fatalf("count client_traffics for %s: %v", c.email, err)
+			}
+			if got != c.want {
+				t.Errorf("client_traffics count for %s: got %d, want %d", c.email, got, c.want)
+			}
+		})
+	}
+}
+
 func TestMigrationRequirements_NormalizesShareAddressFields(t *testing.T) {
 	setupConflictDB(t)
 	db := database.GetDB()