feat: add API-key catalog registration flow - #89
Conversation
a07bb1c to
fd64dbc
Compare
00f59bc to
d35135a
Compare
marekdano
left a comment
There was a problem hiding this comment.
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
left a comment
There was a problem hiding this comment.
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"> |
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>
dca4d3e to
2eb63ae
Compare
Signed-off-by: Marek Dano <mk.dano@gmail.com>

What changed
Added
Verification