Add server catalog card actions and registration flow

[vishu-bh]

Summary

• align server catalog cards with the Figma action layout
• add open catalog servers through POST /v1/catalog/{catalog_id}/register
• refresh cards to show connected state after successful registration
• add localized labels plus unit, API-contract, and Playwright coverage

Why

Catalog cards exposed read-only details but did not provide the designed Add/Connected actions or bind registration to the catalog API.

User impact

Users can now add an open catalog server directly from its card, see request progress and safe errors, and view refreshed connected state after registration.

Validation

• npm run build
• npm run lint:fix
• npm test (npm run test:run is not defined)
• npm run e2e — 180 tests passed

[marekdano]

Two findings are blocking; the rest are non-blocking suggestions.

1. BLOCKED — High

File: src/pages/ServerCatalog.tsx:216

handleAdd's catch block covers both the register call and the post-success refetch(), so a refetch failure after a successful registration is caught by the same generic error handler.

Failure scenario: registerCatalogServer succeeds (server is now registered on the backend), but the subsequent await refetch() (line 216) throws — a transient network blip, aborted request, or 5xx on GET /v1/catalog.
The catch at lines 217-218 fires and shows "Unable to add this server. Try again." even though the add succeeded. The user may re-click Add, potentially double-registering, while believing the first attempt failed.

2. BLOCKED — High

File: src/pages/ServerCatalog.tsx:150

addingServerId is a single shared value for the whole page, not per-server state, so starting an Add on a second server while a first Add is still in flight clears the first server's disabled/adding indicator.

Failure scenario: User clicks Add on server A (addingServerId='A', A's button disabled + "Adding…"). Before A's request resolves, the user clicks Add on server B — addingServerId is overwritten to 'B'. Card A's isAdding (disabled={isAdding} in CatalogResults.tsx:150) becomes false even though A's registerCatalogServer call is still pending, re-enabling A's Add button and permitting a duplicate concurrent registration request for the same server. Whichever request's finally resolves first also nulls addingServerId out from under the other still-in-flight request's UI state.

3. Suggested — Medium

File: src/components/server-catalog/CatalogResults.tsx:160

For a registered/connected server, the card renders two separate controls that both open the same details dialog: the ellipsis menu's "View details" item and the always-rendered standalone "View {name}" icon button.

Failure scenario: For any server with is_registered=true, the DOM contains both an "Actions for {name}" button (lines 121-132, whose menu item at 135-141 calls onView) and a separate unconditional "View {name}" button (lines 160-171) that calls the same onView handler — confirmed by the PR's own test asserting both "Actions for Globalping" and "View Globalping" buttons exist simultaneously (src/pages/ServerCatalog.test.tsx:409-411). This is redundant, confusing UI rather than an intentional secondary affordance — the non-registered branch only shows one action.

4. Suggested — Medium

File: src/components/server-catalog/CatalogResults.tsx:134

Opening the details Dialog from a DropdownMenuItem's onSelect (without suppressing the dropdown's default close-auto-focus) can race Radix's automatic focus-return-to-trigger against the Dialog's own focus trap.

Failure scenario: User opens the "Actions for {name}" menu and activates "View details" (lines 135-141). Radix DropdownMenu's default behavior on close is to return focus to its trigger button; simultaneously the newly opened Dialog (CatalogServerDetailsDialog) tries to move focus inside itself. Depending on timing, focus can end up back on the (now covered) ellipsis trigger instead of inside the modal, breaking keyboard/screen-reader users' ability to interact with the dialog until they tab out from behind it.

5. Suggested — Low

File: src/components/server-catalog/CatalogResults.tsx:84

requiresAuth = server.auth_type !== "Open" is dead code: CatalogResults is only rendered by ServerCatalog.tsx, which pre-filters the list to auth_type === "Open" servers only, so requiresAuth is always false for any card actually shown.

Failure scenario: getOpenServers/filterOpenServers (ServerCatalog.tsx:104-128) strip out every non-Open server before CatalogResults ever sees it, so the KeyRound/Lock icon and "Auth required" CardTag (lines 93-102) can never render in production, and the fixture in ServerCatalog.test.tsx (apiKeyServer, auth_type: "API Key") is likewise filtered out before assertions run — the branch is unreachable and untested.

6. Suggested — Low

File: src/i18n/locales/en-US/mcpServer.json:27 (also es-ES, pt-BR)

The mcpServer.catalog.view translation key is now orphaned in all three locale files after the visible "View" button text was replaced with an icon-only FileText button.

Failure scenario: A repo-wide search shows no remaining mcpServer.catalog.view usage outside the locale JSON files themselves; the key (and its es-ES/pt-BR translations) is dead weight that future maintainers may mistakenly assume is still wired to UI.

[marekdano]

None of the issues are blocking, just suggestions for improvements

1. Registered-state override never reverts

File: src/pages/ServerCatalog.tsx:177
Category: Correctness

The optimistic registeredServerIds override on openServers is a one-way ratchet: once a server id is added, is_registered is forced to true forever, even if a later refetch's authoritative data says otherwise. Nothing in the code ever removes an id from the set.

Failure scenario: A server is registered via Add, then unregistered elsewhere (another session, or the backend gateway is later removed). The Add button stays hidden and the card shows "Connected" indefinitely — until a full page reload.

---

2. Add errors aren't differentiated

File: src/pages/ServerCatalog.tsx:240
Category: Correctness

handleAdd's catch-all collapses distinct backend error codes — 404 (unknown catalog id), 409 (already registered), per mcpgateway/routers/catalog.py — into one generic "Unable to add this server. Try again." message.

Failure scenario: Catalog data is stale (another session already registered the server, or it was removed from the catalog). Retrying just repeats the same failure with no differentiation and no forced refresh, until the user manually reloads.

---

3. Add button missing accessible name

File: src/components/server-catalog/CatalogResults.tsx:145
Category: Accessibility

The "Add"/"Adding…" button has no aria-label with the server name, unlike the sibling "View {name}" and "Actions for {name}" buttons in the same card.

Failure scenario: A screen-reader user on a catalog page with several unregistered servers hears multiple buttons all named exactly "Add" with no way to distinguish which server each belongs to.

---

4. Focus lost after successful Add

File: src/components/server-catalog/CatalogResults.tsx:102
Category: Accessibility

No explicit refocus target when a card's Add button unmounts after registration succeeds, unlike the View-dialog flow which restores focus via lastViewTriggerRef.

Failure scenario: A keyboard-only user tabs to a card's Add button and presses Enter. Once registration succeeds, the button is replaced by "Connected" + an Actions menu, and focus silently reverts to document.body — no test asserts focus location after a successful add.

---

5. Stale-data test gives false confidence

File: src/pages/ServerCatalog.test.tsx:426
Category: Test coverage

The "keeps cached catalog data visible during refreshes and refresh failures" test doesn't actually exercise a real stale-while-revalidate transition — each mockUseQuery call supplies a fresh full data object rather than simulating an in-flight/failed refetch on the same component instance.

Failure scenario: If a future change breaks the real "keep last good data during refetch" behavior (e.g. reintroducing an unconditional isLoading early-return), this test would still pass.

[a-effort]

Nice work! 🙌

Good call moving the view details option into an overflow menu after a server has been added. When disconnect is added, it can be in that menu, too. Let's add that as a followup.

Small, non-blocking adjustment request:

Right-align the catalog card's overflow menu:

The "View details" dropdown on connected server cards in the MCP server catalog was anchored by its left edge to the overflow button, so it opened to the right and hung off the card. Switching align from "start" to "end" anchors its right edge to the button instead, matching other overflow menus in the app (ServerActionsMenu, TokenActionsMenu, UserActionsMenu, ToolsTable, ResourcesTable, and others), which were already align="end".

Single-line change in src/components/server-catalog/CatalogResults.tsx:

                     <DropdownMenuContent
-                      align="start"
+                      align="end"
                       onCloseAutoFocus={(event) => {

No behavior changes beyond positioning; the focus-restoration logic in onCloseAutoFocus and the details dialog handoff are untouched, and there were no lint errors.

[marekdano]

Findings

🔴 Correctness — Race condition on concurrent catalog refetches

File: src/hooks/useQuery.ts:144

useQuery's execute()/refetch() has no request-correlation or cancellation, so concurrent refetch() calls race — the last response to arrive wins, not the last one issued.

Repro: Click Add on Server A, then quickly click Add on Server B (the isAdding guard in CatalogResults.tsx:165,314 is per-server, not global). Both handleAdd() calls (ServerCatalog.tsx:254-310) optimistically setData and then call refreshCatalogSilently() → refetch() against the same useQuery(CATALOG_PATH) instance. If A's GET resolves after B's, the stale A-response overwrites the newer catalog snapshot, silently reverting B's registration state until a later fetch happens to reconcile it.

---

🟡 Efficiency — Unnecessary full catalog refetch after every Add

File: src/pages/ServerCatalog.tsx:271

handleAdd triggers a full catalog refetch (up to 1000 entries) after every successful or 409 registration, even though the optimistic setData already reflects the correct state for that mutation.

Each Add click that succeeds or hits 409 does setData(...) (an accurate local patch) immediately followed by await refreshCatalogSilently() → refetch() of /v1/catalog?limit=1000 (lines 271–272, 275–283). This is a wasted round-trip on a hot user action, is the direct source of the race above, and adds no value over the already-correct optimistic patch for the success/409 paths — only the 404 case genuinely needs server-authoritative reconciliation.

---

🟡 Accessibility — Focus lost when a card is removed after 404

File: src/pages/ServerCatalog.tsx:157

On a registration 404, removeCatalogServer drops the card from the DOM with no focus management, unlike every other interaction in this file, which explicitly redirects focus (handleDetailsOpenChange ~line 312, and the CatalogCard focus-transfer effect in CatalogResults.tsx:88-98).

Repro: A keyboard user tabs to a stale catalog entry's Add button and activates it. The backend returns 404 (server no longer exists), removeCatalogServer (lines 157–171) filters the card out of servers, and the CatalogCard unmounts. Nothing calls .focus() on a fallback element, so the browser drops focus to document.body. The role="alert" toast is still announced via aria-live, but the user's tab position silently resets to the top of the page.