Skip to content

feat: add catalog gateway actions - #81

Merged
vishu-bh merged 8 commits into
mainfrom
feat/catalog-gateway-actions
Aug 28, 2026
Merged

feat: add catalog gateway actions#81
vishu-bh merged 8 commits into
mainfrom
feat/catalog-gateway-actions

Conversation

@vishu-bh

@vishu-bh vishu-bh commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add Test and Disconnect actions for connected catalog MCP servers
  • poll async gateway deletion and show impact confirmation
  • refresh catalog registration with backend gateway IDs
  • cover API, component, unit, and E2E paths

Validation

  • npm run build
  • npm run lint:fix
  • npm test
  • npm run e2e

@marekdano
marekdano requested a review from a-effort August 26, 2026 08:11
gcgoncalves
gcgoncalves requested changes Aug 26, 2026
View reviewed changes
Comment thread src/pages/ServerCatalog.tsx Outdated
Comment thread src/pages/ServerCatalog.tsx Outdated
message: string;
}

const DISCONNECT_POLL_TIMEOUT_MS = 60_000;

@gcgoncalves gcgoncalves Aug 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

60-second timeout may be too long for user experience, it's a looong wait before before seeing timeout error. Consider shorter timeout with retry option:

const DISCONNECT_POLL_TIMEOUT_MS = 30_000; // 30 seconds

Then, In error handling:

setRegistrationNotification({
  type: "error",
  message: intl.formatMessage(
    { id: "mcpServer.catalog.disconnectPending" },
    { name: server.name }
  ),
  action: {
    label: intl.formatMessage({ id: "mcpServer.catalog.retry" }),
    onClick: () => void handleDisconnect(server)
  }
});

marekdano
marekdano reviewed Aug 26, 2026
View reviewed changes

@marekdano marekdano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 Blocking

  1. Shared abort ref cancels wrong server's disconnect
    src/pages/ServerCatalog.tsx:607

disconnectPollAbortRef is one page-wide ref, not scoped per server. Retrying server A's timed-out disconnect while disconnecting server B aborts A's poll silently — A's disconnect state is abandoned client-side even if the backend delete is still pending.

  1. Impact preview gated on wrong permission
    src/pages/ServerCatalog.tsx:496
    Gateway impact preview before disconnect checks canTest (read permission) instead of delete-relevant access, so a user who can delete but not read a gateway never sees which virtual servers are affected before confirming a destructive action.

⚠️ Major

  1. Disconnect notification overwritten across servers
    src/pages/ServerCatalog.tsx:595

An unresolved "still disconnecting — Retry" notification for server A is silently clobbered if the user disconnects server B next, losing the only way to follow up on A.

  1. testSuccess / testFailure strings are identical
    src/i18n/locales/en-US/mcpServer.json:104 (same in es-ES, pt-BR)

Success and failure connectivity-test messages read the same text — colorblind or screen-reader users can't tell a passed test from a failed one.

🟡 Minor

  1. Persistent poll errors read as ordinary timeouts
    src/pages/ServerCatalog.tsx:530

waitForCatalogDisconnect treats every refetch failure as transient, so a real backend error (403/500) during polling looks identical to normal slowness — the user just sees "still disconnecting" forever.

💡 Suggestion

  1. Duplicate DELETE /gateways/{id} implementation
    src/api/catalog.ts:26
    disconnectCatalogGateway re-implements what serversApi.delete already does, just to get status/headers via deleteWithMeta. Two places to keep in sync on backend contract changes.

@a-effort

a-effort commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Disconnect blocks dismissal for the length of the poll. ConfirmDialog.tsx:49 returns early on close while isLoading, and Cancel is disabled, so on the 202 path the dialog cannot be closed by Escape, overlay click, or Cancel until waitForCatalogDisconnect resolves, up to 30 seconds. The delete is already accepted by then, so the dialog can close and let the notification carry the pending state.

Impact preview failure renders identically to no impact. The catch sets impactPreview to null (ServerCatalog.tsx:506) and the list block is also skipped when servers.length is 0, so a failed lookup and an empty result both show an empty dialog body. Ahead of an irreversible action that reads as "nothing will break." Worth an explicit no-affected-servers state plus a separate could-not-check state.

"Still disconnecting" is styled as a failure. InlineNotification only supports success and error, so the pending case gets CircleAlert, red text, and role="alert". It is pending, not failed, and wants the info or warning type from the icon set settled in #62.

Focus lands on <body> after a successful disconnect. The card swaps its dropdown trigger for the Add button, so Radix restores focus to a removed node. ConfirmDialog accepts onCloseAutoFocus for this, and Resources.tsx:660 uses it for the same delete-removes-the-card case; ServerCatalog does not pass it.

testCatalogServer can resolve null, since GatewayTestResponse is nullable. That renders "{name} responded with status 0 in 0 ms" and classifies it as an error. Separate from the identical-strings point above, since it survives fixing those strings.

Nothing on the card indicates that test or disconnect is running. testingServerIds and disconnectingServerIds only disable items in a menu that has already closed, so Test connection shows no state until the notification lands, and the card holds its Connected badge through the whole disconnect poll.

The aria-live on the impact loading text sits on a conditionally rendered <p> (ServerCatalog.tsx:794), so the region mounts together with its content and most screen readers will not announce it. Needs a wrapper that stays mounted with the text swapping inside.

@a-effort

a-effort commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Disconnect visual reference added.

a-effort
a-effort reviewed Aug 27, 2026
View reviewed changes
Comment thread src/components/server-catalog/CatalogResults.tsx Outdated
import { useEffect, useId, useRef, useState } from "react";
import type { ReactNode } from "react";
import { CircleCheck, EllipsisVertical, FileText, Plus } from "lucide-react";
import { CircleCheck, EllipsisVertical, FileText, PlugZap, Plus, Unplug } from "lucide-react";

@a-effort a-effort Aug 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import not needed.

Please remove the icons - ty!

Image

@vishu-bh vishu-bh mentioned this pull request Aug 27, 2026
Closed
22 tasks
@vishu-bh
vishu-bh requested review from a-effort and marekdano August 27, 2026 13:42
@vishu-bh

vishu-bh commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @a-effort and @marekdano for the review.

updated screenshots

image image

Please review when you get a chance

marekdano
marekdano approved these changes Aug 27, 2026
View reviewed changes

@marekdano marekdano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All issues addressed!

LGTM 🚀

@vishu-bh
feat: add catalog gateway actions
d4ca625 
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
@vishu-bh
fix: accept SSE when testing catalog servers
d190454 
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
@vishu-bh
fix: improve catalog disconnect feedback
40a30de 
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
@vishu-bh
test: cover catalog disconnect retry
b5fb5fe 
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
@vishu-bh
test: cover catalog disconnect polling
5d194b0 
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
@vishu-bh
fix: harden catalog disconnect flow
3b9d25e 
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
@vishu-bh
fix: address catalog action review feedback
a42271e 
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
@vishu-bh
vishu-bh force-pushed the feat/catalog-gateway-actions branch from 1ca3e4b to a42271e Compare August 27, 2026 21:59
@vishu-bh
test: fix catalog registration CSRF fixture
6035f43 
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
@vishu-bh
vishu-bh merged commit 56179ec into main Aug 28, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@marekdano marekdano marekdano approved these changes
@gcgoncalves gcgoncalves gcgoncalves approved these changes
@a-effort a-effort Awaiting requested review from a-effort

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vishu-bh @a-effort @marekdano @gcgoncalves