Skip to content

feat: add API-key catalog registration flow - #89

Open
vishu-bh wants to merge 6 commits into
mainfrom
issue-5972-catalog-api-key-flow
Open

feat: add API-key catalog registration flow#89
vishu-bh wants to merge 6 commits into
mainfrom
issue-5972-catalog-api-key-flow

Conversation

@vishu-bh

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

Copy link
Copy Markdown
Contributor

What changed

  • Extends catalog cards and filters to support Open, API Key, and API authentication types.
  • Adds URL-backed authentication filtering with All/Select controls.
  • Adds shadcn API-key registration dialog with optional custom name, visibility, and team selection.
  • Sends typed registration body fields and updates the OpenAPI snapshot.
  • Keeps registration state authoritative on conflicts and restores focus to card actions after success.

Added

  • English, Spanish, and Portuguese strings for API-key registration and conflicts.
  • Vitest coverage for API-key/API cards, registration, validation, conflicts, and auth-type filters.

Verification

  • npm test
  • npm run build
  • npm run lint -- --max-warnings=0
  • npm run format:check
image

@vishu-bh
vishu-bh force-pushed the issue-5972-catalog-api-key-flow branch from a07bb1c to fd64dbc Compare August 28, 2026 10:27
@vishu-bh
vishu-bh force-pushed the issue-5972-catalog-api-key-flow branch from 00f59bc to d35135a Compare August 31, 2026 09:18

@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.

Not ready to approve — one CI blocker with a clear root cause, plus a couple of minor notes.**

🔴 Blocking: CI is red

Lint & Test Client fails on branch-coverage threshold

Branch coverage sits at 89.91%, just under the 90% global gate. I traced it to a specific, real gap rather than a borderline flake:

CatalogApiKeyDialog.tsx |   90 |   68.75 |  77.77 |  91.66 | 163-204

Lines 163-204 in src/components/server-catalog/CatalogApiKeyDialog.tsx are the visibility <Select>'s onValueChange handler, the team <SelectItem> branch, the conditional <TeamSelect> render (visibility === "team"), and the associated teamError validation path. None of that is exercised by the new tests in ServerCatalog.test.tsx.

The reason: useTeamScope is mocked globally at the top of the test file (useTeamScope: () => ({ teams: [], onTeamChange: vi.fn() })), and no test in the PR ever selects "team" visibility in the dialog. So the entire team-scoping path added by this feature — select "team" → TeamSelect renders → validation requires a team → team_id gets sent — is untested. Given this PR is specifically about adding visibility/team-scoping to catalog registration, that's a meaningful gap, not just a number to appease. Needs at least one test that opens the dialog, picks "team" visibility, exercises the "select a team" validation error, and one that submits with a team selected.

🟡 Minor: unused i18n key mcpServer.catalog.addConflict

Added to all three locale files (en-US/es-ES/pt-BR) but never referenced in ServerCatalog.tsx — the 409 conflict path still uses mcpServer.catalog.alreadyConnected. Either dead string to remove, or a sign the conflict-handling copy was meant to change and didn't. Worth a quick check with the author on intent; not blocking.

@a-effort a-effort 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.

Every registration failure reports behind the dialog's own overlay.

handleSubmit closes only on success (CatalogApiKeyDialog.tsx:90), so the dialog stays open for a retry. But all four failure paths in registerServer return false after calling showRegistrationNotification: success: false (ServerCatalog.tsx:458), 409 (:473), 404 (:486), generic catch (:503).

Those notifications render above the grid (:925), which sits behind the overlay (ui/dialog.tsx:22) and inside the subtree Radix passes to hideOthers on the modal path (@radix-ui/react-dialog@1.1.15, index.mjs:137).

Suggested fix: surface the failure inside DialogContent

}}
disabled={isSubmitting}
>
<SelectTrigger id="catalog-server-visibility">

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.

{/* SelectTrigger is w-fit by default; full width lines it up with the inputs above. */}

Image

vishu-bh and others added 5 commits September 1, 2026 13:35
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Signed-off-by: Marek Dano <mk.dano@gmail.com>
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Signed-off-by: Marek Dano <mk.dano@gmail.com>
Signed-off-by: Marek Dano <mk.dano@gmail.com>
Signed-off-by: Marek Dano <mk.dano@gmail.com>
Signed-off-by: Marek Dano <mk.dano@gmail.com>
@marekdano
marekdano force-pushed the issue-5972-catalog-api-key-flow branch from dca4d3e to 2eb63ae Compare September 1, 2026 12:35
Signed-off-by: Marek Dano <mk.dano@gmail.com>
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.

3 participants