feat: add Codex browser OAuth fallback - #17647
Conversation
4f80168 to
2a41969
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for adding a concrete PKCE fallback for Codex accounts where device code is unavailable. Current main still routes fresh Codex logins directly to device code (hermes_cli/auth.py:6930-6939), so the feature remains relevant.
Problems
agent/credential_pool.py:619and:644call_save_provider_stateduring pool refresh. That helper always updatesactive_provider(hermes_cli/auth.py:1222-1228), so a token refresh can silently switch the user's selected provider. Keep the non-activating write path.- The branch predates current Codex refresh serialization. Main holds
_auth_store_lockacross sync, refresh POST, and write-back (agent/credential_pool.py:968-990), added byda6d5fcdto prevent reuse of single-use refresh tokens. Preserve that behavior when salvaging the Codex changes. hermes_cli/auth.py:2219,:2225, and:2227add behavioralHERMES_*overrides. AGENTS.md:102-107 requires behavioral settings to use config.yaml.
Suggested changes
- Carry the browser-PKCE implementation onto current auth/pool code without replacing current refresh and provider-selection safeguards.
- Add focused tests for the resulting current-main browser login and refresh path.
Automated hermes-sweeper review.
| @@ -704,7 +619,7 @@ def _sync_device_code_entry_to_auth_store(self, entry: PooledCredential) -> None | |||
| state[extra_key] = val | |||
There was a problem hiding this comment.
_save_provider_state sets auth_store["active_provider"] unconditionally (hermes_cli/auth.py:1222-1228). A pool refresh is token rotation, not an explicit provider selection; retain _store_provider_state(..., set_active=False) here so it cannot switch the user's active provider.
| "response_type": "code", | ||
| "client_id": CODEX_OAUTH_CLIENT_ID, | ||
| "redirect_uri": CODEX_OAUTH_REDIRECT_URI, | ||
| "scope": os.getenv("HERMES_CODEX_OAUTH_SCOPE", CODEX_OAUTH_SCOPE).strip() or CODEX_OAUTH_SCOPE, |
There was a problem hiding this comment.
Please do not add a new non-secret HERMES_* configuration surface for OAuth scope. Per AGENTS.md:102-107, behavioral settings belong in config.yaml; remove this override unless a supported config.yaml setting is added through the normal configuration flow.
Summary
Test Plan
venv/bin/python -m py_compile agent/credential_pool.py hermes_cli/auth.py hermes_cli/main.pyvenv/bin/python -m pytest tests/hermes_cli/test_auth_codex_provider.py tests/hermes_cli/test_auth_commands.py tests/hermes_cli/test_codex_models.py -o 'addopts=' -q