feat(core): add support for admin-forced MCP server installations#23163
feat(core): add support for admin-forced MCP server installations#23163gsquared94 merged 1 commit intogoogle-gemini:mainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant feature that empowers enterprise administrators to enforce specific MCP server configurations across their organization. By allowing administrators to define 'required' servers, the system ensures that these critical services are always available to users, overriding any conflicting local settings and preventing user-side disabling. This enhancement provides a robust mechanism for centralized control over the MCP server landscape, improving compliance and consistency within managed environments. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
The pull request introduces support for admin-forced MCP server installations, allowing enterprise administrators to inject specific MCP server configurations that override local user settings. The changes are well-tested with new unit tests covering various scenarios, including injection, overrides, and tool filtering. The implementation correctly handles the precedence of admin-required servers and ensures proper logging of injected servers. The use of z.nativeEnum(AuthProviderType) in packages/core/src/code_assist/types.ts aligns with the stated design decision for a single source of truth for enum values.
Note: Security Review did not run due to the size of the PR.
e373d08 to
60ea5f5
Compare
60ea5f5 to
3f73d92
Compare
Add the ability for enterprise administrators to force specific MCP server installations through admin controls. These required servers are always injected regardless of user-local configurations and cannot be disabled. Changes: - Add RequiredMcpServerConfigSchema and RequiredMcpServerConfig type - Parse requiredMcpServers from mcpConfigJson in sanitizeAdminSettings - Add applyRequiredServers() to inject admin-required MCP servers - Wire applyRequiredServers into CLI config loading pipeline - Add requiredConfig to admin MCP settings schema - Map requiredMcpConfig through setRemoteAdminSettings - Add RequiredMcpServerConfig JSON schema definition - Add comprehensive tests for all new code paths
3f73d92 to
140fc84
Compare
adamfweidman
left a comment
There was a problem hiding this comment.
LGTM w a few questions
8615315
Summary
Adds the ability for enterprise administrators to force specific MCP server installations through admin controls. Required servers are always injected regardless of user-local configurations and cannot be disabled by the user.
Changes
Core (
packages/core)types.ts: AddedRequiredMcpServerConfigSchema(Zod schema for admin-required servers with remote transport fields only) and updatedMcpConfigDefinitionSchema/AdminControlsSettingsSchemato includerequiredMcpServers/requiredMcpConfig.admin_controls.ts: UpdatedsanitizeAdminSettings()to parserequiredMcpServersfrommcpConfigJsonand sort tool lists for stable deep-equality comparison.mcpUtils.ts: AddedapplyRequiredServers()function that convertsRequiredMcpServerConfig→MCPServerConfig, defaultingtrusttotrueand completely overriding any local config with the same server name.CLI (
packages/cli)config.ts: CallsapplyRequiredServers()afterapplyAdminAllowlist()to inject admin-required servers.settings.ts: MapsrequiredMcpConfigfrom admin settings tosettings.admin.mcp.requiredConfig.settingsSchema.ts: AddedrequiredConfigfield andRequiredMcpServerConfigJSON schema definition.Design Decisions
requiredMcpServers(distinct from existingmcpServersallowlist) for backward compatibility — older clients ignore the new field.command,args,env,cwd) since admin-pushed servers are always remote.z.nativeEnum(AuthProviderType)instead of duplicating enum values.Tests Added
mcpUtils.test.tscovering injection, overrides, auth preservation, tool filtering, trust defaults, and coexistence with allowlisted servers.admin_controls.test.tsfor requiredMcpServers parsing, tool list sorting, and required-only configs.settings.test.tsfor requiredMcpConfig mapping through setRemoteAdminSettings.list.test.tstest fixture.Verification
Fixes https://github.com/google-gemini/maintainers-gemini-cli/issues/1586