fix: Codex API key auth status detection#672
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCredential detection was moved from instance methods to module-level helpers. The new exported checkStatus() inspects, in order: ChangesCodex Auth Refactoring
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
wushap
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/providers/codex/status.js`:
- Around line 43-94: The current readTomlStringValue/getConfiguredProviderApiKey
logic uses regex and misses valid TOML forms; replace this ad-hoc parsing with
the existing `@iarna/toml` parser: in loadCodexConfig keep reading the file but in
getConfiguredProviderApiKey parse the file content with
require('@iarna/toml').parse(content), read parsed.model_provider and then look
up parsed.model_providers[providerId].env_key (with null checks), then read
process.env[envKey] and return { envKey, apiKey } or null; remove/stop using
readTomlStringValue and ensure parsing errors are caught and cause
getConfiguredProviderApiKey to return null.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3749f2c4-99ba-4fbd-933c-ed9fd23a9462
📒 Files selected for processing (1)
server/providers/codex/status.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/providers/codex/status.js`:
- Around line 66-104: The authentication check in checkCredentials wrongly
prefers generic env vars before the active provider; change checkCredentials to
call loadCodexConfig() and getConfiguredProviderApiKey(config) first and return
authenticated if that returns a valid envKey/apiKey, and only if no configured
provider API key is found fall back to checking process.env.CODEX_API_KEY and
process.env.OPENAI_API_KEY; keep the same return shape ({ authenticated: true,
email: 'API Key Auth', method: 'api_key' }) and ensure you still trim and
validate values as currently done in getConfiguredProviderApiKey and the generic
env checks.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 59cecfdd-3a32-4c89-935e-fce1b0ad3228
📒 Files selected for processing (1)
server/providers/codex/status.js
|
Hey @wushap, thanks for the PR. We want to merge this. However, there has been some big refactor recently and a lot of code has moved around. Can you take a look at the current setup and resubmit the PR? |
wushap
force-pushed
the
codex/fix-codex-auth-status
branch
from
626aba3 to
f9f02bf
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/modules/providers/list/codex/codex-auth.provider.ts`:
- Around line 108-129: The current logic after readConfigTomlStatus falls
through to a "Codex not configured" error even when an OPENAI_API_KEY is present
in the process environment; restore the env-only fallback by checking
process.env.OPENAI_API_KEY (in codex-auth.provider.ts, around the
readConfigTomlStatus/codexHome block) and if set return the same authenticated
response used for API-key-based auth (authenticated: true, appropriate
email/method or method:'apiKey', error:null) instead of the "Codex not
configured" object so providers using only OPENAI_API_KEY appear connected.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: bdab540c-e00b-4337-a40b-b004c406bd3e
📒 Files selected for processing (1)
server/modules/providers/list/codex/codex-auth.provider.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/modules/providers/list/codex/codex-auth.provider.ts`:
- Around line 97-121: The current catch blocks in the Codex auth probe (around
the read of auth.json and config.toml where readAuthJsonStatus and
readConfigTomlStatus are called) return immediately on non-ENOENT errors,
short-circuiting other credential sources (like process.env.OPENAI_API_KEY).
Change the logic to record or stash the non-ENOENT error (e.g., save a local
variable lastReadError or authReadError) and continue to the next probe (do not
return); after all credential sources (auth.json, config.toml, and environment)
have been checked, if none authenticated then return an unauthenticated result
that uses the most relevant stored error.message (or a combined message) as the
error field. Ensure you still treat ENOENT as non-fatal and proceed normally,
and reference codexHome and process.env where those probes run.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4f484e0c-2adb-4d8c-a369-b5c8820c1397
📒 Files selected for processing (1)
server/modules/providers/list/codex/codex-auth.provider.ts
Done! I've updated the PR to match the new project structure. Tested locally and it's all good to go. |
2 participants
Summary
codex/fix-codex-auth-statusbranch onto the latestmainmodel_provider -> model_providers.<provider>.env_keyauth.jsonandOPENAI_API_KEYfallback behaviorRoot Cause
The previous status check only looked at
~/.codex/auth.jsonand did not account for Codex configurations that use a custommodel_providerwith anenv_key. Those setups can run successfully in Codex, but this UI incorrectly showed them as disconnected.Validation
npm run typecheckSummary by CodeRabbit