fix(clients): flag the restart a partly-applied edit or delete still needs

63b46cd6 made a multi-inbound client op apply its inbounds concurrently and
stop aborting at the first failure, so an error can now come back together
with needRestart=true: the inbounds that succeeded committed real changes and
their Xray still needs the restart. That commit taught the two callers it
converted — create and attach — to read the flag before the error check.

d34ec97f then routed Update, Delete, Detach and DeleteByEmail's record-less
fallback through the same fanout but touched no caller, so on a master with
several nodes a partly-applied edit or delete returned (true, err) into a
handler that returned on err first. Xray was never flagged for the work that
landed and notifyClientsChanged never fired, so the running config kept
serving the pre-edit client set and every open panel showed stale rows until
something else happened to trigger a restart.

Read needRestart before the error check in update, delete and detach, and
broadcast on needRestart || err == nil — the same shape create and attach have
had since 63b46cd6. The predicate is a strict superset of the old err == nil,
and needRestart is only ever assigned after a runSerializedTx commit, so it
firing genuinely means something landed.

The three handlers are pinned by a new controller test each: one client on two
inbounds, the second one's settings JSON corrupted so the op commits on one and
fails on the other, asserting both the success:false response and the restart
flag. All three fail without the change.

The API docs for update, del and detach now describe the partial-application
contract, as add and attach already did. Detach ends at the fanout so every one
of its errors carries the inbound prefix; update and delete write the client
record afterwards, and a failure there is reported without one.

docs/content/docs/en/reference/api/clients.mdx

@@ -570,6 +570,25 @@ _openapi:
           one per line. `limitHwid` is applied only when every inbound
           succeeded, so re-run the call after fixing the failure.
         heading: create-a-new-client-and-attach-it-to-one-or-more-inbounds-in-a-single-call-body-is-json-per-protocol-secrets-are-generated-server-side-when-omitted-so-callers-can-send-only-the-universal-fields
+      - content: 'The inbounds are applied concurrently and independently: one that
+          fails no longer stops the others. Every inbound error names the
+          inbound it came from (`inbound 7: <message>`), and several failures
+          are reported together, one per line. So a `success:false` response can
+          still have applied the edit to the remaining inbounds. The client
+          record is written after the inbounds, so a failure there is reported
+          without an `inbound <id>:` prefix and leaves the inbound edits in
+          place.'
+        heading: update-an-existing-client-by-email-changes-propagate-to-every-attached-inbound-body-is-the-json-client-payload--supply-the-full-set-of-fields-you-want-to-keep-the-server-replaces-the-row-it-does-not-patch
+      - content: 'The inbounds are applied concurrently and independently: one that
+          fails no longer stops the others. Every inbound error names the
+          inbound it came from (`inbound 7: <message>`), and several failures
+          are reported together, one per line. So a `success:false` response can
+          still have removed the client from the remaining inbounds; the client
+          record is kept in that case, so re-running the call retries exactly
+          the leftovers. The record and traffic rows are dropped after the
+          inbounds, so a failure there is reported without an `inbound <id>:`
+          prefix and leaves the client already removed from every inbound.'
+        heading: delete-a-client-by-email-removes-it-from-every-attached-inbound-and-drops-its-traffic-record-unless-keeptraffic1-is-passed
       - content: 'A WireGuard client brings its stored `allowedIPs` into the new inbound
           instead of being given a fresh address, so the call fails with
           `inbound <id>: wireguard: allowedIPs entry already used by another
@@ -579,6 +598,13 @@ _openapi:
           independently, so the remaining ones are still attached and a
           `success:false` response can be partial.'
         heading: attach-an-existing-client-to-one-or-more-additional-inbounds-body-is-json
+      - content: 'The inbounds are applied concurrently and independently: one that
+          fails no longer stops the others. Every inbound error names the
+          inbound it came from (`inbound 7: <message>`), and several failures
+          are reported together, one per line. So a `success:false` response can
+          still have detached the remaining inbounds. Detach writes nothing
+          beyond the inbounds, so every error carries the prefix.'
+        heading: detach-a-client-from-one-or-more-inbounds-without-deleting-the-client
 ---
 
 {/* This file was generated by Fumadocs. Do not edit this file directly. Any changes should be made by running the generation command again. */}

docs/public/openapi.json

@@ -7591,6 +7591,7 @@
         ],
         "summary": "Update an existing client by email. Changes propagate to every attached inbound. Body is the JSON client payload — supply the full set of fields you want to keep (the server replaces the row, it does not patch).",
         "operationId": "post_panel_api_clients_update_email",
+        "description": "The inbounds are applied concurrently and independently: one that fails no longer stops the others. Every inbound error names the inbound it came from (`inbound 7: <message>`), and several failures are reported together, one per line. So a `success:false` response can still have applied the edit to the remaining inbounds. The client record is written after the inbounds, so a failure there is reported without an `inbound <id>:` prefix and leaves the inbound edits in place.",
         "parameters": [
           {
             "name": "email",
@@ -7654,6 +7655,7 @@
         ],
         "summary": "Delete a client by email. Removes it from every attached inbound and drops its traffic record unless keepTraffic=1 is passed.",
         "operationId": "post_panel_api_clients_del_email",
+        "description": "The inbounds are applied concurrently and independently: one that fails no longer stops the others. Every inbound error names the inbound it came from (`inbound 7: <message>`), and several failures are reported together, one per line. So a `success:false` response can still have removed the client from the remaining inbounds; the client record is kept in that case, so re-running the call retries exactly the leftovers. The record and traffic rows are dropped after the inbounds, so a failure there is reported without an `inbound <id>:` prefix and leaves the client already removed from every inbound.",
         "parameters": [
           {
             "name": "email",
@@ -7781,6 +7783,7 @@
         ],
         "summary": "Detach a client from one or more inbounds without deleting the client.",
         "operationId": "post_panel_api_clients_email_detach",
+        "description": "The inbounds are applied concurrently and independently: one that fails no longer stops the others. Every inbound error names the inbound it came from (`inbound 7: <message>`), and several failures are reported together, one per line. So a `success:false` response can still have detached the remaining inbounds. Detach writes nothing beyond the inbounds, so every error carries the prefix.",
         "parameters": [
           {
             "name": "email",

frontend/public/openapi.json

@@ -7591,6 +7591,7 @@
         ],
         "summary": "Update an existing client by email. Changes propagate to every attached inbound. Body is the JSON client payload — supply the full set of fields you want to keep (the server replaces the row, it does not patch).",
         "operationId": "post_panel_api_clients_update_email",
+        "description": "The inbounds are applied concurrently and independently: one that fails no longer stops the others. Every inbound error names the inbound it came from (`inbound 7: <message>`), and several failures are reported together, one per line. So a `success:false` response can still have applied the edit to the remaining inbounds. The client record is written after the inbounds, so a failure there is reported without an `inbound <id>:` prefix and leaves the inbound edits in place.",
         "parameters": [
           {
             "name": "email",
@@ -7654,6 +7655,7 @@
         ],
         "summary": "Delete a client by email. Removes it from every attached inbound and drops its traffic record unless keepTraffic=1 is passed.",
         "operationId": "post_panel_api_clients_del_email",
+        "description": "The inbounds are applied concurrently and independently: one that fails no longer stops the others. Every inbound error names the inbound it came from (`inbound 7: <message>`), and several failures are reported together, one per line. So a `success:false` response can still have removed the client from the remaining inbounds; the client record is kept in that case, so re-running the call retries exactly the leftovers. The record and traffic rows are dropped after the inbounds, so a failure there is reported without an `inbound <id>:` prefix and leaves the client already removed from every inbound.",
         "parameters": [
           {
             "name": "email",
@@ -7781,6 +7783,7 @@
         ],
         "summary": "Detach a client from one or more inbounds without deleting the client.",
         "operationId": "post_panel_api_clients_email_detach",
+        "description": "The inbounds are applied concurrently and independently: one that fails no longer stops the others. Every inbound error names the inbound it came from (`inbound 7: <message>`), and several failures are reported together, one per line. So a `success:false` response can still have detached the remaining inbounds. Detach writes nothing beyond the inbounds, so every error carries the prefix.",
         "parameters": [
           {
             "name": "email",

frontend/src/pages/api-docs/endpoints.ts

@@ -1151,6 +1151,8 @@ export const sections: readonly Section[] = [
         path: '/panel/api/clients/update/:email',
         summary:
           'Update an existing client by email. Changes propagate to every attached inbound. Body is the JSON client payload — supply the full set of fields you want to keep (the server replaces the row, it does not patch).',
+        description:
+          'The inbounds are applied concurrently and independently: one that fails no longer stops the others. Every inbound error names the inbound it came from (`inbound 7: <message>`), and several failures are reported together, one per line. So a `success:false` response can still have applied the edit to the remaining inbounds. The client record is written after the inbounds, so a failure there is reported without an `inbound <id>:` prefix and leaves the inbound edits in place.',
         params: [
           {
             name: 'email',
@@ -1167,6 +1169,8 @@ export const sections: readonly Section[] = [
         path: '/panel/api/clients/del/:email',
         summary:
           'Delete a client by email. Removes it from every attached inbound and drops its traffic record unless keepTraffic=1 is passed.',
+        description:
+          'The inbounds are applied concurrently and independently: one that fails no longer stops the others. Every inbound error names the inbound it came from (`inbound 7: <message>`), and several failures are reported together, one per line. So a `success:false` response can still have removed the client from the remaining inbounds; the client record is kept in that case, so re-running the call retries exactly the leftovers. The record and traffic rows are dropped after the inbounds, so a failure there is reported without an `inbound <id>:` prefix and leaves the client already removed from every inbound.',
         params: [
           { name: 'email', in: 'path', type: 'string', desc: 'Client email (unique identifier).' },
           {
@@ -1200,6 +1204,8 @@ export const sections: readonly Section[] = [
         method: 'POST',
         path: '/panel/api/clients/:email/detach',
         summary: 'Detach a client from one or more inbounds without deleting the client.',
+        description:
+          'The inbounds are applied concurrently and independently: one that fails no longer stops the others. Every inbound error names the inbound it came from (`inbound 7: <message>`), and several failures are reported together, one per line. So a `success:false` response can still have detached the remaining inbounds. Detach writes nothing beyond the inbounds, so every error carries the prefix.',
         params: [
           { name: 'email', in: 'path', type: 'string', desc: 'Client email (unique identifier).' },
           {

internal/web/controller/client.go

@@ -215,30 +215,42 @@ func (a *ClientController) update(c *gin.Context) {
 	}
 	inboundFilter := parseInboundIdsQuery(c.Query("inboundIds"))
 	needRestart, err := a.clientService.UpdateByEmail(&a.inboundService, email, req.Client, req.LimitHwid, inboundFilter...)
+	// Flagged before the error check: a partly-applied edit leaves the change
+	// committed on the inbounds that succeeded, and those still need the restart.
+	if needRestart {
+		a.xrayService.SetToNeedRestart()
+	}
+	// A partly-applied call committed real changes; a rejected one touched
+	// nothing, and broadcasting those would refetch every panel for nothing.
+	if needRestart || err == nil {
+		notifyClientsChanged()
+	}
 	if err != nil {
 		jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err)
 		return
 	}
 	jsonMsgObj(c, I18nWeb(c, "pages.inbounds.toasts.inboundClientUpdateSuccess"), pendingNodeObj(a.clientService.HasPendingNode(&a.inboundService, email)), nil)
-	if needRestart {
-		a.xrayService.SetToNeedRestart()
-	}
-	notifyClientsChanged()
 }
 
 func (a *ClientController) delete(c *gin.Context) {
 	email := c.Param("email")
 	keepTraffic := c.Query("keepTraffic") == "1"
 	needRestart, err := a.clientService.DeleteByEmail(&a.inboundService, email, keepTraffic)
+	// Flagged before the error check: a partly-applied delete already removed
+	// the client from the inbounds that succeeded, and those need the restart.
+	if needRestart {
+		a.xrayService.SetToNeedRestart()
+	}
+	// A partly-applied call committed real removals; a rejected one touched
+	// nothing, and broadcasting those would refetch every panel for nothing.
+	if needRestart || err == nil {
+		notifyClientsChanged()
+	}
 	if err != nil {
 		jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err)
 		return
 	}
 	jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.inboundClientDeleteSuccess"), nil)
-	if needRestart {
-		a.xrayService.SetToNeedRestart()
-	}
-	notifyClientsChanged()
 }
 
 type attachDetachBody struct {
@@ -640,15 +652,21 @@ func (a *ClientController) detach(c *gin.Context) {
 		return
 	}
 	needRestart, err := a.clientService.DetachByEmailMany(&a.inboundService, email, body.InboundIds)
+	// Flagged before the error check: a partly-applied detach already removed
+	// the client from the inbounds that succeeded, and those need the restart.
+	if needRestart {
+		a.xrayService.SetToNeedRestart()
+	}
+	// A partly-applied call committed real removals; a rejected one touched
+	// nothing, and broadcasting those would refetch every panel for nothing.
+	if needRestart || err == nil {
+		notifyClientsChanged()
+	}
 	if err != nil {
 		jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err)
 		return
 	}
 	jsonMsgObj(c, I18nWeb(c, "pages.inbounds.toasts.inboundClientDeleteSuccess"), pendingNodeObj(a.inboundService.AnyNodePending(body.InboundIds)), nil)
-	if needRestart {
-		a.xrayService.SetToNeedRestart()
-	}
-	notifyClientsChanged()
 }
 
 type bulkResetRequest struct {

internal/web/controller/client_partial_apply_test.go

@@ -0,0 +1,141 @@
+package controller
+
+import (
+	"bytes"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"path/filepath"
+	"testing"
+
+	"github.com/gin-gonic/gin"
+
+	"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/entity"
+	"github.com/mhsanaei/3x-ui/v3/internal/web/service"
+)
+
+// seedPartlyApplyingClient puts one client on two inbounds and corrupts the second
+// one's settings, so a later op succeeds on one inbound and fails on the other.
+func seedPartlyApplyingClient(t *testing.T, email string, basePort int) (healthyID, brokenID int) {
+	t.Helper()
+	dbDir := t.TempDir()
+	t.Setenv("XUI_DB_FOLDER", dbDir)
+	if err := database.InitDB(filepath.Join(dbDir, "x-ui.db")); err != nil {
+		t.Fatalf("InitDB: %v", err)
+	}
+	t.Cleanup(func() { _ = database.CloseDB() })
+
+	db := database.GetDB()
+	ids := make([]int, 0, 2)
+	for i := range 2 {
+		ib := &model.Inbound{
+			UserId: 1, Enable: true, Port: basePort + i,
+			Tag:      "in-" + string(rune('a'+i)) + "-partial",
+			Protocol: model.VLESS, Settings: `{"clients": []}`,
+			StreamSettings: `{"network":"tcp","security":"none"}`,
+		}
+		if err := db.Create(ib).Error; err != nil {
+			t.Fatalf("create inbound %d: %v", i, err)
+		}
+		ids = append(ids, ib.Id)
+	}
+
+	if _, err := (&service.ClientService{}).Create(&service.InboundService{}, &service.ClientCreatePayload{
+		Client:     model.Client{Email: email, ID: "11111111-2222-3333-4444-555555555555", SubID: "sub-" + email, Enable: true},
+		InboundIds: ids,
+	}); err != nil {
+		t.Fatalf("seed Create across both inbounds: %v", err)
+	}
+
+	if err := db.Model(&model.Inbound{}).Where("id = ?", ids[1]).
+		Update("settings", `{"clients":`).Error; err != nil {
+		t.Fatalf("corrupt inbound %d settings: %v", ids[1], err)
+	}
+	return ids[0], ids[1]
+}
+
+func postCtx(t *testing.T, email string, body any) (*gin.Context, *httptest.ResponseRecorder) {
+	t.Helper()
+	gin.SetMode(gin.TestMode)
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Params = gin.Params{{Key: "email", Value: email}}
+	payload := []byte("{}")
+	if body != nil {
+		var err error
+		if payload, err = json.Marshal(body); err != nil {
+			t.Fatalf("marshal body: %v", err)
+		}
+	}
+	c.Request = httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(payload))
+	c.Request.Header.Set("Content-Type", "application/json")
+	return c, w
+}
+
+// assertPartialApply pins that the op really failed on one inbound, so a green
+// test cannot be a plain full success that never exercised the error path.
+func assertPartialApply(t *testing.T, w *httptest.ResponseRecorder) {
+	t.Helper()
+	var msg entity.Msg
+	if err := json.Unmarshal(w.Body.Bytes(), &msg); err != nil {
+		t.Fatalf("decode response %q: %v", w.Body.String(), err)
+	}
+	if msg.Success {
+		t.Fatalf("response reports success=true, want the partial apply to report failure: %q", w.Body.String())
+	}
+}
+
+// TestUpdateHandlerFlagsRestartOnPartialApply pins that an edit committed on some
+// inbounds and failed on others still flags Xray, as create/attach already did.
+func TestUpdateHandlerFlagsRestartOnPartialApply(t *testing.T) {
+	const email = "[email protected]"
+	seedPartlyApplyingClient(t, email, 43310)
+
+	a := &ClientController{}
+	a.xrayService.IsNeedRestartAndSetFalse()
+	c, w := postCtx(t, email, map[string]any{
+		"email": email, "id": "11111111-2222-3333-4444-555555555555",
+		"subId": "sub-" + email, "enable": true, "comment": "edited",
+	})
+	a.update(c)
+
+	assertPartialApply(t, w)
+	if !a.xrayService.IsNeedRestartAndSetFalse() {
+		t.Fatal("a partly-applied client edit left Xray unflagged for restart")
+	}
+}
+
+// TestDeleteHandlerFlagsRestartOnPartialApply is the delete-side twin: the
+// removals that landed still need the restart the error path used to discard.
+func TestDeleteHandlerFlagsRestartOnPartialApply(t *testing.T) {
+	const email = "[email protected]"
+	seedPartlyApplyingClient(t, email, 43320)
+
+	a := &ClientController{}
+	a.xrayService.IsNeedRestartAndSetFalse()
+	c, w := postCtx(t, email, nil)
+	a.delete(c)
+
+	assertPartialApply(t, w)
+	if !a.xrayService.IsNeedRestartAndSetFalse() {
+		t.Fatal("a partly-applied client delete left Xray unflagged for restart")
+	}
+}
+
+// TestDetachHandlerFlagsRestartOnPartialApply covers the third converted path.
+func TestDetachHandlerFlagsRestartOnPartialApply(t *testing.T) {
+	const email = "[email protected]"
+	healthyID, brokenID := seedPartlyApplyingClient(t, email, 43330)
+
+	a := &ClientController{}
+	a.xrayService.IsNeedRestartAndSetFalse()
+	c, w := postCtx(t, email, attachDetachBody{InboundIds: []int{healthyID, brokenID}})
+	a.detach(c)
+
+	assertPartialApply(t, w)
+	if !a.xrayService.IsNeedRestartAndSetFalse() {
+		t.Fatal("a partly-applied client detach left Xray unflagged for restart")
+	}
+}