mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(auth): parse OpenAI nested error shape in Codex token refresh
OpenAI's OAuth token endpoint returns errors in a nested shape —
{"error": {"code": "refresh_token_reused", "message": "..."}} —
not the OAuth spec's flat {"error": "...", "error_description": "..."}.
The existing parser only handled the flat shape, so:
- `err.get("error")` returned a dict, the `isinstance(str)` guard
rejected it, and `code` stayed `"codex_refresh_failed"`.
- The dedicated `refresh_token_reused` branch (with its actionable
"re-run codex + hermes auth" message and `relogin_required=True`)
never fired.
- Users saw the generic "Codex token refresh failed with status 401"
when another Codex client (CLI, VS Code extension) had consumed
their single-use refresh token — giving no hint that re-auth was
required.
Parse both shapes, mapping OpenAI's nested `code`/`type` onto the
existing `code` variable so downstream branches (`refresh_token_reused`,
`invalid_grant`, etc.) fire correctly.
Add regression tests covering:
- nested `refresh_token_reused` → actionable message + relogin_required
- nested generic code → code + message surfaced
- flat OAuth-spec `invalid_grant` still handled (back-compat)
- unparseable body → generic fallback message, relogin_required=False
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
227afcd80f
commit
f76df30e08
2 changed files with 136 additions and 6 deletions
|
|
@ -17,6 +17,7 @@ from hermes_cli.auth import (
|
|||
_import_codex_cli_tokens,
|
||||
get_codex_auth_status,
|
||||
get_provider_auth_state,
|
||||
refresh_codex_oauth_pure,
|
||||
resolve_codex_runtime_credentials,
|
||||
resolve_provider,
|
||||
)
|
||||
|
|
@ -190,3 +191,123 @@ def test_resolve_returns_hermes_auth_store_source(tmp_path, monkeypatch):
|
|||
assert creds["source"] == "hermes-auth-store"
|
||||
assert creds["provider"] == "openai-codex"
|
||||
assert creds["base_url"] == DEFAULT_CODEX_BASE_URL
|
||||
|
||||
|
||||
class _StubHTTPResponse:
|
||||
def __init__(self, status_code: int, payload):
|
||||
self.status_code = status_code
|
||||
self._payload = payload
|
||||
self.text = json.dumps(payload) if isinstance(payload, (dict, list)) else str(payload)
|
||||
|
||||
def json(self):
|
||||
if isinstance(self._payload, Exception):
|
||||
raise self._payload
|
||||
return self._payload
|
||||
|
||||
|
||||
class _StubHTTPClient:
|
||||
def __init__(self, response):
|
||||
self._response = response
|
||||
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, *args):
|
||||
return False
|
||||
|
||||
def post(self, *args, **kwargs):
|
||||
return self._response
|
||||
|
||||
|
||||
def _patch_httpx(monkeypatch, response):
|
||||
def _factory(*args, **kwargs):
|
||||
return _StubHTTPClient(response)
|
||||
|
||||
monkeypatch.setattr("hermes_cli.auth.httpx.Client", _factory)
|
||||
|
||||
|
||||
def test_refresh_parses_openai_nested_error_shape_refresh_token_reused(monkeypatch):
|
||||
"""OpenAI returns {"error": {"code": "refresh_token_reused", "message": "..."}}
|
||||
— parser must surface relogin_required and the dedicated message.
|
||||
"""
|
||||
response = _StubHTTPResponse(
|
||||
401,
|
||||
{
|
||||
"error": {
|
||||
"message": "Your refresh token has already been used to generate a new access token. Please try signing in again.",
|
||||
"type": "invalid_request_error",
|
||||
"param": None,
|
||||
"code": "refresh_token_reused",
|
||||
}
|
||||
},
|
||||
)
|
||||
_patch_httpx(monkeypatch, response)
|
||||
|
||||
with pytest.raises(AuthError) as exc_info:
|
||||
refresh_codex_oauth_pure("access-old", "refresh-old")
|
||||
|
||||
err = exc_info.value
|
||||
assert err.code == "refresh_token_reused"
|
||||
assert err.relogin_required is True
|
||||
# The existing dedicated branch should override the message with actionable guidance.
|
||||
assert "already consumed by another client" in str(err)
|
||||
|
||||
|
||||
def test_refresh_parses_openai_nested_error_shape_generic_code(monkeypatch):
|
||||
"""Nested error with arbitrary code still surfaces code + message."""
|
||||
response = _StubHTTPResponse(
|
||||
400,
|
||||
{
|
||||
"error": {
|
||||
"message": "Invalid client credentials.",
|
||||
"type": "invalid_request_error",
|
||||
"code": "invalid_client",
|
||||
}
|
||||
},
|
||||
)
|
||||
_patch_httpx(monkeypatch, response)
|
||||
|
||||
with pytest.raises(AuthError) as exc_info:
|
||||
refresh_codex_oauth_pure("access-old", "refresh-old")
|
||||
|
||||
err = exc_info.value
|
||||
assert err.code == "invalid_client"
|
||||
assert "Invalid client credentials." in str(err)
|
||||
|
||||
|
||||
def test_refresh_parses_oauth_spec_flat_error_shape_invalid_grant(monkeypatch):
|
||||
"""Fallback path: OAuth spec-shape {"error": "invalid_grant", "error_description": "..."}
|
||||
must still map to relogin_required=True via the existing code set.
|
||||
"""
|
||||
response = _StubHTTPResponse(
|
||||
400,
|
||||
{
|
||||
"error": "invalid_grant",
|
||||
"error_description": "Refresh token is expired or revoked.",
|
||||
},
|
||||
)
|
||||
_patch_httpx(monkeypatch, response)
|
||||
|
||||
with pytest.raises(AuthError) as exc_info:
|
||||
refresh_codex_oauth_pure("access-old", "refresh-old")
|
||||
|
||||
err = exc_info.value
|
||||
assert err.code == "invalid_grant"
|
||||
assert err.relogin_required is True
|
||||
assert "Refresh token is expired or revoked." in str(err)
|
||||
|
||||
|
||||
def test_refresh_falls_back_to_generic_message_on_unparseable_body(monkeypatch):
|
||||
"""No JSON body → generic 'with status 401' message; 401 always forces relogin."""
|
||||
response = _StubHTTPResponse(401, ValueError("not json"))
|
||||
_patch_httpx(monkeypatch, response)
|
||||
|
||||
with pytest.raises(AuthError) as exc_info:
|
||||
refresh_codex_oauth_pure("access-old", "refresh-old")
|
||||
|
||||
err = exc_info.value
|
||||
assert err.code == "codex_refresh_failed"
|
||||
# 401/403 from the token endpoint always means the refresh token is
|
||||
# invalid/expired — force relogin even without a parseable error body.
|
||||
assert err.relogin_required is True
|
||||
assert "status 401" in str(err)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue