feat(web): reorderable ServerCards — drag-and-drop + keyboard (#1369) (#1447)

* feat(web): reorderable ServerCards — drag-and-drop + keyboard (#1369)

Let users control the order of servers on the Servers screen instead of
hand-editing mcp.json. Each ServerCard gets a grip handle (in the header,
left of the name) that drives an @dnd-kit sortable; pointer and keyboard
sensors both work, with live-region announcements for screen readers.

- Backend: PUT /api/servers/order rewrites mcp.json in the supplied order
  inside the write lock, reusing the atomic-write + watcher-notify path.
  Registered before /:id so "order" isn't captured as a server id. Rejects
  with 409 when the id set no longer matches disk and 400 on duplicate /
  malformed bodies, so a reorder racing an external edit can't drop or
  duplicate an entry.
- Hook: useServers.reorderServers(orderedIds) — optimistic local reorder,
  PUT, refresh-to-disk-truth on failure.
- UI: SortableServerCard wrapper owns the dnd-kit concerns; ServerListScreen
  wraps the grid in DndContext + SortableContext (rect strategy) with
  pointer + keyboard sensors and announcements. Reorder math and a11y copy
  extracted to reorderIds.ts / serverReorder.ts for direct unit testing.
  Renders plain cards (no grip) when no onReorder is wired.
- Wired onServerReorder through InspectorView -> App.tsx.

Tests: pure-helper units, hook (persist / optimistic / 409 revert), 9
backend integration tests (incl. id-set-mismatch rejection and the
route-registration guard), ServerCard/ServerListScreen units, and a
real-browser Storybook keyboard-reorder play (the dnd-kit keyboard sensor
needs layout rects that happy-dom doesn't provide).

Adds @dnd-kit/core, @dnd-kit/sortable, @dnd-kit/utilities.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(web): notify on reorder failure; skip reorder glue when not reorderable

Address PR #1447 review:
- App.tsx: catch reorderServers rejection (409 from a racing external edit,
  or a network error) and show a red toast, matching the error-handling
  pattern used by every other mutation — previously the rejection was
  discarded with `void` and the drag silently bounced back.
- ServerListScreen: only build handleDragEnd / announcements when
  `reorderable`, so the four-closure announcements object isn't allocated
  every render when no onReorder is wired.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Cliff Hall 2026-06-09 14:15:58 -04:00 • committed by GitHub
commit d5f12b2a82
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 1039 additions and 27 deletions

View file

@ -1514,6 +1514,67 @@ export function createRemoteApp(
}
});
// Reorder the on-disk `mcpServers` map. Registered before
// `PUT /api/servers/:id` so the literal `/order` segment isn't captured
// as an `:id` param ("order" would otherwise be a valid store id). The
// request body is `{ order: string[] }` — the complete set of server ids
// in the desired iteration order. We reject (409) unless that set matches
// the on-disk set exactly, so a reorder racing an external add/remove
// can't silently drop or duplicate an entry. Reordering touches no secret
// values, so we just permute the existing stripped entries and reuse the
// same atomic-write + watcher-notify path as the other mutators.
app.put("/api/servers/order", async (c) => {
let body: { order?: unknown };
try {
body = (await c.req.json()) as { order?: unknown };
} catch {
return c.json({ error: "Invalid JSON body" }, 400);
}
if (
!Array.isArray(body.order) ||
!body.order.every((id): id is string => typeof id === "string")
) {
return c.json({ error: "order must be an array of strings" }, 400);
}
const order = body.order;
if (new Set(order).size !== order.length) {
return c.json({ error: "order contains duplicate ids" }, 400);
}
try {
return await withWriteLock(async () => {
const current = await readMcpConfig();
const currentIds = Object.keys(current.mcpServers);
// Exact-set match: same size and every requested id present on disk.
// The duplicate check above plus equal sizes guarantees this is a
// permutation, never a partial reorder that would drop an entry.
const currentSet = new Set(currentIds);
const sameSet =
currentIds.length === order.length &&
order.every((id) => currentSet.has(id));
if (!sameSet) {
return c.json(
{
error:
"order does not match the current server set (it may have changed on disk)",
},
409,
);
}
const next: MCPConfig = { mcpServers: {} };
for (const id of order) {
// Non-null: `sameSet` proves every `id` is a key of the map.
next.mcpServers[id] = current.mcpServers[id]!;
}
await writeMcpAndTrackMtime(serializeStore(next));
return c.json({ ok: true });
});
} catch (error) {
const msg = error instanceof Error ? error.message : String(error);
return c.json({ error: `Failed to reorder servers: ${msg}` }, 500);
}
});
app.put("/api/servers/:id", async (c) => {
const originalId = c.req.param("id");
if (!originalId || !validateStoreId(originalId)) {

View file

@ -44,6 +44,14 @@ export interface UseServersResult {
settings: InspectorServerSettings,
) => Promise<void>;
removeServer: (id: string) => Promise<void>;
/**
* Persist a new ordering for the server list. `orderedIds` must be the
* complete set of current server ids in the desired order. The local list
* is reordered optimistically so the grid reflows instantly; on backend
* failure (e.g. the on-disk set changed underneath us) we re-fetch to snap
* back to disk truth. Routes through `PUT /api/servers/order`.
*/
reorderServers: (orderedIds: string[]) => Promise<void>;
}
function buildHeaders(
@ -252,6 +260,45 @@ export function useServers(opts: UseServersOptions): UseServersResult {
[base, authToken, doFetch, refresh],
);
const reorderServers = useCallback(
async (orderedIds: string[]): Promise<void> => {
// Optimistic reorder: rebuild the local list in the requested order so
// the grid reflows immediately, before the round-trip resolves. Built
// from a lookup off the previous state (inside the setter) so we never
// capture a stale snapshot, and any id not currently present is simply
// skipped rather than producing an `undefined` hole.
setServers((prev) => {
const byId = new Map(prev.map((s) => [s.id, s]));
const reordered = orderedIds
.map((id) => byId.get(id))
.filter((s): s is ServerEntry => s !== undefined);
// Defensive: if the requested order dropped any entries (shouldn't
// happen — callers pass the full set), keep the strays at the end so
// nothing vanishes from the UI before the refresh reconciles.
if (reordered.length !== prev.length) {
const seen = new Set(orderedIds);
for (const s of prev) if (!seen.has(s.id)) reordered.push(s);
}
return reordered;
});
try {
const res = await doFetch(`${base}/api/servers/order`, {
method: "PUT",
headers: buildHeaders(authToken, true),
body: JSON.stringify({ order: orderedIds }),
});
if (!res.ok) {
throw new Error(await readErrorMessage(res));
}
} catch (err) {
// Revert to disk truth — the optimistic order may not have landed.
await refresh();
throw err instanceof Error ? err : new Error(String(err));
}
},
[base, authToken, doFetch, refresh],
);
return {
servers,
loading,
@ -261,5 +308,6 @@ export function useServers(opts: UseServersOptions): UseServersResult {
updateServer,
updateServerSettings,
removeServer,
reorderServers,
};
}