fix(clients): withdraw delete tombstones for clients any serialized write re-creates

Deleting a client tombstones its email for 90s, so a node snapshot taken
before the delete reached the node cannot resurrect it. Client create and
bulk create withdrew that tombstone, but a client re-created through
InboundService.AddInbound (SyncInbound / syncInboundClients) did not. On a
master, re-adding a node inbound with a just-deleted email therefore let
the next node merge filter the new client out of the snapshot, unlink it
from its inbound and mark it a sync orphan; ReapSyncOrphans then
hard-deleted it 15 minutes later and the node lost it.

syncInboundClients now records the emails it creates on the serialized
transaction, and runSerializedTx withdraws their tombstones only after the
commit, still on the writer, so no merge runs in between and a rolled-back
create leaves the tombstone standing. The per-call withdraws in Create and
bulkCreate are covered by this and removed.

Co-authored-by: FZ1010 <FZ1010@users.noreply.github.com>

internal/web/service/client_bulk.go

@@ -1505,9 +1505,6 @@ func (s *ClientService) bulkCreate(inboundSvc *InboundService, payloads []Client
 			skip(prep[idx].client.Email, reason[idx])
 			continue
 		}
-		// The client is already live after fanout; never leave a stale delete
-		// tombstone merely because applying its optional HWID limit failed.
-		withdrawClientTombstones(prep[idx].client.Email)
 		if err := s.setClientLimitHwidByEmail(prep[idx].client.Email, prep[idx].limitHwid); err != nil {
 			skip(prep[idx].client.Email, err.Error())
 			continue
@@ -1517,8 +1514,6 @@ func (s *ClientService) bulkCreate(inboundSvc *InboundService, payloads []Client
 			inserted = append(inserted, prep[idx].payloadIdx)
 		}
 	}
-	// A re-created email is a live identity again: a delete tombstone left
-	// standing makes the next node merge prune the new client's inbound links.
 	return result, inserted, needRestart, nil
 }
 

internal/web/service/client_crud.go

@@ -256,9 +256,6 @@ func (s *ClientService) Create(inboundSvc *InboundService, payload *ClientCreate
 		// already existed, and a create the panel reported as failed must not.
 		return needRestart, fanoutErr
 	}
-	// A re-created email is a live identity again: a delete tombstone left
-	// standing makes the next node merge prune the new client's inbound links.
-	withdrawClientTombstones(client.Email)
 	return needRestart, s.setClientLimitHwidByEmail(client.Email, payload.LimitHwid)
 }
 

internal/web/service/client_delete_tombstone_test.go

@@ -1,9 +1,12 @@
 package service
 
 import (
+	"errors"
 	"fmt"
 	"testing"
 
+	"gorm.io/gorm"
+
 	"github.com/mhsanaei/3x-ui/v3/internal/database"
 	"github.com/mhsanaei/3x-ui/v3/internal/database/model"
 	"github.com/mhsanaei/3x-ui/v3/internal/web/runtime"
@@ -104,3 +107,93 @@ func TestRecreatedClientSurvivesNodeMerge(t *testing.T) {
 		t.Fatalf("re-created client has %d inbound links after the node merge, want 1: the delete tombstone outlived the email and the merge pruned it", links)
 	}
 }
+
+// Re-adding a node inbound with a just-deleted email creates that client again
+// through AddInbound, so the next merge must not prune it as the deleted one.
+func TestReaddedNodeInboundKeepsRecreatedClient(t *testing.T) {
+	db := initTrafficTestDB(t)
+	inboundSvc := &InboundService{}
+	clientSvc := &ClientService{}
+
+	const email = "tunnel-81@x"
+	seedNodeRow(t, db, &model.Node{Id: 1, Name: "n1", Address: "127.0.0.1", Port: 2096, ApiToken: "tok", Enable: true, InboundSyncMode: "all"})
+	recreated := fmt.Sprintf(`{"id": "55555555-5555-4555-8555-555555555555", "email": %q, "enable": true}`, email)
+	nodeInbound := func(clients string) *model.Inbound {
+		nodeID := 1
+		return &model.Inbound{
+			UserId: 1, NodeID: &nodeID, Enable: true, Listen: "127.0.0.1", Port: 18453, Protocol: model.VLESS,
+			Settings:       `{"clients": [` + clients + `], "decryption": "none"}`,
+			StreamSettings: `{"network": "tcp", "security": "none"}`,
+		}
+	}
+
+	first, _, err := inboundSvc.AddInbound(nodeInbound(recreated))
+	if err != nil {
+		t.Fatalf("add node inbound: %v", err)
+	}
+	if _, err := inboundSvc.DelInbound(first.Id); err != nil {
+		t.Fatalf("delete node inbound: %v", err)
+	}
+	t.Cleanup(func() { withdrawClientTombstones(email) })
+	// inbounds/del only detaches; the record must go too or the re-add is a duplicate.
+	if _, err := clientSvc.DeleteByEmail(inboundSvc, email, false); err != nil {
+		t.Fatalf("delete detached client: %v", err)
+	}
+	// A second client keeps the tombstone-filtered snapshot non-empty, past the #6734 guard.
+	readded, _, err := inboundSvc.AddInbound(nodeInbound(recreated + `, {"id": "66666666-6666-4666-8666-666666666666", "email": "neighbour@x", "enable": true}`))
+	if err != nil {
+		t.Fatalf("re-add node inbound: %v", err)
+	}
+
+	// The next tick reconciles the push, clears dirty and merges a snapshot that has it.
+	if err := db.Model(&model.Node{}).Where("id = ?", 1).Update("config_dirty", false).Error; err != nil {
+		t.Fatalf("clear config_dirty: %v", err)
+	}
+	snap := &runtime.TrafficSnapshot{Inbounds: []*model.Inbound{{
+		Tag: readded.Tag, Enable: true, Listen: readded.Listen, Port: readded.Port, Protocol: model.VLESS,
+		Settings: readded.Settings, StreamSettings: readded.StreamSettings,
+		ClientStats: []xray.ClientTraffic{{Email: email, Enable: true}, {Email: "neighbour@x", Enable: true}},
+	}}}
+	if _, err := inboundSvc.setRemoteTrafficLocked(1, snap, false, true); err != nil {
+		t.Fatalf("node merge: %v", err)
+	}
+
+	var rec model.ClientRecord
+	if err := db.Where("email = ?", email).First(&rec).Error; err != nil {
+		t.Fatalf("load re-created client: %v", err)
+	}
+	var links int64
+	if err := db.Model(&model.ClientInbound{}).
+		Where("client_id = ? AND inbound_id = ?", rec.Id, readded.Id).
+		Count(&links).Error; err != nil {
+		t.Fatalf("count client links: %v", err)
+	}
+	if links != 1 || rec.SyncOrphanedAt != 0 {
+		t.Fatalf("after the merge the re-added client has %d links to its inbound and sync_orphaned_at=%d, want 1 and 0: the delete tombstone outlived the re-add and the merge pruned the client for ReapSyncOrphans", links, rec.SyncOrphanedAt)
+	}
+}
+
+// The lift waits for the commit: a create that rolls back must leave the tombstone
+// standing, or a snapshot taken before the delete resurrects the client.
+func TestRolledBackCreateKeepsTombstone(t *testing.T) {
+	setupBulkDB(t)
+	const email = "rolled-back@x"
+	ib := mkInbound(t, 30451, model.VLESS, `{"clients": []}`)
+	tombstoneClientEmail(email)
+	t.Cleanup(func() { withdrawClientTombstones(email) })
+
+	errAbort := errors.New("abort after the client was created")
+	err := runSerializedTx(func(tx *gorm.DB) error {
+		created := []model.Client{{Email: email, ID: "77777777-7777-4777-8777-777777777777", Enable: true}}
+		if err := (&ClientService{}).SyncInbound(tx, ib.Id, created); err != nil {
+			return err
+		}
+		return errAbort
+	})
+	if !errors.Is(err, errAbort) {
+		t.Fatalf("runSerializedTx error = %v, want the injected abort", err)
+	}
+	if !isClientEmailTombstoned(email) {
+		t.Fatal("a create that rolled back withdrew the delete tombstone, so a stale node snapshot can resurrect the deleted client")
+	}
+}

internal/web/service/client_link.go

@@ -208,6 +208,9 @@ func (s *ClientService) syncInboundClients(tx *gorm.DB, inboundId int, clients [
 		if err := tx.CreateInBatches(toCreate, 200).Error; err != nil {
 			return err
 		}
+		// Whichever API re-created the client, a delete tombstone left standing makes
+		// the next node merge prune it and ReapSyncOrphans delete it.
+		liftTombstonesOnCommit(tx, toCreate)
 		disabledIDs := make([]int, 0)
 		for i, rec := range toCreate {
 			idByEmail[rec.Email] = rec.Id

internal/web/service/client_locks.go

@@ -119,6 +119,18 @@ func withdrawClientTombstones(emails ...string) {
 	}
 }
 
+// liftTombstonesOnCommit withdraws the delete tombstones of clients a serialized
+// tx re-created once it commits; a rolled-back create leaves them standing.
+func liftTombstonesOnCommit(tx *gorm.DB, created []*model.ClientRecord) {
+	state := serializedTxOf(tx)
+	if state == nil {
+		return
+	}
+	for _, rec := range created {
+		state.createdClientEmails = append(state.createdClientEmails, rec.Email)
+	}
+}
+
 func tombstoneClientEmails(emails []string) {
 	if len(emails) == 0 {
 		return

internal/web/service/traffic_writer.go

@@ -127,19 +127,34 @@ func runTrafficWriter(ctx context.Context, queue chan *trafficWriteRequest, done
 // timeout. Apply runtime changes after this returns.
 func runSerializedTx(fn func(tx *gorm.DB) error) error {
 	return submitTrafficWrite(func() error {
-		return database.GetDB().Transaction(func(tx *gorm.DB) error {
-			ctx := context.WithValue(tx.Statement.Context, serializedTxContextKey{}, true)
+		state := &serializedTx{}
+		err := database.GetDB().Transaction(func(tx *gorm.DB) error {
+			ctx := context.WithValue(tx.Statement.Context, serializedTxContextKey{}, state)
 			return fn(tx.WithContext(ctx))
 		})
+		if err == nil {
+			// Still on the writer, so no node merge runs between the commit and the lift.
+			withdrawClientTombstones(state.createdClientEmails...)
+		}
+		return err
 	})
 }
 
-func isSerializedTx(tx *gorm.DB) bool {
+// serializedTx carries what may only take effect once its transaction commits.
+type serializedTx struct {
+	createdClientEmails []string
+}
+
+func serializedTxOf(tx *gorm.DB) *serializedTx {
 	if tx == nil || tx.Statement == nil || tx.Statement.Context == nil {
-		return false
+		return nil
 	}
-	active, _ := tx.Statement.Context.Value(serializedTxContextKey{}).(bool)
-	return active
+	state, _ := tx.Statement.Context.Value(serializedTxContextKey{}).(*serializedTx)
+	return state
+}
+
+func isSerializedTx(tx *gorm.DB) bool {
+	return serializedTxOf(tx) != nil
 }
 
 func safeApply(fn func() error) (err error) {