Skip to content

fix: honor Codex OAuth source for credits refresh#1054

Merged
steipete merged 4 commits into
steipete:mainfrom
soumikbhatta:codex/codex-oauth-credits-source
May 20, 2026
Merged

fix: honor Codex OAuth source for credits refresh#1054
steipete merged 4 commits into
steipete:mainfrom
soumikbhatta:codex/codex-oauth-credits-source

Conversation

@soumikbhatta
Copy link
Copy Markdown
Contributor

Summary

  • route Codex credits refresh through the provider fetch plan instead of the raw CLI credits fetcher
  • preserve explicit OAuth source selection when refreshing credits, so OAuth-only setups do not fall back to CLI RPC
  • add regression coverage for explicit OAuth credits refresh with the CLI unavailable

Why

Normal Codex usage refreshes already honor the configured source mode, but credits refresh bypassed that path and called the CLI credits fetcher directly. That could still hit the CLI/RPC path even when Codex was configured for OAuth.

Fixes #1048

Validation

  • swift test --filter CodexAccountScopedRefreshTests
  • make check
  • TZ=UTC swift test

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6755342a94

ℹ️ 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".

Comment on lines +104 to +106
let descriptor = self.providerSpecs[.codex]?.descriptor ?? ProviderDescriptorRegistry.descriptor(for: .codex)
let context = self.makeFetchContext(provider: .codex, override: nil, includeCredits: true)
let outcome = await descriptor.fetchOutcome(context: context)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep credits refresh from depending on usage snapshot

Routing credits refresh through descriptor.fetchOutcome makes the result hinge on strategies that require a non-nil usage snapshot, which regresses CLI-mode credits refresh when Codex RPC returns credits but no usage windows. In that case CodexCLIUsageStrategy.fetch throws UsageError.noRateLimitsFound even though credits exist, so refreshCreditsIfNeeded now fails where the previous direct loadLatestCredits() call succeeded; this affects users with explicit .cli (and some .auto fallback) source modes.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in bbb677e by preserving CLI credits-only refresh when RPC returns credits without rate-limit windows. Added regression coverage for that exact case in CodexBaselineCharacterizationTests.

@steipete steipete merged commit 2c8d83c into steipete:main May 20, 2026
8 of 10 checks passed
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.

Codex provider should support OAuth-only / desktop-only setups without falling back to CLI RPC

2 participants