feat: add accessible MCP server catalog - #6169
Conversation
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
There was a problem hiding this comment.
Thanks, @vishu-bh!
Findings
1. Search + filter changes push a new history entry per keystroke — functionally-impacting (UX)
useCatalogFilters.updateQuery calls navigate(), and the router's navigate only ever does window.history.pushState (src/router/index.tsx:168 — no replace option). Because search is URL-backed and onSearchChange fires on every keystroke, typing "notes" pushes 5 history entries; each filter/tag toggle adds another. The browser Back button becomes "delete one character" instead of "leave the page." This also diverges from the codebase's established useLocalSearch pattern (local debounced state, not URL).
Options (roughly in order of preference):
- Keep the query in local state and only sync to the URL on a debounce (reuse
useDebouncedValue), or - Extend the router with a
replacemode for these in-place state updates.
Worth resolving since the a11y story is otherwise strong and this specifically hurts keyboard/AT users navigating with Back.
2. limit=1000 hard-caps the catalog silently — suggestion
CATALOG_PATH = "/v1/catalog?limit=1000" fetches the full list and filters client-side. If the catalog ever exceeds 1000 entries they vanish with no indication. Fine for MVP, but worth a // TODO: paginate and/or a tracking issue so it isn't forgotten.
3. logo_url scheme isn't constrained — minor (security)
<img src={server.logo_url}> renders whatever the catalog provides. no-referrer already blocks referrer leakage and <img> can't execute javascript:, so risk is low given the catalog is admin-curated — but a stray http: URL on an HTTPS page trips mixed-content, and an arbitrary URL is a tracking pixel. Consider a cheap startsWith("https://") / data: guard before rendering, else fall back to ServerIcon.
4. Test coverage gaps — suggestion
Core paths are covered well (open-only filtering, connected badge, details dialog + focus restore, view toggle, URL restore/update, category + OR-tag filters, 404-disabled/500-error, empty). Not exercised:
- The
CatalogLogoremote-image render and itsonErrorfallback (the one piece with real branching logic). - The provider and auth_type selects.
- The Clear-filters button.
- The
notConnecteddialog status.
The logo fallback in particular is untested behavior worth a case.
5. "Fail-closed" is presentation, not enforcement — minor
The auth_type !== "Open" filter comment reads as a security control, but the full catalog (including non-Open entries' URLs) is still shipped to the browser and filtered client-side. That's fine for public catalog metadata — just suggest wording it as a display filter so a future reader doesn't mistake it for an access boundary.
Suggested next steps
Finding 1 is the one I'd want addressed before merge; 2–5 are reasonable as follow-ups or quick touch-ups.
|
@vishu-bh - can you create this PR against our new repo https://github.com/contextforge-org/contextforge-web-ui |
|
Nice work so far! 🙌 Suggested adjustments to style/content, a bug fix, and added tests: #6178 (lmk if you want to merge that PR into this one or what's best) I also created a sub issue to move the catalog filters into a dialog, as it seems like the most manageable option, at least for now: #6179. Note: initially, when only non-auth MCP servers are offered, the authentication type selector is not needed. |
|
Move to new repo, closing this PR |
Pull Request
🔗 Related Issue
Closes #5970
📝 Summary
Adds the GET-only MCP server catalog experience for the UI rewrite:
/v1/catalogand exposes only exactauth_type === "Open"entrieslogo_urlwith the existing MCP icon fallbackRegistration remains intentionally out of scope until its API is available.
📏 Reviewability
triage🏷️ Type of Change
🧪 Verification
cd client && npm run lintcd client && npm run format:checkcd client && npx tsc -bcd client && npm run test:runcd client && npx vite build✅ Checklist
npm run format:check)📓 Notes (optional)
epic/ui-rewrite.logo_urlis used when supplied; the existing deterministic MCP icon is the fallback.