Add MiniMax provider backend to local bridge - #646
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
📝 Walkthroughminor risk. this adds a minimax backend without changing the existing runtime backend. security coverage is strong: local bearer and key changes
review focus
Walkthroughthe local bridge now supports runtime or regional minimax backends. it adds minimax model metadata, authenticated responses and messages forwarding, usage tracking, public exports, documentation, and tests for global and china endpoints. Changesminimax local bridge
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant LocalClient
participant LocalBridge
participant MiniMaxAPI
LocalClient->>LocalBridge: send authenticated Responses or Messages request
LocalBridge->>LocalBridge: validate credentials and replace inbound headers
LocalBridge->>MiniMaxAPI: forward request with configured regional api key
MiniMaxAPI-->>LocalBridge: return upstream response
LocalBridge-->>LocalClient: return response and record usage
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
test/local-bridge.test.tsOops! Something went wrong! :( ESLint: 10.0.0 Error: The 'jiti' library is required for loading TypeScript configuration files. Make sure to install it. test/minimax-provider.test.tsOops! Something went wrong! :( ESLint: 10.0.0 Error: The 'jiti' library is required for loading TypeScript configuration files. Make sure to install it. 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/local-bridge.ts (1)
312-330: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winbind the MiniMax egress and handle 429 before passing it through.
lib/local-bridge.ts:323 now calls remote MiniMax without a
signal/timeout, solib/local-bridge.ts:331only catches connect/DNS failures, not a stalled upstream stream; use a response-headers deadline forfetchImplso SSE body streams can still continue. also handle429at lib/local-bridge.ts:323 before returning the provider response at lib/local-bridge.ts:362; the retry-after value should be clamped and not converted into an unbounded local delay. add a regression test for a hanging MiniMax upstream returninglocal_bridge_upstream_error.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/local-bridge.ts` around lines 312 - 330, Update forward to call fetchImpl with a response-headers timeout signal, preserving continued streaming after headers arrive; handle upstream 429 responses before the provider response is returned, clamping any Retry-After-derived delay to a bounded local value. Add a regression test covering a hanging MiniMax upstream and asserting local_bridge_upstream_error.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/reference/commands.md`:
- Around line 414-427: Update the direct MiniMax section around startLocalBridge
to validate MINIMAX_API_KEY explicitly or show a preceding export command
instead of using a non-null assertion, and separate its checklist from the
runtime/proxy setup checklist. Keep runtime rotation and proxy requirements only
under the runtime backend path, while documenting the direct regional backend
requirements independently.
In `@lib/local-bridge.ts`:
- Around line 33-45: In startLocalBridge, add a startup validation that rejects
options containing both runtimeBaseUrl and miniMax before backend selection
occurs. Raise a clear configuration error, preserving the existing validations
and ensuring neither backend is silently discarded for JavaScript or widened
callers.
In `@lib/providers/minimax.ts`:
- Around line 61-65: Update getMiniMaxEndpoints to validate that the requested
region exists in MINIMAX_ENDPOINTS before indexing it, and fail immediately with
an appropriate error for unknown values. Preserve the existing default global
region and return behavior for valid MiniMaxRegion values.
In `@test/local-bridge.test.ts`:
- Around line 463-478: Extend the MiniMax tests around the existing
startup-guard case to cover remote egress failures: assert unauthenticated POST
requests return 401 with local_bridge_unauthorized, rejected fetches return 502
with local_bridge_upstream_error, and 429 upstream responses preserve the client
status while recording a failure ledger row. Add a cast-based unsupported-region
test for the guard near the MiniMax target handling and verify the
MiniMax-specific 404 message in the not-found path.
- Around line 354-359: Add test isolation in the local-bridge test setup by
creating a temporary directory with tmpdir() and fs.mkdtemp() in beforeEach,
assigning it to CODEX_MULTI_AUTH_DIR. In afterEach, restore the prior
environment value and remove the temporary directory with removeWithRetry(),
covering the MiniMax request paths exercised by startLocalBridge.
---
Outside diff comments:
In `@lib/local-bridge.ts`:
- Around line 312-330: Update forward to call fetchImpl with a response-headers
timeout signal, preserving continued streaming after headers arrive; handle
upstream 429 responses before the provider response is returned, clamping any
Retry-After-derived delay to a bounded local value. Add a regression test
covering a hanging MiniMax upstream and asserting local_bridge_upstream_error.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3722aebe-be26-4fde-8b2a-79d99956345c
📒 Files selected for processing (11)
docs/reference/commands.mddocs/reference/public-api.mdindex.tslib/index.tslib/local-bridge.tslib/providers/minimax.tslib/usage/ledger.tslib/usage/redaction.tslib/usage/types.tstest/local-bridge.test.tstest/minimax-provider.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (16)
test/**/*.test.ts
📄 CodeRabbit inference engine (test/AGENTS.md)
test/**/*.test.ts: Write Vitest test suites with globals enabled (describe, it, expect)
Maintain 80%+ coverage threshold across statements, branches, functions, and lines
Use removeWithRetry() for Windows filesystem cleanup instead of bare fs.rm to handle EBUSY, EPERM, and ENOTEMPTY errors
Do not rely on dist/ in tests; use source files instead
Do not skip tests without justification
Relax lint rules for test files as configured in eslint.config.js
Files:
test/minimax-provider.test.tstest/local-bridge.test.ts
**/*.{ts,js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,js,mjs}: Use ESM modules throughout the project; the package is configured with"type": "module".
Do not useas any,@ts-ignore, or@ts-expect-error.
Files:
test/minimax-provider.test.tslib/index.tslib/usage/ledger.tsindex.tslib/usage/types.tslib/usage/redaction.tslib/providers/minimax.tstest/local-bridge.test.tslib/local-bridge.ts
test/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Windows-sensitive filesystem tests and helpers must use retry handling for transient lock-related cleanup and write failures.
Files:
test/minimax-provider.test.tstest/local-bridge.test.ts
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Source changes belong in
index.ts,lib/, andscripts/;dist/is generated output and local temporary/cache directories must not be edited.
Files:
test/minimax-provider.test.tsdocs/reference/public-api.mddocs/reference/commands.mdlib/index.tslib/usage/ledger.tsindex.tslib/usage/types.tslib/usage/redaction.tslib/providers/minimax.tstest/local-bridge.test.tslib/local-bridge.ts
**/*.{js,ts,mjs,cjs}
📄 CodeRabbit inference engine (README.md)
**/*.{js,ts,mjs,cjs}: Do not publish or replace a globalcodexbinary; official OpenAI installation paths must retain ownership of thecodexcommand.
Keep OAuth credentials local and restrict runtime rotation and local bridges to loopback interfaces.
Require hashed local client tokens to protect the optional loopback bridge.
Responsesbackground: truecompatibility must remain opt-in; requests using it must use statefulstore=truerouting rather than statelessstore=falserouting.
Never run npm install or update commands automatically; only display a manual upgrade notice when appropriate.
Experimental synchronization and backup flows must be non-destructive by default: preview before applying sync, preserve destination-only accounts, and fail safely on backup filename collisions.
Keep account storage project-scoped under the configured multi-auth root when operating in repo-specific workflows.
Files:
test/minimax-provider.test.tslib/index.tslib/usage/ledger.tsindex.tslib/usage/types.tslib/usage/redaction.tslib/providers/minimax.tstest/local-bridge.test.tslib/local-bridge.ts
test/**
⚙️ CodeRabbit configuration file
tests must stay deterministic and use vitest. demand regression cases that reproduce concurrency bugs, token refresh races, and windows filesystem behavior. reject changes that mock real secrets or skip assertions.
Files:
test/minimax-provider.test.tstest/local-bridge.test.ts
docs/**/*.md
📄 CodeRabbit inference engine (docs/STYLE_GUIDE.md)
docs/**/*.md: User-facing documentation should follow the page template: Title and one-line lead, Quick path commands, Core operational workflow, Troubleshooting or failure handling, and Related links
Use short sections and scan-friendly tables in documentation where they improve clarity
Prefer direct, actionable language in documentation
Use runnable command examples in documentation
Explain expected outcomes after critical commands in documentation
Keep terminology consistent with runtime names in documentation
Avoid speculative language when behavior is deterministic in documentation
Put the user problem in the first paragraph before implementation detail
Use descriptive page titles such ascodex-multi-auth Featuresinstead of generic titles on public docs
Do not repeat keyword lists in every section; search terms should appear only where they help a developer understand the page
Canonical command family iscodex-multi-auth ...
Canonical runtime root is~/.codex/multi-auth
Runtime rotation must be described as default-on unless the release policy changes
Legacy command/path references belong only in migration contexts in documentation
Compatibility aliases (codex multi auth,codex multi-auth,codex multiauth) belong only in command reference, troubleshooting, or migration contexts
Keep command flags aligned with runtime usage text in documentation
Avoid non-runnable command snippets in documentation
Avoid conflicting path guidance across documentation
Avoid legacy-first onboarding language in documentationOrganize repository documentation according to the defined layers: product entry, user operations, reference, and development.
docs/**/*.md: Do not describecodex-multi-authas replacing@openai/codexor publishing the globalcodexbinary; preserve the official CLI's ownership ofcodex.
Usecodex-multi-authfor account management, and reservecodex-multi-auth-codexormcodexfor intentionally forwarding official Codex commands th...
Files:
docs/reference/public-api.mddocs/reference/commands.md
docs/reference/**/*.md
📄 CodeRabbit inference engine (docs/STYLE_GUIDE.md)
New flags/settings/paths must be reflected in
docs/reference/*
docs/reference/**/*.md: Keep command, API, error-contract, settings, and storage-path details in the canonical reference documentation.
Document compatibility aliases (codex multi auth,codex multi-auth, andcodex multiauth) only in command-reference, troubleshooting, or migration sections.
Files:
docs/reference/public-api.mddocs/reference/commands.md
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (docs/troubleshooting.md)
Document that
codex-multi-auth-codexis the optional forwarding wrapper, whilecodex-multi-authis the canonical account-manager command family; the package does not publish a globalcodexbinary.Document the canonical command names, runtime paths, configuration precedence, storage migration behavior, and upgrade procedures consistently across the referenced documentation.
Files:
docs/reference/public-api.mddocs/reference/commands.md
docs/reference/**/*.{md,mdx}
📄 CodeRabbit inference engine (docs/reference/public-api.md)
For intentional contract breaks, update all affected reference documentation, including command, error, settings, and storage contracts.
Files:
docs/reference/public-api.mddocs/reference/commands.md
docs/**
⚙️ CodeRabbit configuration file
keep README, SECURITY, and docs consistent with actual CLI flags and workflows. whenever behavior changes, require updated upgrade notes and mention new npm scripts.
Files:
docs/reference/public-api.mddocs/reference/commands.md
lib/**/*.ts
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/**/*.ts: Route all public exports throughlib/index.tsor documented package subpaths.
Keep module dependencies acyclic and preserve the layeringtypes/constants → storage → accounts → runtime → manager/CLI; lower layers must not import higher layers.
Preserve runtime rotation pass-through semantics except for intentionally changed auth or provider headers.
Deduplicate emails usingnormalizeEmailKey(), which trims and lowercases the email.
Use classes for state requiring multiple independent instances or dependency injection, includingAccountManager,CircuitBreaker,SessionAffinityStore, and theCodexErrorhierarchy. Reserve module-level state for genuinely process-global concerns and provide a test reset helper for such state.
Never import fromdist/in source tests or library code.
Never suppress type errors.
Never patch official Codex application binaries for desktop routing.
Never use bare recursive cleanup in Windows-sensitive paths without retry handling.
Files:
lib/index.tslib/usage/ledger.tslib/usage/types.tslib/usage/redaction.tslib/providers/minimax.tslib/local-bridge.ts
lib/**
⚙️ CodeRabbit configuration file
focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios. check for logging that leaks tokens or emails.
Files:
lib/index.tslib/usage/ledger.tslib/usage/types.tslib/usage/redaction.tslib/providers/minimax.tslib/local-bridge.ts
lib/usage/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Keep usage ledgers, budget guards, account policies, routing profiles, runtime policies, and the local bridge file-backed under
~/.codex/multi-auth.
Files:
lib/usage/ledger.tslib/usage/types.tslib/usage/redaction.ts
lib/{runtime-rotation-proxy.ts,local-bridge.ts,request/**/*.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
Do not forward stale decoded
content-encodingmetadata when Node fetch has already decoded response bytes.
Files:
lib/local-bridge.ts
lib/{runtime-rotation-proxy.ts,local-bridge.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/{runtime-rotation-proxy.ts,local-bridge.ts}: Runtime proxy client-facing headers and responses must never expose account emails or tokens.
Never include account emails or tokens in runtime proxy client responses.
Files:
lib/local-bridge.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-07-31T12:33:50.086Z
Learning: Use `codex-multi-auth` as the canonical command family; compatibility aliases remain supported for migrations and wrapper-routed environments.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-07-31T12:33:50.086Z
Learning: Run `codex-multi-auth uninstall` before removing the npm package so managed bindings, launchers, configuration entries, and residual artifacts are cleaned up.
📚 Learning: 2026-06-04T06:14:18.093Z
Learnt from: ndycode
Repo: ndycode/codex-multi-auth PR: 510
File: test/scheduling-strategy-config.test.ts:1-1
Timestamp: 2026-06-04T06:14:18.093Z
Learning: In ndycode/codex-multi-auth, do not flag explicit imports from "vitest" (e.g., describe, it, expect, beforeEach/afterEach, etc.) in test files as issues—even if the Vitest config sets `globals: true`. The repo’s established convention is to keep these imports for consistency with neighboring tests; removing them would make files outliers.
Applied to files:
test/minimax-provider.test.tstest/local-bridge.test.ts
📚 Learning: 2026-06-04T06:14:24.975Z
Learnt from: ndycode
Repo: ndycode/codex-multi-auth PR: 510
File: test/runtime-rotation-proxy.test.ts:2478-2491
Timestamp: 2026-06-04T06:14:24.975Z
Learning: In ndycode/codex-multi-auth test files (e.g. `test/*.test.ts`), when creating V3 storage fixtures for accounts, it’s an intentional convention to use `as never` for deliberately minimal stored-account objects that only include `refreshToken`, `addedAt`, and `lastUsed`. Do not treat `as never` here as a type-safety problem: optional/other fields are expected to be populated by the runtime during execution, and the cast is used solely to keep the fixture minimal and consistent across existing tests.
Applied to files:
test/minimax-provider.test.tstest/local-bridge.test.ts
🔇 Additional comments (13)
index.ts (1)
4723-4724: 📐 Maintainability & Code Qualitycover both new public barrels with source-level vitest tests.
the new exports can compile while one package barrel remains broken. add tests using the existing vitest setup at
test/index.test.ts:1; do not importdist/.
index.ts#L4723-L4724: verifystartLocalBridgeandcreateMiniMaxModelsResponsethrough the package root.lib/index.ts#L51: verifycreateMiniMaxModelsResponsethroughlib/index.ts.Source: Path instructions
docs/reference/commands.md (1)
370-374: 📐 Maintainability & Code Qualityadd the required upgrade note for this bridge contract.
docs/reference/commands.md:370-374anddocs/reference/commands.md:414-438document new routes, credential replacement, directminiMaxconfiguration, and authentication requirements. verify thatdocs/upgrade.mdrecords this behavior change and mentions any new npm scripts. the upgrade file is not included in the supplied review set.Also applies to: 414-427, 437-438
Source: Path instructions
docs/reference/public-api.md (1)
129-133: 🗄️ Data Integrity & Integrationdefine the direct backend contract in the public api.
docs/reference/public-api.md:129-133names runtime andminiMaxmodes but does not define theminiMaxobject or the selection rule whenruntimeBaseUrlandminiMaxare both present. documentapiKey,region: "global" | "cn", and the validation or precedence rule. keep it consistent withdocs/reference/commands.md:419-425.Source: Path instructions
lib/providers/minimax.ts (1)
1-19: LGTM!Also applies to: 21-59
test/minimax-provider.test.ts (1)
9-18: LGTM!Also applies to: 20-47, 49-68
lib/local-bridge.ts (4)
105-105: LGTM!Also applies to: 120-126
212-252: LGTM!Also applies to: 266-280
282-310: LGTM!
367-436: LGTM!lib/usage/ledger.ts (1)
42-42: LGTM!lib/usage/redaction.ts (1)
22-22: LGTM!lib/usage/types.ts (1)
10-10: LGTM!test/local-bridge.test.ts (1)
338-353: LGTM!Also applies to: 429-461
| For direct MiniMax access, select a regional backend instead of a runtime | ||
| proxy. The provider key remains inside the host process, and local client auth | ||
| must stay enabled: | ||
|
|
||
| ```ts | ||
| const bridge = await startLocalBridge({ | ||
| miniMax: { | ||
| apiKey: process.env.MINIMAX_API_KEY!, | ||
| region: "global", // or "cn" | ||
| }, | ||
| requireAuth: true, | ||
| }); | ||
| ``` | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
separate the runtime and direct minimax setup paths.
docs/reference/commands.md:414-425 introduces direct minimax configuration, but the following checklist at docs/reference/commands.md:430-433 still requires runtime rotation and a proxy. that misdirects direct minimax users. give each backend its own checklist. also replace process.env.MINIMAX_API_KEY! at docs/reference/commands.md:421 with an explicit missing-key check or a preceding export command.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/reference/commands.md` around lines 414 - 427, Update the direct MiniMax
section around startLocalBridge to validate MINIMAX_API_KEY explicitly or show a
preceding export command instead of using a non-null assertion, and separate its
checklist from the runtime/proxy setup checklist. Keep runtime rotation and
proxy requirements only under the runtime backend path, while documenting the
direct regional backend requirements independently.
Source: Path instructions
| export type LocalBridgeOptions = LocalBridgeCommonOptions & | ||
| ( | ||
| | { | ||
| runtimeBaseUrl: string; | ||
| runtimeClientApiKey?: string; | ||
| miniMax?: never; | ||
| } | ||
| | { | ||
| miniMax: MiniMaxBridgeBackendOptions; | ||
| runtimeBaseUrl?: never; | ||
| runtimeClientApiKey?: never; | ||
| } | ||
| ); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
reject both backends at runtime, not only in the type.
the miniMax?: never / runtime*?: never arms are compile-time only. a plain-js caller, or any caller that widens the options object, can pass runtimeBaseUrl and miniMax together. lib/local-bridge.ts:367 then picks MiniMax and the runtime target is dropped silently. add the check beside the other startup validations so misconfiguration fails loudly.
♻️ proposed guard in `startLocalBridge`
const miniMax = options.miniMax;
const runtimeBaseUrl = options.runtimeBaseUrl?.trim().replace(/\/+$/, "") || null;
+ if (miniMax && runtimeBaseUrl) {
+ throw new Error(
+ "Local bridge accepts either a runtimeBaseUrl or a MiniMax backend, not both.",
+ );
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/local-bridge.ts` around lines 33 - 45, In startLocalBridge, add a startup
validation that rejects options containing both runtimeBaseUrl and miniMax
before backend selection occurs. Raise a clear configuration error, preserving
the existing validations and ensuring neither backend is silently discarded for
JavaScript or widened callers.
| export function getMiniMaxEndpoints( | ||
| region: MiniMaxRegion = "global", | ||
| ): MiniMaxRegionalEndpoints { | ||
| return MINIMAX_ENDPOINTS[region]; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
fail fast on an unknown region inside the provider.
getMiniMaxEndpoints indexes MINIMAX_ENDPOINTS without a guard. a region value that arrives from config, env, or a plain-js caller returns undefined, and the crash then surfaces later at the property access in lib/local-bridge.ts:384. today only lib/local-bridge.ts:216 validates the region, and that guard is in the consumer, not in the exported catalog.
♻️ proposed guard
export function getMiniMaxEndpoints(
region: MiniMaxRegion = "global",
): MiniMaxRegionalEndpoints {
- return MINIMAX_ENDPOINTS[region];
+ const endpoints = MINIMAX_ENDPOINTS[region];
+ if (!endpoints) {
+ throw new Error(`Unsupported MiniMax region: ${String(region)}`);
+ }
+ return endpoints;
}📝 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 function getMiniMaxEndpoints( | |
| region: MiniMaxRegion = "global", | |
| ): MiniMaxRegionalEndpoints { | |
| return MINIMAX_ENDPOINTS[region]; | |
| } | |
| export function getMiniMaxEndpoints( | |
| region: MiniMaxRegion = "global", | |
| ): MiniMaxRegionalEndpoints { | |
| const endpoints = MINIMAX_ENDPOINTS[region]; | |
| if (!endpoints) { | |
| throw new Error(`Unsupported MiniMax region: ${String(region)}`); | |
| } | |
| return endpoints; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/providers/minimax.ts` around lines 61 - 65, Update getMiniMaxEndpoints to
validate that the requested region exists in MINIMAX_ENDPOINTS before indexing
it, and fail immediately with an appropriate error for unknown values. Preserve
the existing default global region and return behavior for valid MiniMaxRegion
values.
| const server = await startLocalBridge({ | ||
| miniMax: { apiKey: "mm-test", region: "global" }, | ||
| fetchImpl, | ||
| verifyBearerToken, | ||
| }); | ||
| openServers.push(server); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: verify the local-bridge suite redirects the usage ledger root and uses retrying cleanup.
set -euo pipefail
fd -t f 'local-bridge.test.ts' test --exec rg -nP -C5 'CODEX_MULTI_AUTH_DIR|beforeEach|afterEach|afterAll|mkdtemp|removeWithRetry|fs\.rm' {}
# how do neighbouring ledger-touching suites isolate the root?
rg -nP --type=ts -C3 'CODEX_MULTI_AUTH_DIR' test | head -80
# confirm the retrying cleanup helper name and location
rg -nP --type=ts '\bexport (async )?function removeWithRetry\b' -C3Repository: ndycode/codex-multi-auth
Length of output: 5440
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== local-bridge test setup =="
sed -n '1,80p' test/local-bridge.test.ts | cat -n
echo "== utility exports containing removeWithRetry / createTempRoot =="
rg -n "removeWithRetry|createTempRoot|mkdtemp|CODEX_MULTI_AUTH_DIR" test/lib.ts test/shared.ts test -g '*.ts' --glob '!test/local-bridge.test.ts' | head -200
echo "== usage-ledger test setup context =="
sed -n '1,80p' test/usage-ledger.test.ts | cat -n
echo "== local-client-tokens test setup context =="
sed -n '1,40p' test/local-client-tokens.test.ts | cat -nRepository: ndycode/codex-multi-auth
Length of output: 22975
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== local-bridge test file size and relevant MiniMax setup/usages =="
wc -l test/local-bridge.test.ts
rg -n "tmpdir|mkdtemp|CODEX_MULTI_AUTH_DIR|beforeEach|afterEach|describe|it\\(" test/local-bridge.test.ts
echo
echo "== local-bridge ledger calls =="
rg -n "appendUsageLedgerRow|usage|CODEX_MULTI_AUTH_DIR|mkdtemp|removeWithRetry" lib/local-bridge.ts test/local-bridge.test.ts test/usage-ledger.test.ts -C 4Repository: ndycode/codex-multi-auth
Length of output: 13129
isolate the MiniMax local-bridge tests from the real usage ledger.
test/local-bridge.test.ts:30 does not set CODEX_MULTI_AUTH_DIR, so the MiniMax request paths at lib/local-bridge.ts:332, lib/local-bridge.ts:353, and lib/local-bridge.ts:372 can write to the developer’s home ledger instead of a temp dir. add a beforeEach using tmpdir() + fs.mkdtemp(), set CODEX_MULTI_AUTH_DIR, and restore + delete it with removeWithRetry() in afterEach.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/local-bridge.test.ts` around lines 354 - 359, Add test isolation in the
local-bridge test setup by creating a temporary directory with tmpdir() and
fs.mkdtemp() in beforeEach, assigning it to CODEX_MULTI_AUTH_DIR. In afterEach,
restore the prior environment value and remove the temporary directory with
removeWithRetry(), covering the MiniMax request paths exercised by
startLocalBridge.
Source: Coding guidelines
| it("requires a provider key and local auth for MiniMax", async () => { | ||
| const { fetchImpl } = createFetch(); | ||
| await expect( | ||
| startLocalBridge({ | ||
| miniMax: { apiKey: "" }, | ||
| fetchImpl, | ||
| }), | ||
| ).rejects.toThrow(/requires a MiniMax apiKey/i); | ||
| await expect( | ||
| startLocalBridge({ | ||
| miniMax: { apiKey: "mm-test" }, | ||
| fetchImpl, | ||
| requireAuth: false, | ||
| }), | ||
| ).rejects.toThrow(/requires requireAuth=true/i); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
extend MiniMax coverage to the failure paths.
the happy path and the two startup guards are covered. the failure paths on the new remote egress are not. please add cases for:
- an unauthenticated
POST /anthropic/v1/messagesreturning 401 withlocal_bridge_unauthorized, so thex-api-keyfallback atlib/local-bridge.ts:292cannot regress into an open path. - a
fetchImplthat rejects, asserting 502local_bridge_upstream_errorfor a MiniMax target (lib/local-bridge.ts:331). - a
429upstream response, asserting the status reaches the client and the ledger row recordsfailure(lib/local-bridge.ts:355). - the unsupported-region throw at
lib/local-bridge.ts:216; the union type makes it unreachable from typescript, so only a test with a cast proves the guard still works. - the MiniMax-specific 404 message at
lib/local-bridge.ts:423.
the 502 and 429 cases are also the regressions needed for the missing upstream deadline flagged on lib/local-bridge.ts:312-330.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/local-bridge.test.ts` around lines 463 - 478, Extend the MiniMax tests
around the existing startup-guard case to cover remote egress failures: assert
unauthenticated POST requests return 401 with local_bridge_unauthorized,
rejected fetches return 502 with local_bridge_upstream_error, and 429 upstream
responses preserve the client status while recording a failure ledger row. Add a
cast-based unsupported-region test for the guard near the MiniMax target
handling and verify the MiniMax-specific 404 message in the not-found path.
Source: Path instructions
Reason: Add MiniMax-M3 and MiniMax-M2.7 access through the Global and China Responses and Messages-compatible endpoints.
x-api-keycredentials and replace them with the configured provider credential before egress.Checks:
npm test -- --run test/minimax-provider.test.ts test/local-bridge.test.ts test/usage-redaction.test.ts test/documentation.test.tsnpm run typechecknpm run lintnpm run pack:checkThe full
npm testrun also reports existing platform-specific wrapper, path, backup, and timeout failures that reproduce from base commit89ca969.note: greptile review for oc-chatgpt-multi-auth. cite files like
lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.Greptile Summary
adds direct minimax support to the loopback bridge.
Confidence Score: 4/5
the minimax usage-accounting defect should be fixed before merging because direct provider spend and tokens are omitted from governance reports and limits.
minimax forwarding and token replacement are covered, including concurrent ledger serialization and windows-safe filesystem locking, but every new billable request is persisted as unknown zero usage despite the provider catalog defining model pricing.
Files Needing Attention: lib/local-bridge.ts, lib/providers/minimax.ts, test/local-bridge.test.ts
Important Files Changed
Sequence Diagram
Comments Outside Diff (1)
lib/local-bridge.ts, line 353-359 (link)when the bridge completes a minimax request, it records only the operation, outcome, status, and duration, causing the model to appear as
unknownand its tokens and cost to count as zero in usage reports and budget limits.Context Used: lib/AGENTS.md (source)
Knowledge Base Used: Quota, Usage, and Budget Tracking
Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "Add MiniMax provider bridge backend" | Re-trigger Greptile
Context used (3)