mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-15 23:31:29 +00:00
feat(mcp): mark config servers read-only in UI and cover mcp_info oauth survival
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
811d794b84
commit
23dc79311c
8 changed files with 146 additions and 4 deletions
|
|
@ -51,6 +51,10 @@ class LiteLLM_MCPServerTable(LiteLLMPydanticObjectBase):
|
|||
server_name: Optional[str] = None
|
||||
alias: Optional[str] = None
|
||||
description: Optional[str] = None
|
||||
is_from_config: bool = Field(
|
||||
default=False,
|
||||
description="True if this server is defined in the config file, False if from DB. Config-defined servers cannot be edited via the UI.",
|
||||
)
|
||||
url: Optional[str] = None
|
||||
spec_path: Optional[str] = None
|
||||
transport: MCPTransportType
|
||||
|
|
|
|||
|
|
@ -5729,6 +5729,7 @@ class MCPServerManager:
|
|||
server_name=server.server_name,
|
||||
alias=server.alias,
|
||||
description=(server.mcp_info.get("description") if server.mcp_info else None),
|
||||
is_from_config=self.is_config_declared_server(server.server_id),
|
||||
url=server.url,
|
||||
transport=server.transport,
|
||||
auth_type=server.auth_type,
|
||||
|
|
@ -5836,6 +5837,7 @@ class MCPServerManager:
|
|||
server_name=server.server_name,
|
||||
alias=server.alias,
|
||||
description=(server.mcp_info.get("description") if server.mcp_info else None),
|
||||
is_from_config=self.is_config_declared_server(server.server_id),
|
||||
url=server.url,
|
||||
spec_path=server.spec_path,
|
||||
transport=server.transport,
|
||||
|
|
|
|||
|
|
@ -208,3 +208,50 @@ class TestMCPCustomFields:
|
|||
# Should use mcp_info description, not config level
|
||||
assert mcp_info["description"] == "MCP info description"
|
||||
assert mcp_info["custom_field"] == "custom_value"
|
||||
|
||||
|
||||
class TestMCPServerIsFromConfig:
|
||||
"""The management response must flag config-declared servers so the UI can keep them read-only."""
|
||||
|
||||
async def test_config_server_table_is_from_config_true(self):
|
||||
manager = MCPServerManager()
|
||||
|
||||
await manager.load_servers_from_config(
|
||||
{
|
||||
"config_server": {
|
||||
"url": "http://localhost:3000",
|
||||
"transport": "http",
|
||||
"mcp_info": {"owning_team": "platform"},
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
server = list(manager.config_mcp_servers.values())[0]
|
||||
table = manager._build_mcp_server_table(server)
|
||||
|
||||
assert table.is_from_config is True
|
||||
|
||||
async def test_database_server_table_is_from_config_false(self):
|
||||
manager = MCPServerManager()
|
||||
|
||||
db_server = LiteLLM_MCPServerTable(
|
||||
server_id="db-server-id",
|
||||
server_name="DB Server",
|
||||
alias=None,
|
||||
description="A database server",
|
||||
url="http://localhost:4000",
|
||||
transport="http",
|
||||
auth_type=MCPAuth.bearer_token,
|
||||
mcp_info={"server_name": "DB Server"},
|
||||
command=None,
|
||||
args=[],
|
||||
env={},
|
||||
mcp_access_groups=[],
|
||||
)
|
||||
await manager.add_server(db_server)
|
||||
|
||||
registry_server = manager.get_mcp_server_by_id("db-server-id")
|
||||
assert registry_server is not None
|
||||
table = manager._build_mcp_server_table(registry_server)
|
||||
|
||||
assert table.is_from_config is False
|
||||
|
|
|
|||
|
|
@ -1306,6 +1306,57 @@ describe("CreateMCPServer", () => {
|
|||
expect(payload.token_validation).toEqual({ organization: "my-org", "team.id": "42" });
|
||||
});
|
||||
|
||||
it("keeps typed mcp_info metadata through Authorize & Fetch and includes it in the submit payload", async () => {
|
||||
vi.mocked(networking.createMCPServer).mockResolvedValue({
|
||||
server_id: "new-server-oauth",
|
||||
server_name: "OAuth_Server",
|
||||
alias: "OAuth_Server",
|
||||
url: "https://example.com/mcp",
|
||||
transport: "http",
|
||||
auth_type: "oauth2",
|
||||
created_at: "2024-01-01T00:00:00Z",
|
||||
created_by: "user-1",
|
||||
updated_at: "2024-01-01T00:00:00Z",
|
||||
updated_by: "user-1",
|
||||
});
|
||||
|
||||
await setupOAuthInteractive();
|
||||
|
||||
const nameInput = document.getElementById("server_name") as HTMLInputElement;
|
||||
await act(async () => {
|
||||
fireEvent.change(nameInput, { target: { value: "OAuth_Server" } });
|
||||
});
|
||||
const urlInput = screen.getByPlaceholderText("https://your-mcp-server.com");
|
||||
await act(async () => {
|
||||
fireEvent.change(urlInput, { target: { value: "https://example.com/mcp" } });
|
||||
});
|
||||
|
||||
const metadataInput = document.getElementById("mcp_info_metadata_json") as HTMLTextAreaElement;
|
||||
await act(async () => {
|
||||
fireEvent.change(metadataInput, {
|
||||
target: { value: '{"owning_team": "platform", "cost_center": "1234"}' },
|
||||
});
|
||||
});
|
||||
|
||||
act(() => {
|
||||
oauthHook.onTokenReceived?.({ access_token: "tok-a" }, { clientId: "client-a", clientSecret: "secret-a" });
|
||||
});
|
||||
|
||||
const metadataAfterAuthorize = document.getElementById("mcp_info_metadata_json") as HTMLTextAreaElement;
|
||||
expect(metadataAfterAuthorize.value).toBe('{"owning_team": "platform", "cost_center": "1234"}');
|
||||
|
||||
const submitButton = screen.getByRole("button", { name: "Add MCP Server" });
|
||||
await act(async () => {
|
||||
fireEvent.click(submitButton);
|
||||
});
|
||||
|
||||
await waitFor(() => expect(networking.createMCPServer).toHaveBeenCalledTimes(1));
|
||||
const [, payload] = vi.mocked(networking.createMCPServer).mock.calls[0];
|
||||
expect(payload.mcp_info.owning_team).toBe("platform");
|
||||
expect(payload.mcp_info.cost_center).toBe("1234");
|
||||
expect(payload.mcp_info.server_name).toBe("OAuth_Server");
|
||||
});
|
||||
|
||||
it("invalidates the DCR client and OAuth flow when the MCP URL changes after Authorize & Fetch", async () => {
|
||||
await setupOAuthInteractive();
|
||||
|
||||
|
|
|
|||
|
|
@ -119,6 +119,34 @@ describe("MCPServerView", () => {
|
|||
expect(screen.queryByRole("button", { name: "Edit Settings" })).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("keeps a config-defined server read-only: no Edit Settings, shows a config note", async () => {
|
||||
renderView({ is_from_config: true });
|
||||
|
||||
await userEvent.click(screen.getByRole("tab", { name: "Settings" }));
|
||||
|
||||
expect(await screen.findByText("MCP Server Settings")).toBeInTheDocument();
|
||||
expect(screen.queryByRole("button", { name: "Edit Settings" })).not.toBeInTheDocument();
|
||||
expect(screen.getByText("Defined in config.yaml (read-only)")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("never opens the edit form for a config-defined server even when isEditing is set", async () => {
|
||||
renderView({ is_from_config: true }, { isEditing: true });
|
||||
|
||||
await userEvent.click(screen.getByRole("tab", { name: "Settings" }));
|
||||
|
||||
expect(await screen.findByText("MCP Server Settings")).toBeInTheDocument();
|
||||
expect(screen.queryByText("edit form")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("keeps a database-defined server editable: Edit Settings is offered", async () => {
|
||||
renderView({ is_from_config: false });
|
||||
|
||||
await userEvent.click(screen.getByRole("tab", { name: "Settings" }));
|
||||
|
||||
expect(await screen.findByRole("button", { name: "Edit Settings" })).toBeInTheDocument();
|
||||
expect(screen.queryByText("Defined in config.yaml (read-only)")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("opens on the tab named by initialTabIndex", async () => {
|
||||
renderView({}, { initialTabIndex: 1 });
|
||||
|
||||
|
|
|
|||
|
|
@ -59,7 +59,8 @@ export const MCPServerView: React.FC<MCPServerViewProps> = ({
|
|||
// Open the editing Settings tab on first render when returning from the edit OAuth
|
||||
// redirect, so the "token fetched" feedback shows where the user left off (Settings=2).
|
||||
const returningFromEditOAuth = isReturningFromEditOAuth(isProxyAdmin, mcpServer.server_id);
|
||||
const [editing, setEditing] = useState(isEditing || returningFromEditOAuth);
|
||||
const canEdit = isProxyAdmin && !mcpServer.is_from_config;
|
||||
const [editing, setEditing] = useState((isEditing || returningFromEditOAuth) && canEdit);
|
||||
const [showFullUrl, setShowFullUrl] = useState(false);
|
||||
const [copiedStates, setCopiedStates] = useState<Record<string, boolean>>({});
|
||||
const [selectedTabIndex, setSelectedTabIndex] = useState(returningFromEditOAuth ? 2 : initialTabIndex);
|
||||
|
|
@ -216,13 +217,16 @@ export const MCPServerView: React.FC<MCPServerViewProps> = ({
|
|||
<Card className="p-6">
|
||||
<div className="mb-4 flex items-center justify-between">
|
||||
<h2 className="text-lg font-medium">MCP Server Settings</h2>
|
||||
{editing ? null : (
|
||||
{!editing && canEdit && (
|
||||
<Button variant="outline" onClick={() => setEditing(true)}>
|
||||
Edit Settings
|
||||
</Button>
|
||||
)}
|
||||
{!editing && mcpServer.is_from_config && (
|
||||
<span className="text-sm text-muted-foreground">Defined in config.yaml (read-only)</span>
|
||||
)}
|
||||
</div>
|
||||
{editing ? (
|
||||
{editing && canEdit ? (
|
||||
<MCPServerEdit
|
||||
mcpServer={mcpServer}
|
||||
accessToken={accessToken}
|
||||
|
|
|
|||
|
|
@ -696,7 +696,11 @@ const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID })
|
|||
}
|
||||
onByokConnect={server.is_byok ? () => setByokModalServer(server) : undefined}
|
||||
onOpenFillFields={() => setEnvVarsModalServer(server)}
|
||||
onDelete={isAdminRole(userRole) ? () => handleDelete(server.server_id) : undefined}
|
||||
onDelete={
|
||||
isAdminRole(userRole) && !server.is_from_config
|
||||
? () => handleDelete(server.server_id)
|
||||
: undefined
|
||||
}
|
||||
/>
|
||||
))}
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -416,6 +416,8 @@ export interface MCPServer {
|
|||
/** GitHub / source repository URL */
|
||||
source_url?: string | null;
|
||||
|
||||
is_from_config?: boolean | null;
|
||||
|
||||
/** BYOM (Bring Your Own MCP) submission fields */
|
||||
approval_status?: "active" | "pending_review" | "rejected" | null;
|
||||
submitted_by?: string | null;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue