Repository navigation
[codex] add Codex OAuth support - #405
Conversation
|
Warning Review limit reached
More reviews will be available in 49 minutes and 31 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR adds OAuth 2.0 device-code authentication for OpenAI Codex API. It introduces credential storage with cross-platform file locking, a Codex OAuth client, HTTP bearer token injection with automatic refresh, CLI commands for managing credentials, and integration with the LLM provider system. ChangesCodex OAuth Implementation
Sequence DiagramsequenceDiagram
participant User
participant CLI as oauth_cli
participant Client as CodexOAuthClient
participant Endpoint as OAuth Endpoint
participant Store as OAuthStore
participant LLMPlugin as LLMProviderPlugin
User->>CLI: oauth login codex
CLI->>Client: request_device_code()
Client->>Endpoint: POST /device_code
Endpoint-->>Client: {user_code, device_auth_id}
CLI-->>User: Visit URL + enter code
User->>Endpoint: Authorize device
CLI->>Client: poll_device_token(timeout=15min)
Client->>Endpoint: Poll /device_token
Endpoint-->>Client: {access_token, refresh_token, id_token}
Client->>Store: set_provider("codex", record)
Store-->>CLI: Success
CLI-->>User: Logged in as user@example.com
User->>LLMPlugin: create LLM with "codex" provider
LLMPlugin->>Store: get_provider("codex")
Store-->>LLMPlugin: OAuthProviderRecord
LLMPlugin->>LLMPlugin: create OAuthBearerAuth(token_source)
LLMPlugin->>LLMPlugin: create OpenAICompatProvider(auth=bearer_auth)
LLMPlugin-->>User: Provider ready
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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)
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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
coding-agent/tests/cli/test_oauth_commands.py (1)
45-59: ⚡ Quick winAdd a fresh-install
oauth doctortest.This only covers the happy path where the auth directory already exists. A case with
--auth-fileunder a nonexistent subdirectory would catch the missing-path regression indoctor().🤖 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 `@coding-agent/tests/cli/test_oauth_commands.py` around lines 45 - 59, Add a new test that exercises the "fresh-install" path by invoking the CLI's "oauth doctor" command with an --auth-file path whose parent directory does not exist (so the code must create the directory); reuse the same assertions as test_oauth_doctor_redacts_tokens (check exit_code == 0, that provider text and "<redacted>" appear, and that secret-access-token/secret-refresh-token do not appear) to ensure doctor() handles creating the missing directory and still redacts tokens when run via main with ["oauth", "doctor", "--auth-file", str(auth_file)].
🤖 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 `@coding-agent/docs/adr/0050-codex-oauth-provider.md`:
- Around line 104-105: Replace the machine-local absolute paths
`/workspace/ya-oauth-ref/packages/ya-oauth/` and
`/workspace/ya-oauth-ref/packages/ya-oauth-provider/` with portable references:
use repository-relative paths such as `./packages/ya-oauth/` and
`./packages/ya-oauth-provider/` or, if linking external docs, replace them with
full repo URLs (e.g., the GitHub tree links) so other contributors can resolve
the references.
In `@coding-agent/src/coding_agent/oauth/auth.py`:
- Line 22: The module sets requires_response_body = True which forces httpx to
buffer every response and breaks streaming; remove or set requires_response_body
to False (or delete that attribute) in coding_agent/oauth/auth.py and instead
explicitly read the response body only in the 401 retry path by calling
response.read() (sync) or await response.aread() (async) inside the code that
handles the 401/retry logic (where you currently examine the response to decide
to refresh credentials and retry). This keeps streaming responses incremental
while still consuming the body when necessary for the 401 handling.
In `@coding-agent/src/coding_agent/oauth/cli.py`:
- Around line 168-171: The _print_mode function currently calls os.stat(path)
which raises FileNotFoundError for missing files/dirs; wrap the stat call in a
try/except (or check path.exists()) inside _print_mode so missing paths are
handled gracefully — when the path does not exist, print a clear diagnostic like
"<label> missing" (or "not found") instead of raising, and keep the existing
behavior for existing paths (compute mode with stat.S_IMODE and compare to
expected). Update references to _print_mode (used by the oauth doctor command)
so doctor continues and prints other checks even if this path is absent.
In `@coding-agent/src/coding_agent/oauth/store.py`:
- Around line 202-212: The update callback passed to OAuthStore.update currently
calls self._refresh_provider(record) while holding the auth-file lock, blocking
other readers/writers; instead, read the existing record inside the update
callback (using auth_file.providers.get(self.provider_name)) and return
immediately without refreshing, then release the lock and call
self._refresh_provider(record) outside the lock; after refresh, reacquire the
lock with a second call to self.store.update to re-read the current stored
record, verify it hasn't been changed (or handle conflict if it has), and then
persist the refreshed record back into auth_file.providers[self.provider_name];
reference OAuthStore.update, the update(inner) callback, self._refresh_provider,
auth_file.providers and self.provider_name when making the changes.
In `@coding-agent/tests/coding_agent/test_codex_oauth.py`:
- Around line 42-43: The POSIX-only permission assertions in the test (the two
asserts using stat.S_IMODE on auth_path.parent.stat().st_mode and
auth_path.stat().st_mode) should be skipped on Windows: wrap or guard these
checks with a platform check (e.g., if os.name != "nt") so they only run on
POSIX systems; import os in the test if needed and place the two assertions
inside that conditional block to avoid false failures on Windows.
---
Nitpick comments:
In `@coding-agent/tests/cli/test_oauth_commands.py`:
- Around line 45-59: Add a new test that exercises the "fresh-install" path by
invoking the CLI's "oauth doctor" command with an --auth-file path whose parent
directory does not exist (so the code must create the directory); reuse the same
assertions as test_oauth_doctor_redacts_tokens (check exit_code == 0, that
provider text and "<redacted>" appear, and that
secret-access-token/secret-refresh-token do not appear) to ensure doctor()
handles creating the missing directory and still redacts tokens when run via
main with ["oauth", "doctor", "--auth-file", str(auth_file)].
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7d4d3b7e-f932-4a3d-ac80-07158285783f
📒 Files selected for processing (15)
coding-agent/docs/adr/0050-codex-oauth-provider.mdcoding-agent/src/coding_agent/cli/main.pycoding-agent/src/coding_agent/cli/oauth_commands.pycoding-agent/src/coding_agent/core/config.pycoding-agent/src/coding_agent/oauth/__init__.pycoding-agent/src/coding_agent/oauth/auth.pycoding-agent/src/coding_agent/oauth/cli.pycoding-agent/src/coding_agent/oauth/codex.pycoding-agent/src/coding_agent/oauth/store.pycoding-agent/src/coding_agent/oauth/types.pycoding-agent/src/coding_agent/plugins/llm_provider.pycoding-agent/src/coding_agent/providers/openai_compat.pycoding-agent/tests/cli/test_oauth_commands.pycoding-agent/tests/coding_agent/plugins/test_llm_provider.pycoding-agent/tests/coding_agent/test_codex_oauth.py
Summary
--provider codexthrough the LLM provider plugin using OAuth-backed OpenAI-compatible transportValidation
uv run ruff check src/coding_agent/oauth src/coding_agent/cli/oauth_commands.py src/coding_agent/plugins/llm_provider.py src/coding_agent/providers/openai_compat.py tests/coding_agent/test_codex_oauth.py tests/cli/test_oauth_commands.pyuv run pytest tests/coding_agent/test_codex_oauth.py tests/cli/test_oauth_commands.py tests/coding_agent/plugins/test_llm_provider.py -k "oauth or codex" -vuv run pytest tests/coding_agent/plugins/test_llm_provider.py tests/coding_agent/test_codex_oauth.py -vuv run pytest tests/cli/ -vuv run python -m coding_agent --helpNotes
Initial implementation was delegated to the o6n coding-agent pod with
deepseek-v4-pro; final fixes, tests, and verification were completed locally after the pod stopped before test completion.Summary by CodeRabbit
coding-agent oauthcommand group for managing provider authentication with login, status, refresh, logout, and health check commands.