Support Codex subscription auth in Daytona#296
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90aedd1477
ℹ️ 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".
| from benchflow.agents.providers import find_provider | ||
|
|
||
| return find_provider(model) is None |
There was a problem hiding this comment.
This native-context check treats every unregistered model prefix as OpenAI-native, so CODEX_ACCESS_TOKEN now bypasses the OPENAI_API_KEY requirement even when Codex is routed to a custom endpoint via OPENAI_BASE_URL/BENCHFLOW_PROVIDER_BASE_URL with an unprefixed model (for example local OpenAI-compatible stubs). In that setup, BenchFlow accepts auth that is not a provider API key and skips the intended validation path, which can break custom-provider runs or send the wrong bearer token to non-OpenAI endpoints. The new regression test only covers prefixed providers (vllm/...), so this unprefixed custom-endpoint path remains unguarded.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| assert result["_BENCHFLOW_SUBSCRIPTION_AUTH"] == "1" | ||
|
|
||
| def test_codex_access_token_auth(self, monkeypatch, tmp_path): | ||
| """Guards PR #295: Blocks-style Codex auth via CODEX_ACCESS_TOKEN.""" |
There was a problem hiding this comment.
🟡 Regression test docstrings reference non-existent PR #295 instead of current PR #296
AGENTS.md mandates: "Regression tests must name the PR/commit they guard in the docstring." All five new regression tests reference "PR #295" but this is PR #296, and no PR #295 appears anywhere in the repository's git history (the most recent PR is #294). This breaks the traceability the convention is designed to provide. Affected locations: tests/test_subscription_auth.py:179, tests/test_subscription_auth.py:195, tests/test_subscription_auth.py:214, tests/test_resolve_env_helpers.py:383, tests/test_resolve_env_helpers.py:400.
| """Guards PR #295: Blocks-style Codex auth via CODEX_ACCESS_TOKEN.""" | |
| """Guards PR #296: Blocks-style Codex auth via CODEX_ACCESS_TOKEN.""" |
Was this helpful? React with 👍 or 👎 to provide feedback.
| assert "_BENCHFLOW_SUBSCRIPTION_AUTH" not in result | ||
|
|
||
| def test_codex_api_key_auth_alias(self, monkeypatch, tmp_path): | ||
| """Guards PR #295: CODEX_API_KEY works for native Codex auth.""" |
There was a problem hiding this comment.
🟡 Regression test docstring references non-existent PR #295 instead of PR #296
Same AGENTS.md convention violation as the other four new regression tests — references "PR #295" which doesn't exist in repo history. Should reference current PR #296.
| """Guards PR #295: CODEX_API_KEY works for native Codex auth.""" | |
| """Guards PR #296: CODEX_API_KEY works for native Codex auth.""" |
Was this helpful? React with 👍 or 👎 to provide feedback.
| def test_codex_access_token_does_not_auth_custom_provider( | ||
| self, monkeypatch, tmp_path | ||
| ): | ||
| """Guards PR #295: access tokens are not proxy API keys.""" |
There was a problem hiding this comment.
🟡 Regression test docstring references non-existent PR #295 instead of PR #296
Same AGENTS.md convention violation — references "PR #295" which doesn't exist in repo history. Should reference current PR #296.
| """Guards PR #295: access tokens are not proxy API keys.""" | |
| """Guards PR #296: access tokens are not proxy API keys.""" |
Was this helpful? React with 👍 or 👎 to provide feedback.
| def test_no_model_codex_access_token_wins_over_host_auth( | ||
| self, monkeypatch, tmp_path | ||
| ): | ||
| """Guards PR #295: CODEX_ACCESS_TOKEN is already usable auth.""" |
There was a problem hiding this comment.
🟡 Regression test docstring references non-existent PR #295 instead of PR #296
Same AGENTS.md convention violation — references "PR #295" which doesn't exist in repo history. Should reference current PR #296.
| """Guards PR #295: CODEX_ACCESS_TOKEN is already usable auth.""" | |
| """Guards PR #296: CODEX_ACCESS_TOKEN is already usable auth.""" |
Was this helpful? React with 👍 or 👎 to provide feedback.
| assert "_BENCHFLOW_SUBSCRIPTION_AUTH" not in result | ||
|
|
||
| def test_no_model_codex_api_key_alias_normalizes(self, monkeypatch, tmp_path): | ||
| """Guards PR #295: CODEX_API_KEY is Codex-native API-key auth.""" |
There was a problem hiding this comment.
🟡 Regression test docstring references non-existent PR #295 instead of PR #296
Same AGENTS.md convention violation — references "PR #295" which doesn't exist in repo history. Should reference current PR #296.
| """Guards PR #295: CODEX_API_KEY is Codex-native API-key auth.""" | |
| """Guards PR #296: CODEX_API_KEY is Codex-native API-key auth.""" |
Was this helpful? React with 👍 or 👎 to provide feedback.
| for k in ("OPENAI_API_KEY", "ANTHROPIC_API_KEY"): | ||
| monkeypatch.delenv(k, raising=False) |
There was a problem hiding this comment.
🟡 Test does not clear CODEX_API_KEY from environment, unlike all neighboring tests
test_codex_access_token_does_not_auth_custom_provider clears only OPENAI_API_KEY and ANTHROPIC_API_KEY from the environment but does NOT clear CODEX_API_KEY. Every other Codex auth test in the same class (e.g. test_codex_subscription_auth at line 159-165, test_codex_access_token_auth at line 180, test_codex_api_key_auth_alias at line 196) consistently clears all Codex-related env vars. If a CI runner has CODEX_API_KEY set in its environment, auto_inherit_env will copy it into agent_env. While the test still passes today (because _normalize_codex_auth_env correctly refuses to bridge CODEX_API_KEY → OPENAI_API_KEY for vllm-prefixed models), the incomplete cleanup makes the test fragile and inconsistent with the established pattern.
| for k in ("OPENAI_API_KEY", "ANTHROPIC_API_KEY"): | |
| monkeypatch.delenv(k, raising=False) | |
| for k in ("CODEX_ACCESS_TOKEN", "CODEX_API_KEY", "OPENAI_API_KEY", "ANTHROPIC_API_KEY"): | |
| monkeypatch.delenv(k, raising=False) |
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Validation
Live E2E
Ran with OPENAI_API_KEY, CODEX_API_KEY, and CODEX_ACCESS_TOKEN unset so auth had to come from local ~/.codex/auth.json uploaded into Daytona.
Note
Medium Risk
Updates credential resolution for
codex-acp, which can affect whether runs authenticate via API key, access token, or host login—especially when using custom OpenAI-compatible endpoints. Changes are well-covered by new tests and docs, but auth-path regressions could block Codex runs.Overview
Adds first-class Codex subscription/access-token support by auto-inheriting
CODEX_ACCESS_TOKEN/CODEX_API_KEY, treatingCODEX_ACCESS_TOKENas sufficient auth for native OpenAI Codex runs, and normalizingCODEX_API_KEY→OPENAI_API_KEYso existing~/.codex/auth.jsonwriting continues to work.Tightens auth fallback rules so host login / subscription auth and access tokens are not accepted when Codex is routed to a custom OpenAI-compatible base URL, and updates conformance/integration scripts, example scripts, and docs to reflect the expanded Codex credential options.
Reviewed by Cursor Bugbot for commit 34f3044. Bugbot is set up for automated code reviews on this repo. Configure here.