Skip to content

Add MiniMax copilot provider support#15484

Open
octo-patch wants to merge 2 commits into
alibaba:developfrom
octo-patch:octo/20260708-add-target-provider-model-to-existing-provider-registry-recvoLauGoi7eU
Open

Add MiniMax copilot provider support#15484
octo-patch wants to merge 2 commits into
alibaba:developfrom
octo-patch:octo/20260708-add-target-provider-model-to-existing-provider-registry-recvoLauGoi7eU

Conversation

@octo-patch

@octo-patch octo-patch commented Jul 9, 2026

Copy link
Copy Markdown

Reason: add target provider/model to existing provider registry

Summary

  • Introduce an extensible Copilot model-provider registry shared by runtime configuration and both console UIs.
  • Register MiniMax-M3 and MiniMax-M2.7 with OpenAI- and Anthropic-compatible endpoints for global and China regions.
  • Persist and return provider, protocol, region, model, and Base URL settings through the console configuration controller.
  • Add registry metadata, validation, request-capture tests, controller coverage, and integration-spec documentation.

Test plan

  • ./mvnw -q -pl copilot -am -DskipRat=true -Dcheckstyle.skip=true -Djacoco.skip=true -DskipITs -Dtest=CopilotModelProviderTest,CopilotPropertiesTest,CopilotAgentManagerTest -Dsurefire.failIfNoSpecifiedTests=false test
  • ./mvnw -q -pl console -am -DskipRat=true -Dcheckstyle.skip=true -Djacoco.skip=true -DskipITs -Dtest=ConsoleCopilotConfigControllerTest -Dsurefire.failIfNoSpecifiedTests=false test
  • ./mvnw -q -pl copilot,console -DskipTests checkstyle:check spotless:check
  • cd console-ui-next && ./node_modules/.bin/eslint src/pages/settingCenter/index.tsx && ./node_modules/.bin/tsc -b --pretty false && ./node_modules/.bin/vite build
  • cd console-ui && ./node_modules/.bin/eslint src/pages/SettingCenter/CopilotConfig.js
  • cd console-ui && NODE_OPTIONS=--openssl-legacy-provider NODE_ENV=production ./node_modules/.bin/webpack --config build/webpack.prod.conf.js

@KomachiSion

Copy link
Copy Markdown
Collaborator

Thanks for the PR.

I found one functional blocker: the new provider and baseUrl fields are added in both console UIs and CopilotAgentManager depends on provider to choose MiniMax, but ConsoleCopilotConfigController still keeps the old field whitelist. GET /v3/console/copilot/config only returns apiKey/model/studioUrl/studioProject, and POST only persists those same fields. As a result, selecting MiniMax in the UI will lose provider/baseUrl after save/reload, and runtime config will still fall back to DashScope.

Please update the config controller and related tests to fully read/write the new fields.

Another concern is the design direction. If Copilot is expected to support more models and providers, I think we should avoid extending this by only changing the frontend model list and adding more if/else provider checks in Java code. It may be better to introduce a clear ModelProvider abstraction or configurable provider registry, including provider metadata, model list, base URL/default options, stream/thinking capability, validation, and how the console obtains these options. That would make future provider support extensible instead of growing hard-coded branches.

Since this changes the Copilot configuration contract, please also update the related specs/docs. The current integration spec still only documents nacos.copilot.apiKey, model, studioUrl, and studioProject, but not provider or baseUrl.

Suggested follow-ups:

  • Persist and return provider/baseUrl in ConsoleCopilotConfigController.
  • Add tests for saving/loading MiniMax config and for model creation with both DashScope and MiniMax.
  • Update the Copilot integration spec/docs.
  • Consider a provider abstraction/design before adding more providers in this style.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

Thanks for your this PR. 🙏
Please check again for your PR changes whether contains any usage/api/configuration change such as Add new API , Add new configuration, Change default value of configuration.
If so, please add or update documents(markdown type) in docs/next/ for repository nacos-group/nacos-group.github.io


感谢您提交的PR。 🙏
请再次查看您的PR内容,确认是否包含任何使用方式/API/配置参数的变更,如:新增API新增配置参数修改默认配置等操作。
如果是,请确保在提交之前,在仓库nacos-group/nacos-group.github.io中的docs/next/目录下添加或更新文档(markdown格式)。

@octo-patch

Copy link
Copy Markdown
Author

Thanks for the review. I replaced the provider-specific branch with a registry, persisted and returned the provider, protocol, region, and Base URL fields, added metadata-driven options for both target models, and updated the integration spec. I also added controller and model-factory coverage with real request-capture tests, then ran the scoped Java tests, Checkstyle/Spotless, both UI linters, type checks, and production builds.

@KomachiSion

Copy link
Copy Markdown
Collaborator

Thanks for the update. The previous functional blocker around config persistence looks fixed now: the controller returns/persists provider, protocol, region, model, and baseUrl, and the new CopilotModelProvider registry also addresses the earlier concern about growing provider-specific if/else logic.

I still have two review concerns:

  1. Security risk around custom Base URL

    MiniMaxModelProvider currently accepts a user-configured baseUrl as long as it is an absolute http/https URL and the path ends with /v1 or /anthropic. The console UI also exposes Base URL as an editable field.

    Since runtime model creation sends the configured API key, including COPILOT_API_KEY, to that endpoint, a user with Copilot config write permission could point baseUrl to an attacker-controlled domain or an internal address. This may cause API key leakage and may also introduce SSRF/internal network probing risk.

    I think the default behavior should only allow built-in official endpoints from the provider registry. If custom endpoints are required, we need a clear security design first, such as an explicit enable switch, domain/IP allowlist, HTTPS-only enforcement, and blocking loopback/private/link-local/metadata addresses.

  2. Spec/doc update is incomplete

    The English integration spec has been updated, but the Chinese integration spec still documents only apiKey, model, studioUrl, and studioProject. It should also cover provider, protocol, region, baseUrl, and the provider metadata API so the two specs stay consistent.

Given the security implications, I suggest we do not keep iterating code directly in this PR for now. It may be better to open an issue first and discuss the provider/baseUrl extension design there, including the security boundaries and configuration contract. After the approach is agreed, the implementation can be updated in one direction instead of repeatedly rewriting the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants