MiniMax: Token Plan recharge credits and web session enrichment#1981
MiniMax: Token Plan recharge credits and web session enrichment#1981Yuxin-Qiao wants to merge 7 commits into
Conversation
Add token_plan_credit enrichment for cookie-backed refreshes, MiniMax Agent desktop cookie import, API-token web enrichment resolver, and menu credit display. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Codex review: needs changes before merge. Reviewed July 8, 2026, 11:28 AM ET / 15:28 UTC. Summary Reproducibility: yes. for the review findings by source inspection: the latest head still has the relevant imports, guards, and branch conditions in changed MiniMax files. I did not run live MiniMax or Keychain probes because repository policy directs provider/parser validation through stubs unless explicitly requested. Review metrics: 2 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Copy recommended automerge instructionNext step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Land the credit-only enrichment after token-cost routing, Keychain gating, group-safe credit merging, and split-specific docs/changelog handling are repaired; defer usage-summary dashboard claims to the follow-up PR. Do we have a high-confidence way to reproduce the issue? Yes for the review findings by source inspection: the latest head still has the relevant imports, guards, and branch conditions in changed MiniMax files. I did not run live MiniMax or Keychain probes because repository policy directs provider/parser validation through stubs unless explicitly requested. Is this the best way to solve the issue? No; the credit-only split is a better shape than the closed umbrella PR, but the current patch still has bounded correctness, docs, and credential-boundary blockers. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against aa401f1d8b74. Label changesLabel justifications:
Evidence reviewedSecurity concerns:
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (6 earlier review cycles)
|
There was a problem hiding this comment.
💡 Codex Review
When Codex has no credits snapshot and no credits error yet (for example at startup before the optional credits refresh completes), this fallback now returns the generic creditsHint as creditsText because the Codex-specific nil guard was removed. That makes the menu show “Credits unavailable; keep Codex running to refresh.” as an active credits row even though there is no failure or balance to report; keep suppressing the Codex no-data case until credits or error is present.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| tokenCost: ProviderTokenCostConfig( | ||
| supportsTokenCost: false, | ||
| noDataMessage: { "MiniMax cost summary is not supported." }), | ||
| supportsTokenCost: true, |
There was a problem hiding this comment.
Keep MiniMax token cost disabled until fetch is wired
When MiniMax is enabled and the global token-cost history setting is on, setting supportsTokenCost to true sends the provider through UsageStore.refreshTokenUsage (Sources/CodexBar/UsageStore.swift:1495) instead of being ignored. I checked the token-cost paths: MiniMax is not projected from the provider snapshot in UsageStore+TokenCost.swift, and CostUsageFetcher.loadTokenSnapshot currently only allows Codex, Claude, Vertex AI, and Bedrock before throwing unsupportedProvider (Sources/CodexBarCore/CostUsageFetcher.swift:119). As a result, users will see/log “Cost summary is not supported for minimax” rather than a MiniMax usage summary; keep this false or add the MiniMax loader in the same change.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,182 @@ | |||
| import CommonCrypto | |||
There was a problem hiding this comment.
In non-macOS SwiftPM test builds this import is parsed before the #if os(macOS) around the test type, and CommonCrypto is not available as a module there, so the whole CodexBarTests target fails to compile even though these tests are meant to be skipped. Put the platform/canImport guard around the imports as well as the struct.
Useful? React with 👍 / 👎.
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4232e48791
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return try await self.attachingTokenPlanCreditIfAvailable( | ||
| to: enrichedSnapshot, | ||
| context: context, | ||
| groupID: groupID) |
There was a problem hiding this comment.
Fetch MiniMax usage summary before returning
In the MiniMax web fetch path, especially when includeBillingHistory == false, this now returns immediately after credit enrichment; no code ever calls MiniMaxAPIRegion.tokenPlanUsageSummaryURL. Fresh evidence beyond the existing token-cost finding is the new MiniMaxProviderTests assertion for /backend/account/token_plan/usage_summary, which this path cannot satisfy, so make check fails and no usage summary is collected for users. Add that fetch before returning, or stop expecting/advertising it.
Useful? React with 👍 / 👎.
| region: context.region, | ||
| environment: context.environment, | ||
| transport: context.transport) | ||
| return snapshot.withPointsBalanceIfMissing(credit.balance, expiresAt: credit.expiresAt) |
There was a problem hiding this comment.
Use the fetched MiniMax credit balance
When the coding-plan/remains payload already contains a pointsBalance but no expiry, this path still fetches the dedicated token_plan_credit endpoint, then calls withPointsBalanceIfMissing, which preserves the older balance and only fills the expiry. That means the production web-session path does not actually replace a stale or fallback balance with the dedicated recharge-credit value, despite the new helper/test implying the dedicated endpoint should win.
Useful? React with 👍 / 👎.
| if let resolvedGroupID, | ||
| !credit.groupIDs.isEmpty, | ||
| credit.groupIDs.contains(resolvedGroupID) | ||
| { | ||
| enriched = enriched.withPointsBalanceFromDedicatedEndpoint( | ||
| credit.balance, | ||
| expiresAt: credit.expiresAt) | ||
| receivedWebData = true |
There was a problem hiding this comment.
Accept group-less MiniMax credit payloads
For API-token enrichment, a valid token_plan_credit response that has a balance but omits credit_packages_details never enters either branch because credit.groupIDs is empty, so the balance is dropped and the web session is later reported as unavailable. This affects explicit/manual or desktop cookies when the endpoint returns one of the parser-supported balance fallback shapes; apply the balance when no group IDs are supplied, or otherwise mark the response as unverifiable instead of ignoring it.
Useful? React with 👍 / 👎.
Keep optional credits hidden when Codex has no credits snapshot so Buy Credits remains available without rendering an error-only credits section. Co-authored-by: Cursor <cursoragent@cursor.com>
Drop usage_summary host expectations that belong to the dashboard follow-up PR. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 814add198c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| KeychainNoUIQuery.apply(to: &query) | ||
|
|
||
| var result: AnyObject? | ||
| let status = KeychainSecurity.copyMatching(query as CFDictionary, &result) |
There was a problem hiding this comment.
Honor disabled Keychain access for MiniMax cookies
When the user has disabled Keychain access and the MiniMax desktop cookie DB contains encrypted values, importSession() still reaches this SecItemCopyMatching call to derive the Safe Storage key. The query is non-interactive, but it bypasses the app's KeychainAccessGate setting and performs the keychain read that setting is supposed to suppress; return nil before building/issuing the query when KeychainAccessGate.isDisabled, as the other cookie importers do.
Useful? React with 👍 / 👎.
| region: context.region, | ||
| environment: context.environment, | ||
| transport: context.transport) | ||
| return snapshot.withPointsBalanceIfMissing(credit.balance, expiresAt: credit.expiresAt) |
There was a problem hiding this comment.
Verify MiniMax credit groups before merging
When this web fetch has a resolved group ID but token_plan_credit returns package group IDs for a different group, the balance is still merged into the quota snapshot because credit.groupIDs is ignored here. That can attach another MiniMax account/group's points balance to the displayed usage; the API-token enrichment path treats the same condition as accountMismatch, so this path should skip or reject the credit when the returned groups do not include the resolved group.
Useful? React with 👍 / 👎.
| if let resolvedGroupID, | ||
| !credit.groupIDs.isEmpty, | ||
| credit.groupIDs.contains(resolvedGroupID) |
There was a problem hiding this comment.
Apply verified credits when no local group ID exists
For API-token refreshes that use MiniMax Agent or a simple manual/env cookie without minimax_group_id_v2, resolvedGroupID is nil, so a successful token_plan_credit response with normal credit_packages_details falls through both branches: no balance is merged, receivedWebData stays false, and the valid web session is later reported as unavailable. Either accept a safely verifiable single returned group or report an explicit unverifiable/mismatch state instead of dropping the fetched credit.
Useful? React with 👍 / 👎.
Credit PR enrichment only hits token_plan_credit; usage_summary belongs to the dashboard follow-up. Co-authored-by: Cursor <cursoragent@cursor.com>
When token plan subscription is not active but `points_balance` exists, do not treat the response as a blocking API error; keep rendering credits. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f399347ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if tokenPlanInactiveMessage, hasPointsBalance { | ||
| MiniMaxUsageFetcher.log.debug("MiniMax coding plan status ignored: \(status) \(message)") |
There was a problem hiding this comment.
Return a credit-only snapshot for inactive token plans
When MiniMax returns an inactive-token-plan response that has points_balance but no model_remains entries, this branch only suppresses the API error; execution then falls through to the modelRemains guard below and throws parseFailed, so users still lose the remaining credits display for the no-active-subscription case. Return a credit-only snapshot or bypass the quota guard for this partial payload.
Useful? React with 👍 / 👎.
Keep empty-quota snapshots when Token Plan is inactive so recharge credit enrichment can still run instead of blocking the whole MiniMax refresh. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Closing for now — need to revisit the approach; current changes need more work before merge. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f42c35b4d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return MiniMaxUsageSnapshot( | ||
| planName: self.parsePlanName(data: payload.data), | ||
| availablePrompts: nil, | ||
| currentPrompts: nil, | ||
| remainingPrompts: nil, |
There was a problem hiding this comment.
Avoid synthesizing quota for credit-only MiniMax plans
When MiniMax reports an inactive/no-quota token plan with only points_balance, this new branch returns a successful snapshot with all quota fields nil. Fresh evidence after the earlier inactive-plan issue is that MiniMaxUsageSnapshot.toUsageSnapshot() still falls through its single-service fallback and creates a primary RateWindow with usedPercent defaulting to 0, so the menu/status can show a full/empty “Prompts” quota even though no quota lane was reported. Return a snapshot without a primary quota window for this credit-only case, or mark it as unavailable/placeholder before rendering.
Useful? React with 👍 / 👎.
Summary
Split from #1821 (PR A).
token_plan_creditrecharge balance when a web session cookie is availableMiniMaxWebEnrichmentResolverfor API-token refreshespointsBalancediagnostics, and reset-time parsing fixesdocs/minimax.mdTest plan
make checkswift test --filter MiniMaxTokenPlanCreditswift test --filter MiniMaxWebEnrichmentswift test --filter MiniMaxDesktopCookieStack
feat/minimax-usage-dashboardfeat/minimax-quota-utilizationMade with Cursor