fix(clients): stop recomputing the summary badges from the client_stats snapshot (#6169)

* fix(clients): stop recomputing the summary badges from the client_stats snapshot

pickClientsSummary's coverage guard (serverSummary.total >
allClientStats.length) only catches a net shortfall: an orphaned
client_traffics row and a client still missing one can cancel out, or an
orphan surplus alone can pass uncaught, and either way the guard fails to
fall back (#6116).

client_paging.go's q.summary() already derives the same bucket counts with
clients as the driving table (LEFT JOIN client_traffics), so it cannot
miscount either shape regardless of how the row got there, and listQuery
already polls it every 5s — the same cadence client_stats ticks on. The
client-side recompute bought no fresher a number than the server already
provides on its own poll, only a window to get one wrong, so this drops it:
the summary badges now always read serverSummary directly. allClientStats,
computeClientsSummary, pickClientsSummary and sameSummaryInputs are removed
as dead code along with it; the per-row live traffic patch in
applyClientStatsEvent is untouched, since it reads the same snapshot by
email match rather than by count and was never exposed to this class of bug.

* fix(clients): force a refetch on window focus and drop a stale comment

Review feedback on PR #6169:

listQuery combines staleTime: Infinity with refetchInterval: 5000, which
pauses while the tab is hidden. The WS-driven per-row traffic patch in
applyClientStatsEvent has no such visibility gating, so on a background tab
a row's live numbers keep moving while the summary badges above them freeze
at whatever they were before the tab was hidden, and staleTime: Infinity
blocks refetchOnWindowFocus from closing that gap on return. Before this
PR the client-side recompute this branch removed happened to paper over the
same underlying gap; now that it's gone, the gap is directly visible.
refetchOnWindowFocus: 'always' forces exactly one refetch on refocus,
ignoring staleTime, without touching the interval/staleTime pairing that
governs the rest of this query's behavior.

Separately, useInbounds.ts still referenced computeClientsSummary by name
in a comment explaining bucket priority; that function no longer exists
after this PR. Dropped the comment rather than repoint it, per the repo's
no-//-comment convention.

frontend/src/hooks/useClients.ts

@@ -85,51 +85,6 @@ export interface ClientSpeedEntry {
 
 type ClientStatRow = ClientTraffic & { email?: string };
 
-// Mirror of the server's buildClientsSummary (web/service/client.go). The
-// client_stats WS event already carries every client's traffic, so the
-// summary card can be recomputed live from it instead of waiting for a list
-// refetch — keep the two in lockstep.
-export function computeClientsSummary(
-  stats: ClientStatRow[],
-  onlineSet: Set<string>,
-  expireDiffMs: number,
-  trafficDiffBytes: number,
-): ClientsSummary {
-  const now = Date.now();
-  const online: string[] = [];
-  const depleted: string[] = [];
-  const expiring: string[] = [];
-  const deactive: string[] = [];
-  let active = 0;
-  for (const c of stats) {
-    const email = c.email;
-    if (!email) continue;
-    const used = (c.up || 0) + (c.down || 0);
-    const total = c.total || 0;
-    const exhausted = total > 0 && used >= total;
-    const expired = (c.expiryTime || 0) > 0 && (c.expiryTime || 0) <= now;
-    if (c.enable && onlineSet.has(email)) online.push(email);
-    if (exhausted || expired) { depleted.push(email); continue; }
-    if (!c.enable) { deactive.push(email); continue; }
-    const nearExpiry = (c.expiryTime || 0) > 0 && (c.expiryTime || 0) - now < expireDiffMs;
-    const nearLimit = total > 0 && total - used < trafficDiffBytes;
-    if (nearExpiry || nearLimit) expiring.push(email);
-    else active += 1;
-  }
-  return {
-    total: stats.length,
-    active,
-    onlineCount: online.length,
-    depletedCount: depleted.length,
-    expiringCount: expiring.length,
-    deactiveCount: deactive.length,
-    online,
-    depleted,
-    expiring,
-    deactive,
-  };
-}
-
 export function sameSpeedMap(
   a: Record<string, ClientSpeedEntry>,
   b: Record<string, ClientSpeedEntry>,
@@ -144,37 +99,6 @@ export function sameSpeedMap(
   return true;
 }
 
-// The field list computeClientsSummary reads, and deliberately nothing else.
-// lastOnline in particular churns for every online client on every push and no
-// counter depends on it, so including it here would defeat the comparison.
-export function sameSummaryInputs(a: ClientStatRow[], b: ClientStatRow[]): boolean {
-  if (a.length !== b.length) return false;
-  for (let i = 0; i < a.length; i++) {
-    const left = a[i];
-    const right = b[i];
-    if (left.email !== right.email
-      || left.up !== right.up
-      || left.down !== right.down
-      || left.total !== right.total
-      || left.enable !== right.enable
-      || left.expiryTime !== right.expiryTime) return false;
-  }
-  return true;
-}
-
-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));
@@ -272,6 +196,7 @@ export function useClients(options: UseClientsOptions = {}) {
     // List is sorted/paged server-side, so the WS patch can't add new or
     // re-sort rows; poll the current page to keep it live (pauses when hidden).
     refetchInterval: 5000,
+    refetchOnWindowFocus: 'always',
     placeholderData: keepPreviousData,
   });
 
@@ -349,17 +274,12 @@ export function useClients(options: UseClientsOptions = {}) {
   // settings request still lets the page fall back and render.
   const settingsReady = defaultsQuery.isFetched;
 
-  const [allClientStats, setAllClientStats] = useState<ClientStatRow[]>([]);
   const [clientSpeed, setClientSpeed] = useState<Record<string, ClientSpeedEntry>>({});
-  const summary = useMemo<ClientsSummary>(
-    () => pickClientsSummary(listQuery.data?.summary ?? DEFAULT_SUMMARY, allClientStats, new Set(onlines), expireDiff, trafficDiff),
-    [allClientStats, onlines, expireDiff, trafficDiff, listQuery.data?.summary],
-  );
+  const summary = listQuery.data?.summary ?? DEFAULT_SUMMARY;
 
   const invalidateAll = useCallback(
     () => {
       markLocalInvalidate();
-      setAllClientStats([]);
       return Promise.all([
         queryClient.invalidateQueries({ queryKey: keys.clients.root() }),
         queryClient.invalidateQueries({ queryKey: keys.inbounds.root() }),
@@ -658,12 +578,8 @@ export function useClients(options: UseClientsOptions = {}) {
 
   const applyClientStatsEvent = useCallback((payload: unknown) => {
     if (!payload || typeof payload !== 'object') return;
-    const p = payload as { clients?: ClientStatRow[]; snapshot?: boolean };
+    const p = payload as { clients?: ClientStatRow[] };
     if (!Array.isArray(p.clients) || p.clients.length === 0) return;
-    if (p.snapshot !== false) {
-      const rows = p.clients;
-      setAllClientStats((prev) => (sameSummaryInputs(prev, rows) ? prev : rows));
-    }
     const active = queryRef.current;
     if (!active) return;
     const byEmail = new Map<string, ClientTraffic>();

frontend/src/pages/inbounds/useInbounds.ts

@@ -261,10 +261,6 @@ export function useInbounds() {
           const stats = statsByEmail.get(client.email.toLowerCase());
           const exhausted = stats != null && stats.total > 0 && stats.up + stats.down >= stats.total;
           const expired = stats != null && stats.expiryTime > 0 && stats.expiryTime <= now;
-          // Depleted wins over disabled (same priority as computeClientsSummary):
-          // the auto-disable job also flips client.enable off in settings when a
-          // client ends, so checking enable first would file every ended client
-          // under "Disabled".
           if (expired || exhausted) {
             depleted.push(client.email);
             continue;

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

@@ -1,142 +0,0 @@
-import { describe, it, expect } from 'vitest';
-
-import { computeClientsSummary, pickClientsSummary, sameSpeedMap, sameSummaryInputs } 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)
-// or is recomputed live from the client_stats WS event. A mismatch would make
-// the summary card "jump" on refresh.
-type Row = ClientTraffic & { email?: string };
-
-const GB = 1024 * 1024 * 1024;
-const DAY = 86_400_000;
-
-function row(over: Partial<Row>): Row {
-  return { email: 'x', enable: true, up: 0, down: 0, total: 0, expiryTime: 0, ...over } as Row;
-}
-
-describe('computeClientsSummary', () => {
-  it('buckets each client the way the Go service does', () => {
-    const now = Date.now();
-    const stats: Row[] = [
-      row({ email: 'online@x', enable: true }),
-      row({ email: 'offline@x', enable: true }),
-      row({ email: 'disabled@x', enable: false }),
-      row({ email: 'exhausted@x', enable: true, total: 1 * GB, up: 1 * GB }),
-      row({ email: 'expired@x', enable: true, expiryTime: now - DAY }),
-      row({ email: 'nearexpiry@x', enable: true, expiryTime: now + DAY }),
-      row({ email: 'nearlimit@x', enable: true, total: 10 * GB, up: 9.9 * GB }),
-    ];
-    const online = new Set(['online@x', 'disabled@x']); // disabled-but-online must NOT count as online
-    const expireDiffMs = 3 * DAY;
-    const trafficDiffBytes = 1 * GB;
-
-    const s = computeClientsSummary(stats, online, expireDiffMs, trafficDiffBytes);
-
-    expect(s.total).toBe(7);
-    expect(s.online).toEqual(['online@x']);
-    expect(s.depleted.sort()).toEqual(['exhausted@x', 'expired@x']);
-    expect(s.deactive).toEqual(['disabled@x']);
-    expect(s.expiring.sort()).toEqual(['nearexpiry@x', 'nearlimit@x']);
-    expect(s.active).toBe(2); // online@x + offline@x
-  });
-
-  it('reports a counter alongside every bucket list', () => {
-    const stats: Row[] = [
-      row({ email: 'online@x', enable: true }),
-      row({ email: 'disabled@x', enable: false }),
-      row({ email: 'exhausted@x', enable: true, total: 1 * GB, up: 1 * GB }),
-      row({ email: 'nearlimit@x', enable: true, total: 10 * GB, up: 9.9 * GB }),
-    ];
-    const s = computeClientsSummary(stats, new Set(['online@x']), 3 * DAY, 1 * GB);
-
-    // The server caps its lists but never its counters; the live recompute has
-    // both, so the summary card reads the same either way.
-    expect(s.onlineCount).toBe(s.online.length);
-    expect(s.depletedCount).toBe(s.depleted.length);
-    expect(s.expiringCount).toBe(s.expiring.length);
-    expect(s.deactiveCount).toBe(s.deactive.length);
-    expect(s.active + s.depletedCount + s.expiringCount + s.deactiveCount).toBe(s.total);
-  });
-
-  it('depleted wins over disabled and over online', () => {
-    const stats: Row[] = [
-      row({ email: 'a@x', enable: false, total: 1 * GB, up: 2 * GB }),
-    ];
-    const s = computeClientsSummary(stats, new Set(['a@x']), 0, 0);
-    expect(s.depleted).toEqual(['a@x']);
-    expect(s.deactive).toEqual([]);
-    expect(s.online).toEqual([]); // disabled is never online
-  });
-
-  it('unlimited + no expiry is active', () => {
-    const stats: Row[] = [row({ email: 'a@x', enable: true, total: 0, expiryTime: 0 })];
-    const s = computeClientsSummary(stats, new Set(), 3 * DAY, 1 * GB);
-    expect(s.active).toBe(1);
-    expect(s.expiring).toEqual([]);
-    expect(s.depleted).toEqual([]);
-  });
-});
-
-describe('pickClientsSummary', () => {
-  const serverSummary: ClientsSummary = {
-    total: 67, active: 58,
-    onlineCount: 0, depletedCount: 4, expiringCount: 3, deactiveCount: 2,
-    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);
-  });
-});
-
-describe('websocket payload identity preservation', () => {
-  const speed = (up: number, down: number) => ({ up, down });
-
-  it('treats an unchanged speed map as unchanged', () => {
-    const a = { 'a@x': speed(1, 2), 'b@x': speed(3, 4) };
-    expect(sameSpeedMap(a, { 'a@x': speed(1, 2), 'b@x': speed(3, 4) })).toBe(true);
-    expect(sameSpeedMap(a, { 'a@x': speed(1, 2) })).toBe(false);
-    expect(sameSpeedMap(a, { 'a@x': speed(1, 2), 'b@x': speed(3, 5) })).toBe(false);
-    expect(sameSpeedMap(a, { 'a@x': speed(1, 2), 'c@x': speed(3, 4) })).toBe(false);
-    expect(sameSpeedMap({}, {})).toBe(true);
-  });
-
-  it('compares exactly the fields the summary reads, and ignores lastOnline', () => {
-    const base: Row[] = [row({ email: 'a@x', up: 1, down: 2, total: 10, expiryTime: 99 })];
-
-    // lastOnline moves for every online client on every push and no counter
-    // depends on it, so it must not force a new snapshot.
-    const onlyLastOnlineMoved: Row[] = [
-      row({ email: 'a@x', up: 1, down: 2, total: 10, expiryTime: 99, lastOnline: 12345 }),
-    ];
-    expect(sameSummaryInputs(base, onlyLastOnlineMoved)).toBe(true);
-
-    for (const changed of [
-      row({ email: 'b@x', up: 1, down: 2, total: 10, expiryTime: 99 }),
-      row({ email: 'a@x', up: 2, down: 2, total: 10, expiryTime: 99 }),
-      row({ email: 'a@x', up: 1, down: 3, total: 10, expiryTime: 99 }),
-      row({ email: 'a@x', up: 1, down: 2, total: 11, expiryTime: 99 }),
-      row({ email: 'a@x', up: 1, down: 2, total: 10, expiryTime: 100 }),
-      row({ email: 'a@x', up: 1, down: 2, total: 10, expiryTime: 99, enable: false }),
-    ]) {
-      expect(sameSummaryInputs(base, [changed])).toBe(false);
-    }
-    expect(sameSummaryInputs(base, [])).toBe(false);
-  });
-});

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

@@ -0,0 +1,111 @@
+import type { ReactNode } from 'react';
+import { renderHook, waitFor, act } from '@testing-library/react';
+import { QueryClientProvider } from '@tanstack/react-query';
+import { afterEach, describe, expect, it, vi } from 'vitest';
+
+import { sameSpeedMap, useClients } from '@/hooks/useClients';
+import { makeTestQueryClient } from '@/test/test-utils';
+import { HttpUtil, Msg } from '@/utils';
+import type { ClientsSummary } from '@/schemas/client';
+
+afterEach(() => {
+  vi.restoreAllMocks();
+});
+
+describe('websocket payload identity preservation', () => {
+  const speed = (up: number, down: number) => ({ up, down });
+
+  it('treats an unchanged speed map as unchanged', () => {
+    const a = { 'a@x': speed(1, 2), 'b@x': speed(3, 4) };
+    expect(sameSpeedMap(a, { 'a@x': speed(1, 2), 'b@x': speed(3, 4) })).toBe(true);
+    expect(sameSpeedMap(a, { 'a@x': speed(1, 2) })).toBe(false);
+    expect(sameSpeedMap(a, { 'a@x': speed(1, 2), 'b@x': speed(3, 5) })).toBe(false);
+    expect(sameSpeedMap(a, { 'a@x': speed(1, 2), 'c@x': speed(3, 4) })).toBe(false);
+    expect(sameSpeedMap({}, {})).toBe(true);
+  });
+});
+
+describe('client summary always reflects the server, never a client_stats recompute (#6116)', () => {
+  const serverSummary: ClientsSummary = {
+    total: 3, active: 3,
+    onlineCount: 0, depletedCount: 0, expiringCount: 0, deactiveCount: 0,
+    online: [], depleted: [], expiring: [], deactive: [],
+  };
+
+  const pagedResponse = {
+    items: [],
+    total: 3,
+    filtered: 3,
+    page: 1,
+    pageSize: 25,
+    groups: [],
+    summary: serverSummary,
+  };
+
+  function mockPanel() {
+    vi.spyOn(HttpUtil, 'get').mockImplementation(async (url: string) => {
+      if (url.includes('/clients/list/paged')) return new Msg(true, '', pagedResponse);
+      if (url.includes('/inbounds/options')) return new Msg(true, '', []);
+      return new Msg(true, '', null);
+    });
+    vi.spyOn(HttpUtil, 'post').mockImplementation(async (url: string) => {
+      if (url.includes('/setting/defaultSettings')) return new Msg(true, '', { pageSize: 25 });
+      if (url.includes('/clients/onlines')) return new Msg(true, '', []);
+      return new Msg(true, '', null);
+    });
+  }
+
+  function wrapperFor() {
+    const queryClient = makeTestQueryClient();
+    return ({ children }: { children: ReactNode }) => (
+      <QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
+    );
+  }
+
+  async function loadedHook() {
+    mockPanel();
+    const { result } = renderHook(() => useClients(), { wrapper: wrapperFor() });
+    await waitFor(() => expect(result.current.settingsReady).toBe(true));
+    act(() => {
+      result.current.setQuery({ page: 1, pageSize: 25, sort: 'createdAt', order: 'ascend' });
+    });
+    await waitFor(() => expect(result.current.fetched).toBe(true));
+    expect(result.current.summary).toEqual(serverSummary);
+    return result;
+  }
+
+  it('stays pinned to the server summary across a client_stats push carrying an orphan row with no matching gap', async () => {
+    const result = await loadedHook();
+
+    act(() => {
+      result.current.applyClientStatsEvent({
+        snapshot: true,
+        clients: [
+          { email: 'a@x', enable: true, up: 0, down: 0, total: 0, expiryTime: 0 },
+          { email: 'b@x', enable: true, up: 0, down: 0, total: 0, expiryTime: 0 },
+          { email: 'c@x', enable: true, up: 0, down: 0, total: 0, expiryTime: 0 },
+          { email: 'ghost@x', enable: false, up: 0, down: 0, total: 1, expiryTime: 1 },
+        ],
+      });
+    });
+
+    expect(result.current.summary).toEqual(serverSummary);
+  });
+
+  it('stays pinned to the server summary across a client_stats push where an orphan and a gap net out to the server total', async () => {
+    const result = await loadedHook();
+
+    act(() => {
+      result.current.applyClientStatsEvent({
+        snapshot: true,
+        clients: [
+          { email: 'a@x', enable: true, up: 0, down: 0, total: 0, expiryTime: 0 },
+          { email: 'b@x', enable: true, up: 0, down: 0, total: 0, expiryTime: 0 },
+          { email: 'ghost@x', enable: false, up: 0, down: 0, total: 1, expiryTime: 1 },
+        ],
+      });
+    });
+
+    expect(result.current.summary).toEqual(serverSummary);
+  });
+});