fix(ui): add caller-scoped OAuth status handling

[vishu-bh]

Summary

• add a centralized, batched caller-scoped OAuth status client and race-safe React hook
• share OAuth-aware status presentation across Catalog, MCP Servers, and virtual-server source views
• add authoritative post-authorization refresh, retry handling, permission-aware actions, and localized status copy
• reuse existing shadcn components and shared UI patterns

Testing

• full Vitest suite: 3534 passed, 1 skipped
• focused OAuth suites: 248 passed
• Playwright catalog/auth smoke: 17 passed
• npm run build
• npm run lint
• npx tsc --noEmit

The pre-push suite later hit the existing flaky App navigation test once; the same test passed immediately when rerun alone.

Resolves IBM/mcp-context-forge#6459

[marekdano]

Findings

🔴 High — fix before merge

1. OAuth statuses flicker to "loading" and re-fetch on any id-set change

File: src/hooks/useOAuthStatuses.ts

The refresh effect keys off idsKey (the full joined id set). On any change to that set — one id added or removed — it calls load(currentIds) with the entire current set, and load() unconditionally resets every targeted id to {state: "loading"} before refetching, including ids whose status was already resolved and unchanged.

Repro: On /app/servers, click "Load More" (or delete a server, or connect/disconnect a catalog card). Every already-displayed OAuth badge — even for unrelated, already-authorized servers — flashes to "Checking authorization" and re-issues a network call.

Fix direction: only load() the ids that are new to the set; only clear entries for ids that dropped out of scope.

---

2. Disabled OAuth servers no longer fall back to "Inactive" when status is loading/unavailable

File: src/lib/serverStatus.ts — getServerAvailability

if (oauthStatus?.state === "loading") return "authorization_checking";
if (oauthStatus?.state === "unavailable") return "authorization_unavailable";

These now outrank enabled/reachable unconditionally. The deleted test explicitly guaranteed the opposite for the old equivalent ("unknown" token status did not outrank connectivity). No new test covers a disabled server combined with loading/unavailable, and disabled OAuth servers are still included in the status-fetch list — so a disabled server can now show "Authorization status unavailable" indefinitely instead of "Inactive" (e.g. on a transient 500).

Fix direction: decide if this is intended; if not, gate the OAuth-state short-circuit on server.enabled, and add a regression test for the disabled+unavailable/loading combo.

---

🟡 Medium — worth fixing, not blocking

3. Raw unparseable timestamp could leak to the UI

File: src/components/servers/ServerStatusDetail.tsx:39

formatLocalDateTime(lastSeen, lastSeen)

formatLocalDateTime returns its second arg (fallback) whenever the value is unparseable. Passing lastSeen as its own fallback means a malformed backend timestamp gets rendered raw instead of blank. Previously formatLocalDateTime(lastSeen, ""). Looks like a copy-paste of the variable name rather than an intentional change.

4. Independent awaits serialized unnecessarily

File: src/pages/ServerCatalog.tsx (handleOAuthSubmit, handleAuthorize)

await refreshOAuthStatus(gatewayId);
await serversApi.fetchToolsAfterOAuth(gatewayId);

Neither call depends on the other's result (both only depend on the preceding toggleEnabled). Serializing them adds the full OAuth-status fetch latency in front of the tools refresh on every OAuth-completion path. Could run via Promise.allSettled.

---

🟢 Low — nits (reuse/duplication, not bugs)

5. OAuth-server classification duplicated three ways

• src/lib/serverStatus.ts → exported isOAuthServer
• src/pages/ServerCatalog.tsx → local isOAuthServer
• src/components/server-catalog/CatalogResults.tsx → inline equivalent in getOAuthCardState/isOAuthCardUsable

Different shapes (MCPServer vs CatalogServer), but same concept — a future rule change (new auth type, renamed config flag) is easy to apply to one and miss the others.

6. Id-sanitizing logic duplicated

normalizedIds() in useOAuthStatuses.ts vs uniqueGatewayIds() in src/api/oauth.ts — both trim/dedupe/filter a gateway-id list (one also sorts). Low risk, but two places to keep in sync.

[gcgoncalves]

Minor requests.

[a-effort]

FYI (no action needed)... I split a refactor out of #153 into #160, which moves the presentational part of ServerStatusIndicator into ui/status-indicator. It preserves behaviour, and ServerStatusIndicator.test.tsx is unchanged from main.

It rewrites the same return statement #150 does. Since #150 is ahead in review, I will rebase #160 onto it once it merges and re-apply the move on top of your version. Your popover content stays ServerStatusDetail, it just becomes children of the shared component, with onRetry and authorizationManagementHint passed through as they are now.

#160 also gives PopoverContent an aria-label. Radix announces it as an unnamed dialog without one, which is main's behaviour today, so nothing in #150 needs to change for it.

[vishu-bh]

Addressed review findings in signed commit 1cf99ef.

• OAuth hook now fetches only added IDs, preserves unchanged resolved entries, removes dropped IDs without refetching survivors, and remains safe under Strict Mode cleanup.
• Disabled-server OAuth precedence remains intentional: caller-scoped loading/unavailable state still outranks lifecycle state so a failed authorization check never implies usable authorization. Added explicit disabled + loading/unavailable regression tests.
• Invalid last-seen timestamps now use an empty fallback.
• Post-authorization status and component refreshes run concurrently with independent component-failure handling.
• Catalog OAuth classification and gateway-ID normalization are centralized.
• Removed dead translation keys and replaced the string-prefix state check with a typed helper.

Validation: 3,572 unit tests passed, branch coverage 90.11%, production build and lint passed, catalog/OAuth Playwright smoke 13/13 passed.

Please re-review when ready.

[marekdano]

Findings

🔴 High

1. Missing permission gate on OAuth tool refresh

File: src/pages/ServerCatalog.tsx (refreshGatewayData)

The new shared helper calls serversApi.fetchToolsAfterOAuth(gatewayId) unconditionally. Servers.tsx gates the equivalent call behind hasPermission("gateways.update"); the catalog page doesn't. A user without gateways.update will authorize an OAuth catalog server, hit a 403, and always see "authorized, but tools could not be fetched."

2. Gateway ID validation dropped on the new OAuth status path

File: src/api/oauth.ts (getOAuthStatuses / normalizeGatewayIds)

Only trims/dedupes IDs — the validateServerId() character-allowlist that every other method in servers.ts enforces is gone from this path. The old serversApi.getOAuthStatus (servers.ts:224) still has it but has no remaining callers (dead code).

🟡 Medium

3. useOAuthStatuses unguarded by permission in two call sites

Files: src/components/gateways/SourceSelection.tsx:110, src/pages/CreateServer.tsx:569

Both call useOAuthStatuses(oauthServerIds) with no enabled gate, while Servers.tsx/ServerCatalog.tsx gate the identical hook on hasPermission("gateways.read") — inconsistent with the PR's own permission-aware-actions goal.

🟢 Low (cleanup)

4. Duplicated retry-eligibility logic

Files: src/components/servers/ServerStatusIndicator.tsx:42, src/pages/CreateServer.tsx:464

oauthStatus?.state === "unavailable" && oauthStatus.retryable is inlined in both places instead of a shared helper (unlike the startsWith("authorization_") check, which was consolidated into isAuthorizationAvailability in the latest commit).

5. Dead i18n keys — mcpServer.json (en-US/es-ES/pt-BR)

mcpServer.status.action.error, mcpServer.status.action.authorizing — zero references in src/.

6. Dead i18n keys — gateways.json (en-US/es-ES/pt-BR)

gateways.source.status.active, .warning, .offline, .inactive — zero references in src/.

[a-effort]

Approving.

The findings from the last round are addressed. The refresh effect now loads only newly added ids and prunes removals without refetching, with a per-id version guard so a late response cannot overwrite a newer one. refreshGatewayData gates fetchToolsAfterOAuth behind canUpdateServer. validateServerId is back in normalizeGatewayIds, and the unused serversApi.getOAuthStatus is gone.

CI has not run on b4d29f5, since the conflict stops GitHub building the merge ref, so the commit carrying those fixes has no check evidence. I ran it locally: 387 tests pass across the affected files, tsc and eslint are clean, and the three locales carry matching key sets with no dangling message ids.

[a-effort]

Merge note.

#153 added src/components/ui/status-indicator.tsx to main, and ServerStatusIndicator.tsx now uses that shared component and passes contentAriaLabel, which gives the popover an accessible name:

contextforge-web-ui/src/components/servers/ServerStatusIndicator.tsx

Lines 108 to 130 in 1b4ce31

	<StatusIndicator
	Icon={StatusIcon}
	iconClassName={presentation.iconClassName}
	label={label}
	fullLabel={isAbbreviated ? fullLabel : undefined}
	triggerAriaLabel={intl.formatMessage(
	{ id: "mcpServer.status.trigger" },
	{ name: server.name, status: fullLabel },
	)}
	contentAriaLabel={intl.formatMessage(
	{ id: "mcpServer.status.detail.label" },
	{ name: server.name },
	)}
	interactive={interactive}
	className={className}
	>
	<ServerStatusDetail
	availability={availability}
	enabled={server.enabled}
	lastSeen={server.lastSeen}
	lastError={server.lastError}
	/>
	</StatusIndicator>

This branch still carries its own copy of the markup, so keeping it would remove that accessible name. The status changes here should be rewritten to use the shared component.