[vishu-bh]

Surface an explicit Test connection action after OAuth authorization on MCP Servers. Share inline stage results between the table row and details panel, enforce read/update permission checks, and clear stale results on configuration or authorization changes. Map structured callback failures to localized guidance.

Part 3 of the frontend split for IBM/mcp-context-forge#5722. Based on #220 in native GitHub stack #223. Catalog integration follows separately.

Validation: production build, ESLint, full frontend coverage suite, and browser tests for row/details authorization, explicit validation, scope failure and reauthorization, read-only callers, and callback errors. Browser screenshots remain outside the repository.

Native GitHub stack: #223, rooted at main.

1. feat: define OAuth connection validation contract #219 — Handwritten OAuth compatibility contract; base: main.
2. feat: add shared OAuth connection validation #220 — Shared validation; base: feat: define OAuth connection validation contract #219's branch.
3. feat: surface OAuth validation on MCP Servers #221 — MCP Servers UI; base: feat: add shared OAuth connection validation #220's branch.
4. feat: surface OAuth validation on Catalog cards #222 — Catalog UI; base: feat: surface OAuth validation on MCP Servers #221's branch.

#218 contains the independent E2E typing fix and can merge separately. These PRs replace #216.

[marekdano]

Findings

Suggestions

1. Failure dialogs now drop the provider's description (src/pages/Servers.tsx, in handleAuthorize)
   The reason is now oauthErrorMessage(intl, err.errorCode), and errorCode falls back to code. Any code outside the allowlist maps to the generic "OAuth connection failed. Check the configuration and try again." That includes invalid_grant, oauth_error without a structured errorCode, and provider-specific codes. Before this change the user saw the sanitized errorDescription. The allowlist is the safe choice, but some failures become harder to diagnose. Consider falling back to the sanitized description (error.message) when the code isn't in the allowlist. This also affects the OAuth dialog flow Catalog will reuse in #222.

2. The changed tests no longer prove what they claim (src/pages/Servers.test.tsx, e2e/mcp-oauth-outcomes.spec.ts)
   The mocks now pass the localized string as the OAuthAuthorizationError description, for example new OAuthAuthorizationError("access_denied", "The provider denied authorization…"). The description is ignored, so the tests pass only because of the code mapping. Keep the original provider description in the mock and assert that it is not displayed.

3. The validation call uses the cached server.url, not a fresh saved gateway (src/pages/Servers.tsx, in renderOAuthValidation)
   onTest sends { gatewayId: server.id, baseUrl: server.url } from list or details state. If the server was edited in another tab or session, the request carries a stale URL. The hook from #220 supports a resolver function that fetches the saved gateway and cancels while doing so, and using it here would make the "exact saved gateway" guarantee hold. If the backend rejects a mismatch, the user gets request_invalid with no explanation. Is the plain object intentional?

4. The result can reset when the details panel opens or closes (src/pages/Servers.tsx, validationTargets)
   validationTargets merges servers and panelServer by id, so the details copy overrides the list copy. If oauthConfigurationKey differs between the two payloads (for example, masked or omitted oauthConfig fields in the list response), opening or closing details changes the key and silently clears the result. Please verify that list and details produce identical keys for the same server. A test that opens details after a result and expects it to survive would cover this. This is also another reason to make the key canonical (see #220, suggestion 4).

5. The read-only tests can pass before OAuth status has loaded (e2e/oauth-connection-validation.spec.ts, src/pages/Servers.test.tsx)
   The button only renders once the status is ready and the token is valid. Asserting its absence right after the server name appears passes trivially if the status request is still pending. Wait for the status response, or add a positive control, such as the same setup with full permissions in the same test. In e2e, use toHaveCount(0) instead of not.toBeVisible(), since the latter also passes for a missing element.

6. Leftover split scaffolding in the e2e spec
   for (const catalog of [false]) makes all the catalog branches and the catalog route mocks dead code in this PR. They belong in #222. Remove them here to keep the spec readable.

7. Possible duplicate "Test connection" button in the details panel (src/components/servers/MCPServerDetailsPanel.tsx)
   The stored-token validation sits directly above the existing TestConnectionPanel in the "Try it" tab. If that panel also has a "Test connection" button, there are two similarly named controls with different meanings: stored-token strict validation and a generic handshake. Consider distinguishing the label, for example "Test stored authorization". I couldn't confirm the existing label from my checkout.

[gcgoncalves]

This is cool, I didn't know you could use a fragment like this to add attributes to it! 👏

[gcgoncalves]

We're dropping the authorization server message here, replacing it with our fixed intl one. Can we add the original as part of the message?

e.g.: something like ${oauthErrorMessage(intl, err.errorCode)} ${intl.formatMessage(CONNECTOR_MESSAGE_ID_HERE)} ${oauthFailureReason(err)}

[gcgoncalves]

For e-readers, add an aria-label with something like ${message(running ? "testing" : entry ? "retest" : "test")} ${mcpServer.name}. This will preserve the layout but add some more info for screen-reader users.

[gcgoncalves]

renderOAuthValidation?.(server, "result") is being called twice, here. Might be worth extracting to a const and then using for the check and for display.

[gcgoncalves]

A better approach here is oauthAttempts[server.id]?.state === "pending", it's state derived and not ref-dependent.

[marekdano]

The PR looks solid now!

LGTM 🚀

[gcgoncalves]

Missing intl