fix(node): fully delete clients on nodes instead of only detaching them

Deleting a client on the master propagated to nodes via the detach
endpoint, which removes the client from that one inbound's settings but
deliberately keeps the client record. The node ended up with an
orphaned record that kept showing in its Clients view; the master and
node could never converge on a delete.

Full-delete and detach intent now travel separately: the Runtime
interface gains DeleteClient, which on Remote hits the node's
panel/api/clients/del endpoint (record, attachments, traffic; repeat
calls for a client on several inbounds of the same node are swallowed
as idempotent "not found"). Delete/DeleteByEmail/BulkDelete use it for
node inbounds, while Detach/BulkDetach keep the inbound-scoped detach
RPC so removing a client from one inbound never wipes it node-wide
(the #5543 guarantee is preserved and covered by tests). Bulk deletes
above the fold threshold still converge membership via reconcile; their
leftover node records can be cleaned with the node's delete-orphans
action.

Closes #5797

internal/web/runtime/local.go

@@ -130,6 +130,10 @@ func (l *Local) DeleteUser(ctx context.Context, ib *model.Inbound, email string)
 	return nil
 }
 
+func (l *Local) DeleteClient(context.Context, string) error {
+	return nil
+}
+
 func (l *Local) UpdateUser(ctx context.Context, ib *model.Inbound, oldEmail string, payload model.Client) error {
 	if oldEmail != "" {
 		if err := l.RemoveUser(ctx, ib, oldEmail); err != nil && !strings.Contains(err.Error(), "not found") {

internal/web/runtime/remote.go

@@ -553,6 +553,22 @@ func (r *Remote) DeleteUser(ctx context.Context, ib *model.Inbound, email string
 	return err
 }
 
+func (r *Remote) DeleteClient(ctx context.Context, email string) error {
+	if email == "" {
+		return nil
+	}
+	_, err := r.do(ctx, http.MethodPost,
+		"panel/api/clients/del/"+url.PathEscape(email), nil)
+	if err == nil {
+		return nil
+	}
+	var apiErr *remoteAPIError
+	if errors.As(err, &apiErr) && strings.Contains(strings.ToLower(apiErr.msg), "not found") {
+		return nil
+	}
+	return err
+}
+
 func (r *Remote) UpdateUser(ctx context.Context, ib *model.Inbound, oldEmail string, payload model.Client) error {
 	if oldEmail == "" {
 		oldEmail = payload.Email

internal/web/runtime/runtime.go

@@ -23,6 +23,13 @@ type Runtime interface {
 	DeleteUser(ctx context.Context, ib *model.Inbound, email string) error
 	AddClient(ctx context.Context, ib *model.Inbound, client model.Client) error
 
+	// DeleteClient removes the client identified by email entirely from the
+	// runtime's own store: on Remote it hits the node's full-delete endpoint
+	// (record, attachments, traffic), unlike DeleteUser which only detaches
+	// from one inbound and leaves the node's client record behind. Local has
+	// no client store of its own, so it is a no-op there.
+	DeleteClient(ctx context.Context, email string) error
+
 	RestartXray(ctx context.Context) error
 
 	ResetClientTraffic(ctx context.Context, ib *model.Inbound, email string) error

internal/web/service/client_bulk.go

@@ -1067,9 +1067,12 @@ func (s *ClientService) bulkDelInboundClients(
 				push = false
 			}
 			if push {
+				// bulkDelInboundClients only runs for full client deletion
+				// (BulkDelete), so the node must drop its client record too,
+				// not just detach from this inbound (#5797).
 				pushFailed := false
 				for email := range foundEmails {
-					if err1 := rt.DeleteUser(context.Background(), oldInbound, email); err1 != nil {
+					if err1 := rt.DeleteClient(context.Background(), email); err1 != nil {
 						logger.Warning("Error in deleting client on", rt.Name(), ":", err1)
 						pushFailed = true
 					}

internal/web/service/client_crud.go

@@ -451,7 +451,7 @@ func (s *ClientService) Delete(inboundSvc *InboundService, id int, keepTraffic b
 		if existing.Email == "" {
 			continue
 		}
-		nr, delErr := s.DelInboundClientByEmail(inboundSvc, ibId, existing.Email, false)
+		nr, delErr := s.DelInboundClientByEmail(inboundSvc, ibId, existing.Email, false, true)
 		if delErr != nil {
 			// The client is already absent from this inbound (data drift or a
 			// retried delete). Skip it — deletion stays idempotent.
@@ -609,7 +609,7 @@ func (s *ClientService) DeleteByEmail(inboundSvc *InboundService, email string,
 	}
 	needRestart := false
 	for _, ibId := range inboundIds {
-		nr, delErr := s.DelInboundClientByEmail(inboundSvc, ibId, email, false)
+		nr, delErr := s.DelInboundClientByEmail(inboundSvc, ibId, email, false, true)
 		if delErr != nil {
 			if errors.Is(delErr, ErrClientNotInInbound) {
 				continue
@@ -672,7 +672,7 @@ func (s *ClientService) Detach(inboundSvc *InboundService, id int, inboundIds []
 		if existing.Email == "" {
 			continue
 		}
-		nr, delErr := s.DelInboundClientByEmail(inboundSvc, ibId, existing.Email, true)
+		nr, delErr := s.DelInboundClientByEmail(inboundSvc, ibId, existing.Email, true, false)
 		if delErr != nil {
 			if errors.Is(delErr, ErrClientNotInInbound) {
 				continue

internal/web/service/client_delete_node_full_test.go

@@ -0,0 +1,66 @@
+package service
+
+import (
+	"testing"
+
+	"github.com/google/uuid"
+
+	"github.com/mhsanaei/3x-ui/v3/internal/database/model"
+)
+
+// A full client delete must reach the node as the full-delete RPC so the node
+// drops its own client record too — the detach RPC leaves an orphaned record
+// that keeps showing in the node's client list (#5797).
+func TestDelete_NodeClientDispatchesFullDeleteRPC(t *testing.T) {
+	setupBulkDB(t)
+	nodeID, fake := setupNodeRuntime(t)
+
+	clients := []model.Client{{ID: uuid.NewString(), Email: "full-del@x", Enable: true}}
+	nodeInbound(t, nodeID, 32001, clients)
+
+	svc := &ClientService{}
+	inboundSvc := &InboundService{}
+
+	rec, err := svc.GetRecordByEmail(nil, "full-del@x")
+	if err != nil {
+		t.Fatalf("GetRecordByEmail: %v", err)
+	}
+	if _, err := svc.Delete(inboundSvc, rec.Id, false); err != nil {
+		t.Fatalf("Delete: %v", err)
+	}
+
+	if got := fake.deleteClient.Load(); got != 1 {
+		t.Fatalf("full delete dispatched %d DeleteClient RPCs, want 1", got)
+	}
+	if got := fake.deleteUser.Load(); got != 0 {
+		t.Fatalf("full delete dispatched %d DeleteUser (detach) RPCs, want 0", got)
+	}
+}
+
+// A plain detach must stay scoped to the one inbound via the detach RPC and
+// never escalate to the node-wide full delete.
+func TestDetach_NodeClientStaysOnDetachRPC(t *testing.T) {
+	setupBulkDB(t)
+	nodeID, fake := setupNodeRuntime(t)
+
+	clients := []model.Client{{ID: uuid.NewString(), Email: "detach-me@x", Enable: true}}
+	ib := nodeInbound(t, nodeID, 32002, clients)
+
+	svc := &ClientService{}
+	inboundSvc := &InboundService{}
+
+	rec, err := svc.GetRecordByEmail(nil, "detach-me@x")
+	if err != nil {
+		t.Fatalf("GetRecordByEmail: %v", err)
+	}
+	if _, err := svc.Detach(inboundSvc, rec.Id, []int{ib.Id}); err != nil {
+		t.Fatalf("Detach: %v", err)
+	}
+
+	if got := fake.deleteUser.Load(); got != 1 {
+		t.Fatalf("detach dispatched %d DeleteUser RPCs, want 1", got)
+	}
+	if got := fake.deleteClient.Load(); got != 0 {
+		t.Fatalf("detach dispatched %d DeleteClient RPCs, want 0", got)
+	}
+}

internal/web/service/client_inbound_apply.go

@@ -831,7 +831,7 @@ func (s *ClientService) UpdateInboundClient(inboundSvc *InboundService, data *mo
 	return needRestart, nil
 }
 
-func (s *ClientService) DelInboundClientByEmail(inboundSvc *InboundService, inboundId int, email string, keepTraffic bool) (bool, error) {
+func (s *ClientService) DelInboundClientByEmail(inboundSvc *InboundService, inboundId int, email string, keepTraffic bool, fullDelete bool) (bool, error) {
 	defer lockInbound(inboundId).Unlock()
 
 	oldInbound, err := inboundSvc.GetInbound(inboundId)
@@ -972,9 +972,17 @@ func (s *ClientService) DelInboundClientByEmail(inboundSvc *InboundService, inbo
 		} else {
 			// Node inbound: propagate the delete regardless of the enable flag —
 			// the node's own DB still carries a disabled client and would
-			// resurrect it on the next snapshot otherwise.
+			// resurrect it on the next snapshot otherwise. A full client delete
+			// must remove the node's client record too, not just detach it from
+			// this inbound (#5797).
 			if push {
-				if err1 := rt.DeleteUser(context.Background(), oldInbound, email); err1 != nil {
+				var err1 error
+				if fullDelete {
+					err1 = rt.DeleteClient(context.Background(), email)
+				} else {
+					err1 = rt.DeleteUser(context.Background(), oldInbound, email)
+				}
+				if err1 != nil {
 					logger.Warning("Error in deleting client on", rt.Name(), ":", err1)
 				} else {
 					advancePushedInbound(rt, prevSettings, oldInbound)

internal/web/service/client_wireguard_crud_test.go

@@ -80,7 +80,7 @@ func TestWireGuardClientAddUpdateDeleteRoundTrip(t *testing.T) {
 		t.Fatalf("settings lost wg fields after metadata edit: %+v", listAfter)
 	}
 
-	if _, err := svc.DelInboundClientByEmail(inboundSvc, ib.Id, "alice@wg", false); err != nil {
+	if _, err := svc.DelInboundClientByEmail(inboundSvc, ib.Id, "alice@wg", false, false); err != nil {
 		t.Fatalf("DelInboundClientByEmail: %v", err)
 	}
 	final, err := svc.ListForInbound(nil, ib.Id)

internal/web/service/del_shared_email_runtime_test.go

@@ -25,7 +25,7 @@ func TestDelInboundClientByEmail_SharedEmailStillRemovesFromRuntime(t *testing.T
 	svc := &ClientService{}
 	inboundSvc := &InboundService{}
 
-	if _, err := svc.DelInboundClientByEmail(inboundSvc, ibA.Id, "shared@x", false); err != nil {
+	if _, err := svc.DelInboundClientByEmail(inboundSvc, ibA.Id, "shared@x", false, false); err != nil {
 		t.Fatalf("DelInboundClientByEmail: %v", err)
 	}
 

internal/web/service/node_bulk_dispatch_test.go

@@ -16,9 +16,10 @@ import (
 // fakeNodeRuntime is a runtime.Runtime stub that counts the per-client dispatch
 // calls so a test can assert a bulk op does NOT stream one RPC per client.
 type fakeNodeRuntime struct {
-	addClient  atomic.Int32
-	deleteUser atomic.Int32
-	updateUser atomic.Int32
+	addClient    atomic.Int32
+	deleteUser   atomic.Int32
+	deleteClient atomic.Int32
+	updateUser   atomic.Int32
 }
 
 func (f *fakeNodeRuntime) Name() string { return "fake-node" }
@@ -40,6 +41,11 @@ func (f *fakeNodeRuntime) DeleteUser(context.Context, *model.Inbound, string) er
 	return nil
 }
 
+func (f *fakeNodeRuntime) DeleteClient(context.Context, string) error {
+	f.deleteClient.Add(1)
+	return nil
+}
+
 func (f *fakeNodeRuntime) AddClient(context.Context, *model.Inbound, model.Client) error {
 	f.addClient.Add(1)
 	return nil

internal/web/service/node_dirty_test.go

@@ -99,7 +99,7 @@ func TestDelInboundClientByEmail_DisabledNodeClientMarksDirty(t *testing.T) {
 
 	inboundSvc := &InboundService{}
 	clientSvc := &ClientService{}
-	if _, err := clientSvc.DelInboundClientByEmail(inboundSvc, central.Id, "a@x", false); err != nil {
+	if _, err := clientSvc.DelInboundClientByEmail(inboundSvc, central.Id, "a@x", false, false); err != nil {
 		t.Fatalf("DelInboundClientByEmail: %v", err)
 	}
 

internal/web/service/sync_scale_postgres_test.go

@@ -201,7 +201,7 @@ func TestAddDelClientPostgresScale(t *testing.T) {
 
 			delEmail := clients[n/2].Email
 			start = time.Now()
-			if _, err := svc.DelInboundClientByEmail(inboundSvc, ib.Id, delEmail, false); err != nil {
+			if _, err := svc.DelInboundClientByEmail(inboundSvc, ib.Id, delEmail, false, false); err != nil {
 				t.Fatalf("DelInboundClientByEmail: %v", err)
 			}
 			delDur := time.Since(start)