feat: add MiniMax provider support#1277
Conversation
- Add MiniMax chat model provider using OpenAI-compatible API - Support MiniMax-M2.7 (default) and MiniMax-M2.7-highspeed models - Add MINIMAX_API_KEY environment variable support - Add unit tests for provider, config, and capabilities - Register MiniMax as a built-in provider in the registry - Export MiniMax provider and config from @archon/providers
📝 WalkthroughWalkthroughThis pull request introduces a new MiniMax provider to the providers package. It includes the complete provider implementation with configuration parsing, capability definitions, and comprehensive tests, along with integration into the package's public exports and built-in registry. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client / Archon
participant Provider as MiniMaxProvider
participant API as MiniMax API
participant Stream as SSE Stream
Client->>Provider: sendQuery(prompt, options)
Provider->>Provider: Parse config & resolve baseURL, model
Provider->>Provider: Build request body with streaming enabled
Provider->>API: POST /chat/completions (stream: true)
API-->>Stream: Return streaming response
Provider->>Stream: Read response.body (SSE lines)
loop Parse SSE chunks
Stream-->>Provider: "data: {delta: {content: '...'}}"
Provider->>Provider: Parse JSON, extract delta
Provider-->>Client: Yield MessageChunk with token delta
end
Stream-->>Provider: "data: {result: {usage: {...}}}"
Provider->>Provider: Accumulate token usage
Provider-->>Client: Yield result chunk with token counts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
packages/providers/src/minimax/config.ts (1)
18-30: Consider validatingbaseURLshape.
baseURLis accepted as-is as long as it's a string. A malformed value (e.g., missing scheme, or a path without host) will only surface later as an opaquefetchfailure. A minimalnew URL(raw.baseURL)check with an early throw (per guideline "Prefer throwing early with a clear error for unsupported or unsafe states") would yield a much clearer error at config-load time. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/providers/src/minimax/config.ts` around lines 18 - 30, parseMiniMaxConfig currently accepts any string for the baseURL which can lead to opaque fetch failures later; update parseMiniMaxConfig to validate raw.baseURL by attempting to construct a new URL(raw.baseURL) and if that throws, rethrow a clear, early error (e.g., "Invalid MiniMax baseURL: <value>") so callers fail fast and get a helpful message; locate the logic in function parseMiniMaxConfig and perform the new URL check before assigning result.baseURL.packages/providers/package.json (1)
21-21: Test script is getting unwieldy; consider a glob.Chaining seven
bun testinvocations with&&is brittle (one added test file requires editing this line). Considerbun testwith a directory, or moving to a test runner config. Not blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/providers/package.json` at line 21, The package.json "test" script is brittle because it chains many explicit bun test calls; replace the hardcoded chain (the "test" script value) with a single glob or directory invocation (e.g. run bun test against src or a pattern like src/**/*.test.ts) so new tests run automatically without editing the script; update the "test" script entry to use that glob/directory form instead of the long && chain.packages/providers/src/registry.ts (1)
106-108: Update the docstring.The comment still says "Register built-in providers (Claude, Codex)." — add MiniMax.
- * Register built-in providers (Claude, Codex). Idempotent — skips already-registered IDs. + * Register built-in providers (Claude, Codex, MiniMax). Idempotent — skips already-registered IDs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/providers/src/registry.ts` around lines 106 - 108, Update the top-of-file/doc comment for the built-in provider registration to mention MiniMax in addition to Claude and Codex; locate the documentation block above the registerBuiltInProviders function (or the Register built-in providers comment in registry.ts) and change the text "Register built-in providers (Claude, Codex)." to include "MiniMax" so it reads something like "Register built-in providers (Claude, Codex, MiniMax)."packages/docs-web/src/content/docs/guides/skills.md (1)
171-171: Doc entry looks fine.Minor: the surrounding guide is explicitly scoped to Claude (line 16 states "Claude only — Codex nodes will warn and ignore the
skillsfield"). Since MiniMax capabilities setskills: false, this new row fits the "Popular Skills" list (skill installation is provider-agnostic on disk) but readers may expect the skill to be usable on MiniMax nodes. Consider a brief note thatmmx-clionly takes effect on Claude nodes, or place MiniMax-related content in a MiniMax-specific guide.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/docs-web/src/content/docs/guides/skills.md` at line 171, The "Popular Skills" row for `mmx-cli` / `MiniMax-AI/cli/skill` could mislead readers because the surrounding guide is Claude-only and MiniMax nodes set `skills: false`; add a brief clarifying sentence near the table (or immediately after the `mmx-cli` row) stating that `mmx-cli`/`MiniMax-AI/cli/skill` installation is provider-agnostic on disk but the `skills` field is disabled for MiniMax (i.e., `skills: false`), so the skill only takes effect on Claude nodes — alternatively move this MiniMax-specific entry into a MiniMax-specific guide to avoid confusion.packages/providers/src/registry.test.ts (1)
271-277: Consider covering the codex/minimax overlap.Codex's
isModelCompatiblereturnstruefor any model that is not a Claude alias /claude-*/inherit, which includesMiniMax-M2.7. A test likeexpect(getRegistration('codex').isModelCompatible('MiniMax-M2.7')).toBe(false)would currently fail and surface the ambiguity flagged onregistry.ts. Worth adding once that is resolved.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/providers/src/registry.test.ts` around lines 271 - 277, The test suite misses the Codex vs MiniMax ambiguity: add a test asserting getRegistration('codex').isModelCompatible('MiniMax-M2.7') returns false, and fix registry logic so Codex doesn't claim MiniMax-style names; either make Codex's isModelCompatible explicitly exclude MiniMax patterns (e.g. names matching /^MiniMax-/) or change the registration matching precedence in registry.ts to check MiniMax (getRegistration('minimax') / its isModelCompatible) before Codex (getRegistration('codex')) so MiniMax names are claimed by MiniMax; update tests accordingly to include expect(getRegistration('codex').isModelCompatible('MiniMax-M2.7')).toBe(false).packages/providers/src/minimax/provider.test.ts (1)
130-130: RestoreglobalThis.fetchbetween tests.
globalThis.fetchis overwritten in everysendQuerytest but never restored. Once this suite runs, the overridden fetch leaks into any subsequent test in the same bun process and into teardown code, which can produce cross-suite pollution and hard-to-diagnose failures.Capture the original once and restore it in
afterEach/afterAll:♻️ Suggested refactor
-import { describe, test, expect, mock, beforeEach } from 'bun:test'; +import { describe, test, expect, mock, beforeEach, afterEach } from 'bun:test'; ... describe('MiniMaxProvider', () => { let provider: MiniMaxProvider; + const originalFetch = globalThis.fetch; + + afterEach(() => { + globalThis.fetch = originalFetch; + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/providers/src/minimax/provider.test.ts` at line 130, The tests overwrite globalThis.fetch with mockFetch in the sendQuery suite but never restore it; capture the original fetch before overriding (e.g., const originalFetch = globalThis.fetch) and restore it in an afterEach or afterAll hook to avoid leaking the mock into other suites—update the packages/providers/src/minimax/provider.test.ts test file to save the original globalThis.fetch before assigning mockFetch and call globalThis.fetch = originalFetch in a teardown hook after each test (or after all) so sendQuery tests clean up their mock.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/providers/src/index.ts`:
- Line 43: The package currently re-exports parseMiniMaxConfig and redeclares
MiniMaxProviderDefaults from ./minimax/config, creating a duplicate type; change
the export so MiniMaxProviderDefaults is re-exported from the single source of
truth (the providers' types module where MiniMaxProviderDefaults is declared)
instead of from minimax/config, and remove the redundant interface declaration
in minimax/config so parseMiniMaxConfig remains exported but the type is
imported/re-exported from the canonical types declaration (ensure the symbol
MiniMaxProviderDefaults is imported from the types module and re-exported
alongside parseMiniMaxConfig).
In `@packages/providers/src/minimax/config.ts`:
- Around line 5-12: Remove the local duplicate interface declaration for
MiniMaxProviderDefaults and instead import the canonical type from ../types
using a type-only import; then re-export it as MiniMaxConfig (e.g., import type
{ MiniMaxProviderDefaults } from '../types' and export type {
MiniMaxProviderDefaults as MiniMaxConfig }). Update the file to reference only
the imported MiniMaxProviderDefaults symbol (no local interface) and ensure you
use import type for the type-only import per guidelines.
In `@packages/providers/src/minimax/provider.test.ts`:
- Around line 5-7: The test currently calls mock.module('@archon/paths', ...)
which globally mutates the module registry; instead import the module locally
(e.g. import * as paths from '@archon/paths') and replace mock.module(...) with
spyOn(paths, 'createLogger').mockReturnValue(mockLogger), and ensure you restore
the spy in afterEach via (paths.createLogger as any).mockRestore() or
spy.mockRestore(); update any test setup referencing mock.module to use spyOn
and mockLogger so the mock is file-scoped and does not pollute other tests.
In `@packages/providers/src/minimax/provider.ts`:
- Around line 41-50: classifyError currently uses loose substring matching
(RATE_LIMIT_PATTERNS and AUTH_PATTERNS) that misclassifies errors; update
classifyError to (1) first try to parse the incoming message as JSON and inspect
structured fields like error.code, error.status, or statusCode to decide
'rate_limit' vs 'auth', and only fall back to string matching if parsing fails,
(2) tighten string matches by using whole-word/regex boundaries and replace
generic tokens like 'invalid' with specific phrases such as 'invalid api key' or
'invalid token', and (3) remove or regex-anchor raw numeric tokens (e.g.,
'1002','1039','1004') so they match only if they appear as standalone error
codes. Ensure these changes are applied inside the classifyError function and
reference RATE_LIMIT_PATTERNS and AUTH_PATTERNS for pattern updates.
- Line 264: The structured log events in provider.ts violate the
`{domain}.{action}_{state}` convention: rename `minimax.query_started`
(getLog().debug call) to `minimax.query_started`, then emit
`minimax.query_completed` after the successful streaming finish (after the
`yield*`/stream end in the query flow) and emit `minimax.query_failed` on the
terminal throw path (the throw around line 335); also rename other events so
they use `_failed` or explicit states instead of `_error`/bare actions —
specifically update `minimax.fetch_error` -> `minimax.fetch_failed`,
`minimax.http_error` -> `minimax.http_failed`, `minimax.stream_error` ->
`minimax.stream_failed`, `minimax.sse_parse_error` ->
`minimax.sse_parse_failed`, `minimax.empty_response` ->
`minimax.empty_response_failed` (or another explicit failed state), and
`minimax.retrying` -> `minimax.retrying_started`/`minimax.retrying_completed` as
appropriate; locate and change the getLog() calls in the streaming/query
functions (the getLog().debug/getLog().warn/getLog().error invocations) so each
`_started` has a paired `_completed` or `_failed` and keep message context
(model, attempt, baseURL).
- Around line 321-333: The current retry loop can re-deliver partial streamed
assistant chunks because yield* streamMiniMaxResponse(...) may emit data before
a stream error; change the retry logic to detect if any chunks were emitted and,
if so, do not retry—immediately rethrow the stream error instead of attempting a
new request. Concretely, introduce a local boolean (e.g., emittedAnyChunk) that
is set by a small wrapper/iterator around streamMiniMaxResponse when the first
chunk is yielded, then in the catch block check that flag: if emittedAnyChunk is
true, rethrow streamErr; otherwise continue with the existing retry/backoff
logic (use the existing variables attempt, MAX_RETRIES, retryBaseDelayMs,
lastError).
In `@packages/providers/src/registry.ts`:
- Around line 135-142: Codex's isModelCompatible currently claims any
non-Claude/non-'inherit' model which causes MiniMax-* names to match; update the
Codex provider's isModelCompatible predicate to explicitly exclude MiniMax
prefixes (e.g. return false if model.startsWith('MiniMax-')) or better replace
the predicate with an allow-list of recognized OpenAI prefixes (e.g. check
model.startsWith one of ['gpt-', 'text-davinci-', ...]) so MiniMaxProvider
(factory: () => new MiniMaxProvider(), isModelCompatible: (model) =>
model.startsWith('MiniMax-')) will be selected by inferProviderFromModel;
add/adjust a unit test for inferProviderFromModel to assert 'MiniMax-M2.7'
resolves to the 'minimax' provider.
---
Nitpick comments:
In `@packages/docs-web/src/content/docs/guides/skills.md`:
- Line 171: The "Popular Skills" row for `mmx-cli` / `MiniMax-AI/cli/skill`
could mislead readers because the surrounding guide is Claude-only and MiniMax
nodes set `skills: false`; add a brief clarifying sentence near the table (or
immediately after the `mmx-cli` row) stating that
`mmx-cli`/`MiniMax-AI/cli/skill` installation is provider-agnostic on disk but
the `skills` field is disabled for MiniMax (i.e., `skills: false`), so the skill
only takes effect on Claude nodes — alternatively move this MiniMax-specific
entry into a MiniMax-specific guide to avoid confusion.
In `@packages/providers/package.json`:
- Line 21: The package.json "test" script is brittle because it chains many
explicit bun test calls; replace the hardcoded chain (the "test" script value)
with a single glob or directory invocation (e.g. run bun test against src or a
pattern like src/**/*.test.ts) so new tests run automatically without editing
the script; update the "test" script entry to use that glob/directory form
instead of the long && chain.
In `@packages/providers/src/minimax/config.ts`:
- Around line 18-30: parseMiniMaxConfig currently accepts any string for the
baseURL which can lead to opaque fetch failures later; update parseMiniMaxConfig
to validate raw.baseURL by attempting to construct a new URL(raw.baseURL) and if
that throws, rethrow a clear, early error (e.g., "Invalid MiniMax baseURL:
<value>") so callers fail fast and get a helpful message; locate the logic in
function parseMiniMaxConfig and perform the new URL check before assigning
result.baseURL.
In `@packages/providers/src/minimax/provider.test.ts`:
- Line 130: The tests overwrite globalThis.fetch with mockFetch in the sendQuery
suite but never restore it; capture the original fetch before overriding (e.g.,
const originalFetch = globalThis.fetch) and restore it in an afterEach or
afterAll hook to avoid leaking the mock into other suites—update the
packages/providers/src/minimax/provider.test.ts test file to save the original
globalThis.fetch before assigning mockFetch and call globalThis.fetch =
originalFetch in a teardown hook after each test (or after all) so sendQuery
tests clean up their mock.
In `@packages/providers/src/registry.test.ts`:
- Around line 271-277: The test suite misses the Codex vs MiniMax ambiguity: add
a test asserting getRegistration('codex').isModelCompatible('MiniMax-M2.7')
returns false, and fix registry logic so Codex doesn't claim MiniMax-style
names; either make Codex's isModelCompatible explicitly exclude MiniMax patterns
(e.g. names matching /^MiniMax-/) or change the registration matching precedence
in registry.ts to check MiniMax (getRegistration('minimax') / its
isModelCompatible) before Codex (getRegistration('codex')) so MiniMax names are
claimed by MiniMax; update tests accordingly to include
expect(getRegistration('codex').isModelCompatible('MiniMax-M2.7')).toBe(false).
In `@packages/providers/src/registry.ts`:
- Around line 106-108: Update the top-of-file/doc comment for the built-in
provider registration to mention MiniMax in addition to Claude and Codex; locate
the documentation block above the registerBuiltInProviders function (or the
Register built-in providers comment in registry.ts) and change the text
"Register built-in providers (Claude, Codex)." to include "MiniMax" so it reads
something like "Register built-in providers (Claude, Codex, MiniMax)."
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d42feb65-0d04-4f52-81eb-0f9a83527c12
📒 Files selected for processing (10)
packages/docs-web/src/content/docs/guides/skills.mdpackages/providers/package.jsonpackages/providers/src/index.tspackages/providers/src/minimax/capabilities.tspackages/providers/src/minimax/config.tspackages/providers/src/minimax/provider.test.tspackages/providers/src/minimax/provider.tspackages/providers/src/registry.test.tspackages/providers/src/registry.tspackages/providers/src/types.ts
| // Config parsers | ||
| export { parseClaudeConfig, type ClaudeProviderDefaults } from './claude/config'; | ||
| export { parseCodexConfig, type CodexProviderDefaults } from './codex/config'; | ||
| export { parseMiniMaxConfig, type MiniMaxProviderDefaults } from './minimax/config'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Re-export source is inconsistent with the canonical type location.
MiniMaxProviderDefaults is re-exported here from ./minimax/config, but packages/providers/src/types.ts declares itself the "Single source of truth for provider-specific config shapes" and defines MiniMaxProviderDefaults at lines 30-35. The config module currently redeclares the interface rather than re-exporting from ./types, producing two structurally-identical types that can silently drift. See the comment on minimax/config.ts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/providers/src/index.ts` at line 43, The package currently re-exports
parseMiniMaxConfig and redeclares MiniMaxProviderDefaults from ./minimax/config,
creating a duplicate type; change the export so MiniMaxProviderDefaults is
re-exported from the single source of truth (the providers' types module where
MiniMaxProviderDefaults is declared) instead of from minimax/config, and remove
the redundant interface declaration in minimax/config so parseMiniMaxConfig
remains exported but the type is imported/re-exported from the canonical types
declaration (ensure the symbol MiniMaxProviderDefaults is imported from the
types module and re-exported alongside parseMiniMaxConfig).
| export interface MiniMaxProviderDefaults { | ||
| [key: string]: unknown; | ||
| model?: string; | ||
| baseURL?: string; | ||
| } | ||
|
|
||
| // Re-export so consumers can import the type from either location | ||
| export type { MiniMaxProviderDefaults as MiniMaxConfig }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Duplicate interface declaration — re-export from ../types instead.
MiniMaxProviderDefaults is already the canonical definition in packages/providers/src/types.ts (lines 30-35), which that file explicitly identifies as the "Single source of truth for provider-specific config shapes." Redeclaring the same interface here creates two nominally distinct types that can drift (a field added in one location will not appear in the other, and @archon/core/config/config-types.ts — per the comment in types.ts — imports from ./types, not from here).
Align with the Claude/Codex pattern by re-exporting from ../types:
♻️ Proposed fix
-export interface MiniMaxProviderDefaults {
- [key: string]: unknown;
- model?: string;
- baseURL?: string;
-}
-
-// Re-export so consumers can import the type from either location
-export type { MiniMaxProviderDefaults as MiniMaxConfig };
+export type { MiniMaxProviderDefaults } from '../types';
+export type { MiniMaxProviderDefaults as MiniMaxConfig } from '../types';As per coding guidelines: "Always use import type for type-only imports and specific named imports for values".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface MiniMaxProviderDefaults { | |
| [key: string]: unknown; | |
| model?: string; | |
| baseURL?: string; | |
| } | |
| // Re-export so consumers can import the type from either location | |
| export type { MiniMaxProviderDefaults as MiniMaxConfig }; | |
| export type { MiniMaxProviderDefaults } from '../types'; | |
| export type { MiniMaxProviderDefaults as MiniMaxConfig } from '../types'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/providers/src/minimax/config.ts` around lines 5 - 12, Remove the
local duplicate interface declaration for MiniMaxProviderDefaults and instead
import the canonical type from ../types using a type-only import; then re-export
it as MiniMaxConfig (e.g., import type { MiniMaxProviderDefaults } from
'../types' and export type { MiniMaxProviderDefaults as MiniMaxConfig }). Update
the file to reference only the imported MiniMaxProviderDefaults symbol (no local
interface) and ensure you use import type for the type-only import per
guidelines.
| mock.module('@archon/paths', () => ({ | ||
| createLogger: mock(() => mockLogger), | ||
| })); |
There was a problem hiding this comment.
Avoid mock.module('@archon/paths', ...); use spyOn(paths, 'createLogger') instead.
@archon/paths is a shared module likely mocked by other test files in this package with different shapes. mock.module() mutates the global module registry for the entire bun test process, so whichever test file runs first wins and the rest silently get the wrong logger — leading to flaky, order-dependent failures.
Switch to spyOn() (which restores cleanly via mockRestore()), or isolate this file in a separate bun test invocation in package.json.
♻️ Suggested refactor
-import { describe, test, expect, mock, beforeEach } from 'bun:test';
+import { describe, test, expect, mock, spyOn, beforeEach, afterAll } from 'bun:test';
+import * as paths from '@archon/paths';
import { createMockLogger } from '../test/mocks/logger';
const mockLogger = createMockLogger();
-mock.module('@archon/paths', () => ({
- createLogger: mock(() => mockLogger),
-}));
+const createLoggerSpy = spyOn(paths, 'createLogger').mockReturnValue(mockLogger);
+afterAll(() => createLoggerSpy.mockRestore());As per coding guidelines: "Never use mock.module() for modules that other test files also mock with different implementations; use spyOn() instead ... use separate bun test invocations in package.json for conflicting mocks to prevent process-global pollution".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/providers/src/minimax/provider.test.ts` around lines 5 - 7, The test
currently calls mock.module('@archon/paths', ...) which globally mutates the
module registry; instead import the module locally (e.g. import * as paths from
'@archon/paths') and replace mock.module(...) with spyOn(paths,
'createLogger').mockReturnValue(mockLogger), and ensure you restore the spy in
afterEach via (paths.createLogger as any).mockRestore() or spy.mockRestore();
update any test setup referencing mock.module to use spyOn and mockLogger so the
mock is file-scoped and does not pollute other tests.
| const RATE_LIMIT_PATTERNS = ['rate limit', 'too many requests', '429', '1002', '1039']; | ||
| const AUTH_PATTERNS = ['unauthorized', 'authentication', 'invalid', '401', '403', '1004']; | ||
|
|
||
| /** Classify MiniMax error for retry decisions */ | ||
| function classifyError(message: string): 'rate_limit' | 'auth' | 'unknown' { | ||
| const m = message.toLowerCase(); | ||
| if (RATE_LIMIT_PATTERNS.some(p => m.includes(p))) return 'rate_limit'; | ||
| if (AUTH_PATTERNS.some(p => m.includes(p))) return 'auth'; | ||
| return 'unknown'; | ||
| } |
There was a problem hiding this comment.
classifyError substring patterns are too loose and can misroute retryable vs. fatal errors.
AUTH_PATTERNSincludes the bare word'invalid', so errors like"invalid request body","invalid model", or any 400 validation response will be classified asauthand throw immediately — even when a retry or a clearer error would be appropriate.RATE_LIMIT_PATTERNS/AUTH_PATTERNSinclude raw numeric tokens ('1002','1039','1004') that can also match inside unrelated substrings (timestamps, request IDs, byte counts in an error body).
Consider matching on word boundaries or structured fields (e.g. parse JSON error body and inspect an explicit code/status field) and tightening 'invalid' to 'invalid api key' / 'invalid token'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/providers/src/minimax/provider.ts` around lines 41 - 50,
classifyError currently uses loose substring matching (RATE_LIMIT_PATTERNS and
AUTH_PATTERNS) that misclassifies errors; update classifyError to (1) first try
to parse the incoming message as JSON and inspect structured fields like
error.code, error.status, or statusCode to decide 'rate_limit' vs 'auth', and
only fall back to string matching if parsing fails, (2) tighten string matches
by using whole-word/regex boundaries and replace generic tokens like 'invalid'
with specific phrases such as 'invalid api key' or 'invalid token', and (3)
remove or regex-anchor raw numeric tokens (e.g., '1002','1039','1004') so they
match only if they appear as standalone error codes. Ensure these changes are
applied inside the classifyError function and reference RATE_LIMIT_PATTERNS and
AUTH_PATTERNS for pattern updates.
| throw new Error('Query aborted'); | ||
| } | ||
|
|
||
| getLog().debug({ model, attempt, baseURL }, 'minimax.query_started'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Structured log event names don't follow the required {domain}.{action}_{state} convention.
minimax.query_started(line 264) has no pairedminimax.query_completed/minimax.query_failedon the success or terminal-failure paths.minimax.fetch_error,minimax.http_error,minimax.stream_error,minimax.sse_parse_error,minimax.empty_response,minimax.retryinguse_error/ bare-action names instead of_failed/ explicit states.
Rename to follow the convention and add the completion/failure pair for query_started (e.g., emit minimax.query_completed after successful yield* and minimax.query_failed on the terminal throw at line 335).
As per coding guidelines: "Logging must use Pino with structured events: format {domain}.{action}_{state} ... always pair _started with _completed or _failed".
Also applies to: 284-284, 303-303, 315-315, 327-327
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/providers/src/minimax/provider.ts` at line 264, The structured log
events in provider.ts violate the `{domain}.{action}_{state}` convention: rename
`minimax.query_started` (getLog().debug call) to `minimax.query_started`, then
emit `minimax.query_completed` after the successful streaming finish (after the
`yield*`/stream end in the query flow) and emit `minimax.query_failed` on the
terminal throw path (the throw around line 335); also rename other events so
they use `_failed` or explicit states instead of `_error`/bare actions —
specifically update `minimax.fetch_error` -> `minimax.fetch_failed`,
`minimax.http_error` -> `minimax.http_failed`, `minimax.stream_error` ->
`minimax.stream_failed`, `minimax.sse_parse_error` ->
`minimax.sse_parse_failed`, `minimax.empty_response` ->
`minimax.empty_response_failed` (or another explicit failed state), and
`minimax.retrying` -> `minimax.retrying_started`/`minimax.retrying_completed` as
appropriate; locate and change the getLog() calls in the streaming/query
functions (the getLog().debug/getLog().warn/getLog().error invocations) so each
`_started` has a paired `_completed` or `_failed` and keep message context
(model, attempt, baseURL).
| try { | ||
| yield* streamMiniMaxResponse(response, requestOptions?.abortSignal); | ||
| return; | ||
| } catch (streamErr) { | ||
| const err = streamErr as Error; | ||
| if (err.message === 'Query aborted') throw err; | ||
| getLog().error({ err, attempt }, 'minimax.stream_error'); | ||
| if (attempt >= MAX_RETRIES) throw err; | ||
| const delayMs = this.retryBaseDelayMs * Math.pow(2, attempt); | ||
| await new Promise(resolve => setTimeout(resolve, delayMs)); | ||
| lastError = err; | ||
| } | ||
| } |
There was a problem hiding this comment.
Streaming retry can emit duplicate content to the consumer.
yield* streamMiniMaxResponse(...) may yield several {type:'assistant', content:...} chunks before it throws (e.g., mid-stream network drop). The outer loop then retries, opens a fresh request, and starts streaming the full response again — so the caller receives the earlier partial content twice, corrupting the assembled assistant message.
Either don't retry once streaming has started, or only retry if no chunks were emitted yet:
🔧 Suggested fix
try {
- yield* streamMiniMaxResponse(response, requestOptions?.abortSignal);
- return;
+ let emitted = false;
+ for await (const chunk of streamMiniMaxResponse(response, requestOptions?.abortSignal)) {
+ emitted = true;
+ yield chunk;
+ }
+ return;
} catch (streamErr) {
const err = streamErr as Error;
if (err.message === 'Query aborted') throw err;
getLog().error({ err, attempt }, 'minimax.stream_error');
- if (attempt >= MAX_RETRIES) throw err;
+ // Don't retry if we've already yielded chunks — the caller would see duplicates.
+ if (emitted || attempt >= MAX_RETRIES) throw err;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/providers/src/minimax/provider.ts` around lines 321 - 333, The
current retry loop can re-deliver partial streamed assistant chunks because
yield* streamMiniMaxResponse(...) may emit data before a stream error; change
the retry logic to detect if any chunks were emitted and, if so, do not
retry—immediately rethrow the stream error instead of attempting a new request.
Concretely, introduce a local boolean (e.g., emittedAnyChunk) that is set by a
small wrapper/iterator around streamMiniMaxResponse when the first chunk is
yielded, then in the catch block check that flag: if emittedAnyChunk is true,
rethrow streamErr; otherwise continue with the existing retry/backoff logic (use
the existing variables attempt, MAX_RETRIES, retryBaseDelayMs, lastError).
| { | ||
| id: 'minimax', | ||
| displayName: 'MiniMax', | ||
| factory: () => new MiniMaxProvider(), | ||
| capabilities: MINIMAX_CAPABILITIES, | ||
| isModelCompatible: (model: string): boolean => model.startsWith('MiniMax-'), | ||
| builtIn: true, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find how isModelCompatible / provider inference from model name is used.
rg -nP -C3 'isModelCompatible\b'
rg -nP -C3 'inferProvider|providerFromModel|resolveProvider'Repository: coleam00/Archon
Length of output: 31746
MiniMax models are incorrectly routed to Codex due to registration order and predicate overlap.
Codex's isModelCompatible predicate (line 127–131) accepts any model that is not a Claude alias or 'inherit'. A model like MiniMax-M2.7 matches this condition. When inferProviderFromModel (packages/workflows/src/model-validation.ts:15–20) iterates through registered providers in order, it returns the first built-in provider whose isModelCompatible returns true. Since Codex is registered before MiniMax, MiniMax-M2.7 models are inferred as 'codex' and will fail at runtime when sent to CodexProvider.
This affects any workflow or node that omits an explicit provider: field but specifies a MiniMax model. No test covers MiniMax inference, leaving this gap undetected.
Either:
- Exclude
MiniMax-*from Codex's predicate (and ideally switch to an allow-list of recognized OpenAI prefixes), or - Document that MiniMax workflows must use explicit
provider: minimax(not rely on model-based inference).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/providers/src/registry.ts` around lines 135 - 142, Codex's
isModelCompatible currently claims any non-Claude/non-'inherit' model which
causes MiniMax-* names to match; update the Codex provider's isModelCompatible
predicate to explicitly exclude MiniMax prefixes (e.g. return false if
model.startsWith('MiniMax-')) or better replace the predicate with an allow-list
of recognized OpenAI prefixes (e.g. check model.startsWith one of ['gpt-',
'text-davinci-', ...]) so MiniMaxProvider (factory: () => new MiniMaxProvider(),
isModelCompatible: (model) => model.startsWith('MiniMax-')) will be selected by
inferProviderFromModel; add/adjust a unit test for inferProviderFromModel to
assert 'MiniMax-M2.7' resolves to the 'minimax' provider.
|
Thanks for this contribution, but we will not add this you can use minimax through claude code or through the PI provider if you want to add provider support it must be added through a coding agent SDK |
2 participants
Summary
Adds MiniMax as a built-in AI agent provider for Archon, enabling users to use cost-effective MiniMax models alongside Claude and Codex.
Changes
packages/providers/src/minimax/provider.ts—MiniMaxProviderimplementingIAgentProvidervia MiniMax's OpenAI-compatible streaming APIpackages/providers/src/minimax/capabilities.ts— Provider capability flags (env injection enabled; no session resume, MCP, hooks, or tool restrictions)packages/providers/src/minimax/config.ts— Config parser formodelandbaseURLassistant defaultspackages/providers/src/registry.ts— Registersminimaxas a built-in provider withMiniMax-*model compatibilitypackages/providers/src/types.ts— AddsMiniMaxProviderDefaultsinterfacepackages/providers/src/index.ts— ExportsMiniMaxProviderandparseMiniMaxConfigpackages/providers/package.json— Adds minimax subpath exports and includes minimax testspackages/providers/src/minimax/provider.test.ts— 23 unit tests covering config parsing, capabilities, streaming, error handling, and model selectionpackages/providers/src/registry.test.ts— Updates existing registry tests to account for the new built-in providerSupported Models
MiniMax-M2.7MiniMax-M2.7-highspeedConfiguration
Set
MINIMAX_API_KEYin your environment. Optionally configure defaults in.archon/config.yaml:Use in a workflow:
API Reference
Notes
1.0(MiniMax requires(0.0, 1.0])Summary by CodeRabbit
New Features
Tests