fix(providers): persist account reorder correctly and resync on failure

src/app/(dashboard)/dashboard/providers/[id]/page.js

@@ -18,6 +18,7 @@ import ModelRow from "./ModelRow";
 import PassthroughModelsSection from "./PassthroughModelsSection";
 import CompatibleModelsSection from "./CompatibleModelsSection";
 import ConnectionRow from "./ConnectionRow";
+import { buildPriorityUpdates } from "../utils";
 import AddApiKeyModal from "./AddApiKeyModal";
 import EditCompatibleNodeModal from "./EditCompatibleNodeModal";
 import AddCustomModelModal from "./AddCustomModelModal";
@@ -892,31 +893,93 @@ export default function ProviderDetailPage() {
     }
   };
 
-  const handleSwapPriority = async (index1, index2) => {
-    // Optimistic update state
-    const newConnections = [...connections];
-    [newConnections[index1], newConnections[index2]] = [newConnections[index2], newConnections[index1]];
-    setConnections(newConnections);
+  const moveQueueRef = useRef(Promise.resolve());
+  // Blocks new moves while a failure-path refetch restores a trustworthy base.
+  const connectionsResyncingRef = useRef(false);
+
+  // Mirror of connections for the move queue: queued moves run between
+  // renders, so each move must read the latest optimistic order rather than
+  // the snapshot captured when its click was queued.
+  const connectionsRef = useRef(connections);
+  useEffect(() => {
+    connectionsRef.current = connections;
+  }, [connections]);
+
+  // Persist a single move: optimistic re-order, then sequential PUTs that
+  // write every row's final 1-based position.
+  //
+  // Sequential ascending writes are load-bearing: the server renumbers the
+  // provider's rows 1..N after every PUT (connectionsRepo.reorderInTx) and
+  // breaks priority ties by most-recently-updated. Ascending order keeps each
+  // write's target position valid in every intermediate state, so the final
+  // order always matches the request for single moves with distinct stored
+  // priorities. Writing every row (not just moved ones) additionally covers
+  // stored duplicate priorities, where the server tiebreak would otherwise
+  // drift skipped rows. Parallel writes have neither guarantee.
+  const persistConnectionMove = async (fromIndex, toIndex) => {
+    const current = connectionsRef.current;
+    if (fromIndex < 0 || toIndex < 0 || toIndex >= current.length) return;
+    const movedRow = current[fromIndex];
+    const targetRow = current[toIndex];
+
+    // Optimistic update — the UI must reflect the requested final order.
+    const next = [...current];
+    next.splice(fromIndex, 1);
+    next.splice(toIndex, 0, movedRow);
+    connectionsRef.current = next;
+    setConnections(next);
+
+    // Guard against a delete/refresh landing between queueing and running:
+    // both endpoint rows must still be the ones the click was made on.
+    if (next[toIndex] !== movedRow || current[toIndex] !== targetRow) {
+      await fetchConnections();
+      return;
+    }
+
+    const updates = buildPriorityUpdates(next);
 
     try {
-      await Promise.all([
-        fetch(`/api/providers/${newConnections[index1].id}`, {
+      for (const update of updates) {
+        const res = await fetch(`/api/providers/${update.id}`, {
           method: "PUT",
           headers: { "Content-Type": "application/json" },
-          body: JSON.stringify({ priority: index1 }),
-        }),
-        fetch(`/api/providers/${newConnections[index2].id}`, {
-          method: "PUT",
-          headers: { "Content-Type": "application/json" },
-          body: JSON.stringify({ priority: index2 }),
-        }),
-      ]);
+          body: JSON.stringify({ priority: update.priority }),
+        });
+        if (!res.ok) throw new Error(`Update failed with status ${res.status}`);
+      }
+      // Persist the new priorities into local state as well — the next move
+      // diffs against them.
+      const byId = new Map(updates.map((update) => [update.id, update.priority]));
+      const finalized = next.map(
+        (row) => (byId.has(row.id) ? { ...row, priority: byId.get(row.id) } : row)
+      );
+      connectionsRef.current = finalized;
+      setConnections(finalized);
     } catch (error) {
       console.log("Error swapping priority:", error);
-      await fetchConnections();
+      // Roll back: block queued moves until the server refetch lands, so a
+      // queued click never reorders a phantom base, then re-sync.
+      connectionsResyncingRef.current = true;
+      try {
+        await fetchConnections();
+      } finally {
+        connectionsResyncingRef.current = false;
+      }
     }
   };
 
+  const moveConnection = (fromIndex, toIndex) => {
+    if (fromIndex === toIndex) return;
+    if (connectionsResyncingRef.current) return;
+    // Serialize moves — rapid clicks must not interleave PUT sequences,
+    // each queued move builds on the optimistic state of the previous one.
+    moveQueueRef.current = moveQueueRef.current
+      .then(() => persistConnectionMove(fromIndex, toIndex))
+      .catch((error) => console.log("Error swapping priority:", error));
+  };
+
+  const handleSwapPriority = moveConnection;
+
   const selectedConnections = connections.filter((conn) => selectedConnectionIds.includes(conn.id));
   const allSelected = connections.length > 0 && selectedConnectionIds.length === connections.length;
 

src/app/(dashboard)/dashboard/providers/utils.js

@@ -17,3 +17,27 @@ export function matchesStatusFilter(statusFilter, stats, isNoAuth = false) {
   if (statusFilter === "all") return true;
   return getConnectionStatus(stats, isNoAuth) === statusFilter;
 }
+
+// Build the { id, priority } writes that persist `next` (the re-ordered
+// list): every row gets its final 1-based position — the same meaning the
+// server gives it (connectionsRepo orders by priority ASC and renumbers
+// 1..N after every change).
+//
+// Every row must be written, not only the moved ones: if the stored
+// priorities contain duplicates (bulk imports can create ties), the server
+// re-breaks those ties by most-recently-updated on each PUT, so skipping a
+// tied row lets it drift to a position the user did not ask for. Writing
+// all positions keeps every write collision-free and converges regardless
+// of what is already stored.
+//
+// The writes MUST be sent sequentially in ascending position order: the
+// server renumbers 1..N after each PUT, and ascending order keeps every
+// later write's target position valid in the intermediate state.
+export function buildPriorityUpdates(next) {
+  if (next.every((row, i) => row && row.id && row.priority === i + 1)) {
+    return [];
+  }
+  return next
+    .map((row, i) => ({ id: row.id, priority: i + 1 }))
+    .filter((row) => Boolean(row.id));
+}

tests/unit/provider-swap-priority.test.js

@@ -0,0 +1,171 @@
+import { describe, expect, it } from "vitest";
+import { buildPriorityUpdates } from "../../src/app/(dashboard)/dashboard/providers/utils.js";
+
+// Simulates the server-side invariant enforced by connectionsRepo:
+// rows are ordered by priority ASC, ties broken by most-recently-updated,
+// then renumbered 1..N. Called after every persisted write.
+function flush(rows) {
+  const sorted = [...rows].sort((a, b) => {
+    const diff = (a.priority || 0) - (b.priority || 0);
+    if (diff !== 0) return diff;
+    return new Date(b.updatedAt || 0) - new Date(a.updatedAt || 0);
+  });
+  sorted.forEach((row, i) => {
+    row.priority = i + 1;
+  });
+  return sorted;
+}
+
+function makeConnections(priorities) {
+  return priorities.map((priority, i) => ({
+    id: `c${i}`,
+    priority,
+    updatedAt: new Date(2026, 0, i + 1).toISOString(),
+  }));
+}
+
+// Order of rows as the server would serve them back (priority ASC).
+function orderedIds(rows) {
+  return [...rows]
+    .sort((a, b) => a.priority - b.priority)
+    .map((row) => row.id);
+}
+
+// The fixed client: writes applied one at a time, server flushes after each.
+// Each PUT reaches the server in its own HTTP round trip, so the server's
+// updatedAt clock strictly increases between writes — modeled with a
+// monotonic counter (Date.now() in a tight loop would collide at the same
+// millisecond, which cannot happen across sequential HTTP requests).
+let writeClock = 0;
+function applySequential(rows, updates) {
+  const db = rows.map((row) => ({ ...row }));
+  for (const update of updates) {
+    const row = db.find((entry) => entry.id === update.id);
+    row.priority = update.priority;
+    writeClock += 1;
+    row.updatedAt = new Date(2026, 5, 1, 0, 0, 0, writeClock).toISOString();
+    flush(db);
+  }
+  return db;
+}
+
+// The old client: Promise.all — all writes land before the last flush.
+function applyParallel(rows, updates) {
+  const db = rows.map((row) => ({ ...row }));
+  for (const update of updates) {
+    const row = db.find((entry) => entry.id === update.id);
+    row.priority = update.priority;
+    row.updatedAt = new Date().toISOString();
+  }
+  flush(db);
+  return db;
+}
+
+// Every single move of `current` must converge to its requested order.
+function expectEverySingleMoveConverges(current) {
+  const n = current.length;
+  for (let from = 0; from < n; from += 1) {
+    for (let to = 0; to < n; to += 1) {
+      if (from === to) continue;
+      const next = [...current];
+      const [moved] = next.splice(from, 1);
+      next.splice(to, 0, moved);
+
+      const rows = applySequential(current, buildPriorityUpdates(next));
+      expect(orderedIds(rows)).toEqual(next.map((c) => c.id));
+    }
+  }
+}
+
+describe("buildPriorityUpdates", () => {
+  it("writes every row's final 1-based position", () => {
+    const current = makeConnections([1, 2, 3, 4, 5]);
+    const next = [...current];
+    [next[1], next[2]] = [next[2], next[1]];
+
+    const updates = buildPriorityUpdates(next);
+
+    expect(updates).toEqual([
+      { id: "c0", priority: 1 },
+      { id: "c2", priority: 2 },
+      { id: "c1", priority: 3 },
+      { id: "c3", priority: 4 },
+      { id: "c4", priority: 5 },
+    ]);
+  });
+
+  it("returns an empty set when the order is already final", () => {
+    const current = makeConnections([1, 2, 3]);
+    expect(buildPriorityUpdates([...current])).toEqual([]);
+  });
+
+  it("lands every possible single move (all from→to pairs, sizes 2–8, pristine priorities)", () => {
+    // The UI exposes only single moves (up/down arrows). With sequential
+    // ascending writes, every one of them must land exactly.
+    for (let n = 2; n <= 8; n += 1) {
+      expectEverySingleMoveConverges(makeConnections(Array.from({ length: n }, (_, k) => k + 1)));
+    }
+  });
+
+  it("lands every single move when stored priorities contain duplicates", () => {
+    // Bulk import can leave tied priorities (POST defaults to 1). The server
+    // re-breaks ties by most-recently-updated on every PUT, so skipped rows
+    // would drift — every row must be written. This case diverged 440/572
+    // under the previous changed-rows-only strategy.
+    const shapes = [
+      [1, 3, 1],
+      [1, 3, 1, 3, 1],
+      [2, 2, 2, 2],
+      [2, 2, 2, 5],
+      [7, 7, 1, 1, 4, 4],
+    ];
+    for (const shape of shapes) {
+      expectEverySingleMoveConverges(makeConnections(shape));
+    }
+  });
+
+  it("converges from the [1,3,1] middle→last case that diverged under changed-rows-only writes", () => {
+    // Minimal regression case from review: writes that skipped tied rows let
+    // the server's updatedAt tiebreak reorder them behind the client's back.
+    const current = makeConnections([1, 3, 1]);
+    const next = [...current];
+    const [moved] = next.splice(1, 1);
+    next.splice(2, 0, moved);
+
+    const rows = applySequential(current, buildPriorityUpdates(next));
+    expect(orderedIds(rows)).toEqual(["c0", "c2", "c1"]);
+  });
+
+  it("lands consecutive rapid moves (each applied against the previous result)", () => {
+    let rows = makeConnections([1, 2, 3, 4, 5]);
+
+    // Move last row up twice in a row — like a user clicking quickly.
+    for (const [from, to] of [[4, 3], [3, 2]]) {
+      const current = [...rows].sort((a, b) => a.priority - b.priority);
+      const next = [...current];
+      const [moved] = next.splice(from, 1);
+      next.splice(to, 0, moved);
+      rows = applySequential(current, buildPriorityUpdates(next));
+      expect(orderedIds(rows)).toEqual(next.map((c) => c.id));
+    }
+    expect(orderedIds(rows)).toEqual(["c0", "c1", "c4", "c2", "c3"]);
+  });
+
+  it("fixes the raw-index collision bug (#329) where parallel legacy writes promoted the row to the top", () => {
+    const current = makeConnections([1, 2, 3, 4, 5]);
+
+    // Old client behavior: raw 0-based indices for the swapped pair, in
+    // parallel. The server renumber promoted c2 all the way to the top.
+    const legacyRows = applyParallel(current, [
+      { id: "c2", priority: 1 },
+      { id: "c1", priority: 2 },
+    ]);
+    expect(orderedIds(legacyRows)).toEqual(["c2", "c0", "c1", "c3", "c4"]);
+
+    // Fixed behavior reaches the requested one-position move instead.
+    const next = [...current];
+    [next[1], next[2]] = [next[2], next[1]];
+    const rows = applySequential(current, buildPriorityUpdates(next));
+    expect(orderedIds(rows)).toEqual(["c0", "c2", "c1", "c3", "c4"]);
+  });
+});