3x-ui

3,436 Commits 1 Branch 129 Tags

3 Commits

Author	SHA1	Message	Date
Mr. Nickson	ecadfd0e60	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.	2026-08-14 16:45:31 +02:00
Mr. Nickson	ff954ec48c	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.	2026-07-28 22:14:01 +02:00
Mr. Nickson	8cd71e07ea	fix: refresh stale client_traffics row when an inbound-deleted client's email is reused (#6003 ) * fix: refresh stale client_traffics row when an inbound-deleted client's email is reused AddClientStat's OnConflict was DoNothing on email, so once an inbound is deleted (DelInbound only removes the client_inbounds link, matching ClientService.Detach's intentional Detach-then-later-Attach behavior) the orphaned client_traffics row for that email survives untouched. Re-creating a client under the same email on a new inbound silently kept the old enable/expiry_time/reset/total/inbound_id instead of adopting the new client's config. Switch the conflict path to DoUpdates on inbound_id/total/expiry_time/ enable/reset. up/down stay excluded on purpose: every call for an already-attached identity carries the same config values (one call per inbound), so the refresh is a no-op for that legitimate multi-inbound share, while zeroing usage counters on each additional attach would erase real traffic. Fixes #5958 * fix: don't let AddClientStat clobber import's forced-enabled ClientStats rows github-actions[bot] review on #6003 found that AddInbound writes client_traffics twice for the same import payload: first inserting each ClientStats row (DoNothing, with Enable forced true by controller.importInbound), then calling AddClientStat once per Settings-derived client. With AddClientStat's OnConflict now DoUpdates, that second call was unconditionally overwriting enable (and total/expiry_time/reset/inbound_id) with the Settings.clients[].enable value — which still holds whatever the client had at export time, silently undoing the controller's "always import as enabled" behavior for any client disabled at export. Fix: track which emails were already seeded by the ClientStats loop and skip the AddClientStat call for those emails, leaving the import path's forced values as authoritative. Plain (non-import) creates are unaffected since ClientStats is empty there, so every client still goes through AddClientStat's refresh as before. Also updated a stale comment in addClientTraffic that still described AddClientStat as DoNothing. Added TestAddInbound_ImportForcedEnableSurvivesDisabledSettingsClient, which reproduces the exact regression (verified it fails without this fix) and passes with it.	2026-07-21 15:57:55 +02:00

Author

SHA1

Message

Date

Mr. Nickson

ecadfd0e60

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.

2026-08-14 16:45:31 +02:00

Mr. Nickson

ff954ec48c

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.

2026-07-28 22:14:01 +02:00

Mr. Nickson

8cd71e07ea

fix: refresh stale client_traffics row when an inbound-deleted client's email is reused (#6003 )

* fix: refresh stale client_traffics row when an inbound-deleted client's email is reused

AddClientStat's OnConflict was DoNothing on email, so once an inbound is
deleted (DelInbound only removes the client_inbounds link, matching
ClientService.Detach's intentional Detach-then-later-Attach behavior) the
orphaned client_traffics row for that email survives untouched. Re-creating
a client under the same email on a new inbound silently kept the old
enable/expiry_time/reset/total/inbound_id instead of adopting the new
client's config.

Switch the conflict path to DoUpdates on inbound_id/total/expiry_time/
enable/reset. up/down stay excluded on purpose: every call for an
already-attached identity carries the same config values (one call per
inbound), so the refresh is a no-op for that legitimate multi-inbound
share, while zeroing usage counters on each additional attach would erase
real traffic.

Fixes #5958

* fix: don't let AddClientStat clobber import's forced-enabled ClientStats rows

github-actions[bot] review on #6003 found that AddInbound writes client_traffics
twice for the same import payload: first inserting each ClientStats row
(DoNothing, with Enable forced true by controller.importInbound), then calling
AddClientStat once per Settings-derived client. With AddClientStat's OnConflict
now DoUpdates, that second call was unconditionally overwriting enable (and
total/expiry_time/reset/inbound_id) with the Settings.clients[].enable value —
which still holds whatever the client had at export time, silently undoing the
controller's "always import as enabled" behavior for any client disabled at
export.

Fix: track which emails were already seeded by the ClientStats loop and skip
the AddClientStat call for those emails, leaving the import path's forced
values as authoritative. Plain (non-import) creates are unaffected since
ClientStats is empty there, so every client still goes through AddClientStat's
refresh as before.

Also updated a stale comment in addClientTraffic that still described
AddClientStat as DoNothing.

Added TestAddInbound_ImportForcedEnableSurvivesDisabledSettingsClient, which
reproduces the exact regression (verified it fails without this fix) and
passes with it.

2026-07-21 15:57:55 +02:00