Repository navigation
fix(mcp): align hub publication status and controls - #43241
Conversation
|
bugbot run Please review the current publication fix, especially derived visibility, empty selections, and persistence before runtime changes |
|
@greptileai Please review the current commit for publication consistency, explicit versus effective visibility, and failed-save behavior across supported configuration modes |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
85932d4 to
a161e62
Compare
|
@greptileai Please review a161e62. Missing explicit publication metadata now blocks editing, YAML generation, and saving, with live compatibility proof |
|
bugbot run Please review a161e62, including the unknown-metadata gate, catalog refresh behavior, and empty publication saves against supported proxies |
a161e62 to
999e9be
Compare
|
@greptileai Please review rebased head 999e9be against current main. The MCP patch is unchanged and upstream test assignments are fixed. |
|
bugbot run Please review rebased head 999e9be against current main and check for regressions in MCP publication behavior. |
|
@greptileai Please review origin/main...999e9be. Classifier and logging changes belong to merged main commits and are absent from this PR’s current diff |
999e9be to
c53859b
Compare
|
@greptileai Please review c53859b, focusing on effective network access labels, explicit publication overrides, unknown metadata handling, and removed redundant hub badges |
|
bugbot run Please review c53859b for regressions in network access labels, explicit hub overrides, unknown metadata, and card and detail consistency |
c53859b to
dbaf6ca
Compare
|
@greptileai Please review dbaf6ca. This follow-up shortens network badges to All Networks and Internal Only, without changing access behavior |
|
bugbot run Please review dbaf6ca for regressions after shortening the network badges and updating their existing tests and documentation |
|
@greptileai @cursor review Please review the current tip after resolving test conflicts with main and confirm prior findings remain resolved |
|
bugbot run |
GitHub Actions
Error
|
@greptileai Please review 6649ed1 after merging current main. Confirm prior findings remain resolved; the PR diff stays isolated |
|
@greptileai Please review f1914b3, including the behavior-preserving visibility guard extraction and publication behavior. Full UI checks pass in Devin |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f1914b3. Configure here.
| @@ -0,0 +1,37 @@ | |||
| # Publish MCP servers in the AI Hub | |||
There was a problem hiding this comment.
is this needed ?
There was a problem hiding this comment.
Removed the standalone README in #43394; the visibility dialog still provides YAML examples, canonical server ID guidance, and clearing controls
| @staticmethod | ||
| def _is_public_mcp_server(server: MCPServer, public_ids: Container[str]) -> bool: | ||
| return server.server_id in public_ids or ( | ||
| not litellm.public_mcp_hub_strict_whitelist and server.available_on_public_internet | ||
| ) | ||
|
|
||
| def is_mcp_server_public(self, server_id: str) -> bool: | ||
| server: Final = self.registry.get(server_id) or self.config_mcp_servers.get(server_id) | ||
| return server is not None and self._is_public_mcp_server(server, litellm.public_mcp_servers or ()) | ||
|
|
There was a problem hiding this comment.
these feel like they be one method, a little confusing naming here
There was a problem hiding this comment.
Consolidated the helpers into is_mcp_server_public in #43394, preserving batch lookups and strict/legacy behavior; the existing publication matrix passes
ryan-crabbe-berri
left a comment
ryan-crabbe-berri
left a comment
There was a problem hiding this comment.
LGTM after nits
This branch was successfully deployed
3 participants
TLDR
Problem this solves:
How it solves it:
Intentional product change: cards and details show All Networks, Internal Only, or Unknown. Hub publication controls stay in AI Hub and support clearing the list. Explicit hub publication still allows public-IP requests; authentication and access permissions retain their existing behavior
User Flow
Before: an admin sees conflicting publication information and cannot clear the hub
/ui/mcp-servers/: Public/Internal badges obscure effective network accessAfter: the admin sees network access separately from hub publication
/ui/mcp-servers/: All Networks/Internal Only labels reflect effective accessChanges
Caveats
Medium
Low
Validation status
Current tip
f1914b3b32includesmainat635a718ba1through an ordinary merge. Both PRs merge together cleanlyAll required CI and current test jobs pass. Greptile is 5/5; Veria and Cursor Bugbot are clean on this tip. No unresolved review threads remain
Independent Devin verification: 1,441 affected backend tests; 103, with 46 rerun after the UI guard extraction affected dashboard tests. Canonical backend lint/type gates, dashboard formatting/full lint/budgets, type tests, dead-code checks, API-type synchronization, and builds pass. No new or worsened UI warnings; the existing AVIF build warning is unchanged
Completed Codecov report: 28 uploads, 14/14 changed executable backend lines covered against the merge base. Repository-wide line coverage is 78.87%; dashboard coverage is measured separately in Vitest. All 40 changed executable dashboard lines are covered. Two defensive early-return arcs in the publication dialog cannot be reached through its disabled buttons; existing tests click those buttons and verify no API call. Two backend arcs at the pre-existing missing-server 404 remain uncovered in the unit report; the real API rejection was verified separately. No exclusions or lower thresholds were added. Live source verification passes; screenshot publication awaits deployment approval
Pre-Submission checklist
Screenshots / Proof of Fix
Independent Devin execution used source-built proxies at the commits below and isolated PostgreSQL databases. Register identical servers
hub_pubAandhub_pubB; start with strict publication enabled and onlyhub_pubApublished. SetBASE_URLto each proxy andLITELLM_MASTER_KEYto its test credentialBefore (635a718)
Publication metadata and network labels
/v1/mcp/serverand/v1/mcp/server/hub_pubA, then GET/public/mcp_hub: list/detail publication metadata disagree and explicit membership is unavailable/v1/mcp/make_public, then clear with{"mcp_server_ids":[]}: HTTP 202 updates the public feed, but detail metadata still omits publication state/ui/mcp-servers/: Public/Internal badges obscure the distinction between network access and hub membershipFailed persistence
hub_pubA, then install a PostgreSQL trigger rejecting configuration writes in the isolated test databasecurl -s -X POST "$BASE_URL/v1/mcp/make_public" -H "Authorization: Bearer $LITELLM_MASTER_KEY" -H 'Content-Type: application/json' -d '{"mcp_server_ids":["hub_pubA","hub_pubB"]}': HTTP 500 reports the rejected write/public/mcp_hub: both servers appear, despite the database still containing onlyhub_pubA. Retrying preserves the incorrect running statehub_pubAAfter (f1914b3)
Publication metadata and network labels
hub_pubAreportsis_public=trueandis_public_explicit=true;hub_pubBreports both false. The public feed contains onlyhub_pubA/ui/mcp-servers/: an explicitly published internal server shows All Networks, while an unpublished internal server shows Internal OnlyFailed persistence
hub_pubA, then install the same database write-rejection trigger/public/mcp_hub: onlyhub_pubAappears. List metadata and the database also retain onlyhub_pubA; a repeated failed request changes nothinghub_pubADashboard before/after captures exist for these source commits. Public screenshot upload awaits deployment approval; private verification URLs are intentionally omitted
Type
Bug Fix