Skip to content

feat: virtual server add source step - #131

Open
a-effort wants to merge 12 commits into
6404-shared-server-status-badgefrom
6404-source-picker-status
Open

a-effort wants to merge 12 commits into
6404-shared-server-status-badgefrom
6404-source-picker-status

Conversation

@a-effort

@a-effort a-effort commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Stacked on #130, which adds the shared status indicator this uses. Please review that one first; this PR targets its branch so the diff stays readable.

The source picker now uses the shared derivation from #130. A source reads the same here as on the servers page, including OAuth sources the caller has not authorized yet. Auth note: this version includes a popover vs connecting to the OAuth authorize flow (IBM/mcp-context-forge#6859), because no authorize handler is wired into the picker.

Sources stay selectable in every state and components discovered from a source remain in the catalog when it goes unreachable, so a virtual server built from them still resolves. The health loop takes up to three intervals to mark a source unreachable, so this approach enables the user to move forward and fix any issues later, if there are any.

Tools, resources and prompts now fail independently, each with its own message (popover).

131.mp4

Part of IBM/mcp-context-forge#6404

The sources step listed every MCP server with a status it did not act on, so
a user could select a server that contributes nothing and get an empty virtual
server with no explanation.

Sources stay selectable in every state. Marking a gateway unreachable skips the
cascade that disables its components, so an offline source keeps a usable
catalog and adding it produces a working virtual server whose calls fail until
the server returns. Instead of blocking, each row carries the shared status
indicator and a submit-time note names any selected source with nothing to add.

Splits the zero-component empty state, which covered three situations with one
sentence. A failed list now names itself and offers a retry, an unavailable
source explains why it is empty, and a healthy source exposing nothing still
reads as empty.

Attributes a failed component fetch to the source that caused it, rather than
failing the whole create with one unattributed error.

Extracts useOAuthTokenStatuses, shared with the servers page.

Closes #6404

Signed-off-by: Anna Effort <anna.effort@ibm.com>
@a-effort a-effort changed the title feat: explain source availability in the create virtual server picker feat: virtual server add source step Sep 16, 2026
@a-effort
a-effort marked this pull request as ready for review September 16, 2026 19:15
a-effort and others added 6 commits September 16, 2026 13:22
The unreachable detail text says components stay listed from the last
sync, which reads as a contradiction when the list is empty. Adds an
empty-state message per availability, and drops the status keys and the
ServerStatus type left unused by the shared indicator.

Signed-off-by: Anna Effort <anna.effort@ibm.com>
Signed-off-by: Anna Effort <anna.effort@ibm.com>
…r-status

Signed-off-by: Anna Effort <anna.effort@ibm.com>
Recast the status explanations and empty states around the server rather than
ContextForge, and stop showing the last error on inactive servers, where the
health loop leaves it behind from an earlier outage.

Narrow the popover to max-w-xs and anchor it to the end of the status label.
The source picker also needed justify-self-start: its trigger is a grid item,
so it stretched to the full column and the panel anchored to the column edge.

Shorten the compact label to Auth, keeping the full word for screen readers.

Signed-off-by: Anna Effort <anna.effort@ibm.com>
Stack the status label and the authorizing label in one grid cell so the
wider of the two sets the width. Reserving it rather than setting a min-width
keeps it right per locale, where the two labels differ by different amounts.

The stack is start-aligned so the slack falls after the label, leaving the
icon and text where every other row has them.

Signed-off-by: Anna Effort <anna.effort@ibm.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

OAuth status loading fails once the accumulated server list exceeds the API’s 100-ID limit.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Uses shared MCP availability states throughout virtual-server source selection and improves component-loading feedback.

Changes:

  • Adds shared OAuth status loading and availability-aware empty states.
  • Keeps unavailable sources selectable while warning about empty sources.
  • Adds independent component errors, retries, and source-attributed creation errors.
File summaries
File Description
src/types/server.ts Removes the obsolete status type.
src/pages/Servers.tsx Adopts the shared OAuth-status hook.
src/pages/CreateServer.tsx Adds shared statuses, retries, and attributed errors.
src/pages/CreateServer.test.tsx Updates status and error tests.
src/lib/serverStatus.ts Adds availability-specific empty-state messages.
src/lib/serverStatus.test.ts Validates presentation message IDs.
src/hooks/useOAuthTokenStatuses.ts Centralizes OAuth token-status loading.
src/components/gateways/SourceSelection.tsx Adds shared statuses and empty-source warnings.
src/components/gateways/SourceSelection.test.tsx Tests selection, warnings, and source names.
src/i18n/locales/en-US/mcpServer.json Updates English status messages.
src/i18n/locales/en-US/gateways.json Adds English source and loading messages.
src/i18n/locales/es-ES/mcpServer.json Updates Spanish status messages.
src/i18n/locales/es-ES/gateways.json Adds Spanish source and loading messages.
src/i18n/locales/pt-BR/mcpServer.json Updates Portuguese status messages.
src/i18n/locales/pt-BR/gateways.json Adds Portuguese source and loading messages.
Review details
  • Files reviewed: 15/15 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/hooks/useOAuthTokenStatuses.ts
@a-effort
a-effort force-pushed the 6404-shared-server-status-badge branch from 45a7d1d to 9af64f3 Compare September 18, 2026 00:08
…r-status

# Conflicts:
#	src/pages/Servers.tsx
Load More issues a second lookup while the first may still be running,
and an earlier response carrying fewer ids would drop the rows it omits
back to reachability.

Signed-off-by: Anna Effort <anna.effort@ibm.com>
@a-effort
a-effort force-pushed the 6404-source-picker-status branch from 182a4aa to 730f2f2 Compare September 18, 2026 00:09
@a-effort a-effort self-assigned this Sep 18, 2026
@a-effort

a-effort commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

LLM review feedback:

Reviewed the diff and the Copilot thread.

The Copilot comment about the 100-ID cap is resolved correctly — the fix belongs in getOAuthStatus itself rather than in the hook, so callers pass the full list and batching is handled once.

Extracting useOAuthTokenStatuses was the right call. The versioned-request pattern needed to live in a hook rather than inlined in the page, and Servers.tsx is cleaner for it.

The idsKey join as a memo dependency avoids array reference churn without needing a deep-equality comparison.

SourceComponentsError wrapping the per-server fetch failure is a clean way to attribute errors without surfacing them as unattributed strings at the creation level.

Per-list independent errors with partial success (components still render when only some lists fail) is better than the previous single componentError. The retry wiring is straightforward.

The emptySelectedSources warning being non-blocking is correct per the stated intent.

The race condition test in useOAuthTokenStatuses.test.ts covers the stale-result scenario that motivated the versioning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants