feat: add MiniMax as a built-in onboarding provider (default M3)#270
feat: add MiniMax as a built-in onboarding provider (default M3)#270octo-patch wants to merge 3 commits into
Conversation
- Add 'minimax' to SUPPORTED_ONBOARDING_PROVIDERS and BUILTIN_PROVIDERS with wire: 'openai-chat', baseUrl: 'https://api.minimax.io/v1' - Add MiniMax-M2.7 and MiniMax-M2.7-highspeed to static model hint - Support MINIMAX_API_KEY env var via envKey field - Add MINIMAX_API_KEY to ALLOWED_IMPORT_ENV_KEYS allowlist - Add pingProvider support for minimax in validate.ts - Add unit tests for MiniMax provider config and validation - Add changeset for the new provider API reference: - Chat (OpenAI Compatible): https://platform.minimax.io/docs/api-reference/text-openai-api
There was a problem hiding this comment.
Review mode: initial
Findings
-
[Minor]
onboarding-ipc.tserror message missingollamafrom the list of supported providers — the message now reads "Only anthropic, openai, openrouter, minimax" but the corresponding message inpackages/providers/src/validate.tsincludesollamaas well. For consistency, the error message inonboarding-ipc.tsshould also mentionollama. This is a pre-existing inconsistency, but the PR touched that line.
Suggested fix:- `Provider "${provider}" is not supported in v0.1. Only anthropic, openai, openrouter, minimax.` + `Provider "${provider}" is not supported in v0.1. Only anthropic, openai, openrouter, ollama, minimax.`
-
[Nit] Test description change from "seeds three builtin providers" to "seeds all builtin providers" is accurate but the test body only checks that all entries exist, not the count. Consider also adding an assertion on the number of seeded providers to future-proof the test.
Suggested fix (optional):const providerCount = SUPPORTED_ONBOARDING_PROVIDERS.length; expect(Object.keys(v3.providers).length).toBe(providerCount);
Questions
- Does the frontend onboarding UI automatically pick up the new provider from
SUPPORTED_ONBOARDING_PROVIDERSandPROVIDER_SHORTLIST, or is there a hardcoded list in the renderer that should be updated? The diff only touches shared config and IPC logic; if there's a renderer-side static list, it would remain unaware of MiniMax.
Summary
The PR correctly adds MiniMax as a built-in onboarding provider with OpenAI-compatible wire, static model hints, environment variable support, and validation. The changes are focused, well-tested, and consistent with the project's architecture. No blockers or major issues found. See minor consistency improvement above.
Testing
- Unit tests added in
packages/providers/src/validate.test.tsandpackages/shared/src/config.test.tscover the new provider's configuration properties, validation, and error behavior. Tests are adequate. - No E2E tests impacted (no UI changes).
Open-CoDesign Bot
|
Thanks, this is close. The core provider wiring looks good, and I verified the focused tests locally. Before merging, could you please address a few small integration issues?
I verified locally: pnpm --filter @open-codesign/shared test && pnpm --filter @open-codesign/shared typecheck
pnpm --filter @open-codesign/providers test && pnpm --filter @open-codesign/providers typecheck
pnpm --filter @open-codesign/desktop test -- src/main/onboarding-ipc.test.ts src/main/provider-settings.test.ts src/main/connection-ipc.test.ts
pnpm --filter @open-codesign/desktop typecheck
pnpm exec biome check .changeset/add-minimax-provider.md apps/desktop/src/main/imports/codex-config.ts apps/desktop/src/main/onboarding-ipc.ts packages/providers/src/validate.ts packages/providers/src/validate.test.ts packages/shared/src/config.ts packages/shared/src/config.test.tsOnce those are addressed, this should be good to merge. |
|
Is this PR be merged with the latest release? If so when can that be expected. Thank you! |
- Add MiniMax-M3 to model list and set as default - Keep MiniMax-M2.7 and MiniMax-M2.7-highspeed - Update related tests and changeset
There was a problem hiding this comment.
Review mode: follow-up after new commits
Findings
-
[Minor] The
onboarding-ipc.tserror message still omitsollamafrom the list of supported providers, as noted in the previous review. The message reads "Only anthropic, openai, openrouter, minimax" but should also includeollamafor consistency withvalidate.ts. This was not addressed in the latest commits.
Suggested fix:- `Provider "${provider}" is not supported in v0.1. Only anthropic, openai, openrouter, minimax.` + `Provider "${provider}" is not supported in v0.1. Only anthropic, openai, openrouter, ollama, minimax.`
-
[Nit] The changeset and test descriptions now correctly reflect
MiniMax-M3as the default model, and model arrays consistently includeMiniMax-M3,MiniMax-M2.7, andMiniMax-M2.7-highspeed. No further inconsistencies.
Questions
- The previous review asked whether the frontend onboarding UI automatically picks up the new provider from
SUPPORTED_ONBOARDING_PROVIDERSandPROVIDER_SHORTLIST, or whether there is a hardcoded list in the renderer. This question remains unanswered — the diff only touches shared config and IPC logic. If a renderer-side static list exists, it would need updating. Please clarify.
Summary
The latest commit updates the default model from MiniMax-M2.7 to MiniMax-M3, matching the official model lineup. The rest of the implementation is consistent and well-tested. The only remaining open item is the onboarding-ipc.ts error message missing ollama, and the unanswered question about the renderer UI.
Testing
- The new tests (validation, config properties) have been updated to match the
MiniMax-M3default. All tests pass. No additional test gaps identified.
Open-CoDesign Bot
Per github-actions[bot] follow-up review on PR OpenCoworkAI#270, the error message in onboarding-ipc.ts omitted `ollama` from the list of supported providers while validate.ts already lists it. Align the two messages. Co-Authored-By: Octopus <liyuan851277048@icloud.com>
There was a problem hiding this comment.
Review mode: follow-up after new commits
Findings
- [Minor] The
onboarding-ipc.tserror message now correctly includesollamain the list of supported providers. This resolves the previous review's finding. No new issues introduced.
Questions
- The previous review asked whether the frontend onboarding UI automatically picks up the new provider from
SUPPORTED_ONBOARDING_PROVIDERSandPROVIDER_SHORTLIST, or whether there is a hardcoded list in the renderer. This question remains unanswered — the diff only touches shared config and IPC logic. If a renderer-side static list exists, it would need updating. Please clarify whether a renderer change is needed.
Summary
All previously identified issues have been addressed. The error message now lists all supported providers including ollama. The implementation is consistent and well-tested. The only open item is confirming the renderer UI behavior.
Testing
Not run (automation)
Open-CoDesign Bot
|
Thanks for the follow-up review! Pushed db8f2ce to include |
Summary
minimaxtoSUPPORTED_ONBOARDING_PROVIDERSandBUILTIN_PROVIDERSwithwire: 'openai-chat'andbaseUrl: 'https://api.minimax.io/v1'MiniMax-M3(default),MiniMax-M2.7, andMiniMax-M2.7-highspeedas static model hints (no/modelsendpoint call needed)MINIMAX_API_KEYenv var via theenvKeyfield — works for Codex import and env-fallback resolutionMINIMAX_API_KEYtoALLOWED_IMPORT_ENV_KEYSallowlist in codex-configpingProvidercase forminimaxin validate.ts (Bearer auth,https://api.minimax.io/v1/models)minorbump for@open-codesign/shared)Why
MiniMax offers an OpenAI-compatible API (
https://api.minimax.io/v1) with competitive models. Adding it as a built-in provider lets users select it directly from the onboarding screen without needing to configure a custom endpoint manually.MiniMax-M3is the latest model and is shipped as the default;MiniMax-M2.7andMiniMax-M2.7-highspeedremain available as alternatives for users who prefer the previous generation.API reference
Test plan
packages/sharedpass (pnpm --filter @open-codesign/shared test)packages/providerspass (pnpm --filter @open-codesign/providers test)apps/desktoponboarding-ipc passpackages/sharedandpackages/providers