From 6fd67dd791ed92cbf3007857c0ace920f352510d Mon Sep 17 00:00:00 2001 From: joaomarcos Date: Wed, 29 Jul 2026 16:30:48 -0300 Subject: [PATCH] fix(web_server): stop codex oauth worker on cancel and pin resolved profile Cancelling an OpenAI Codex device-code login left the background poller running, and the token-write path re-resolved the target profile lazily via _oauth_session_profile(), which returned None once cancel popped the session and silently fell back to whatever profile was "current". Adds a per-session threading.Event set by the cancel endpoint. The codex worker now pins its target profile once at start (before the session can be popped), checks the cancel event before each poll iteration and again before exchanging the code and before writing tokens, and aborts without writing if cancelled. --- hermes_cli/web_server.py | 34 +++++++- tests/hermes_cli/test_web_oauth_dispatch.py | 86 +++++++++++++++++++++ 2 files changed, 119 insertions(+), 1 deletion(-) diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 4c2d76a27598..b8f0e09d3e33 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -10588,6 +10588,9 @@ def _new_oauth_session( "created_at": time.time(), "status": "pending", # pending | approved | denied | expired | error "error_message": None, + # Set by DELETE /api/providers/oauth/sessions/{id}; background + # pollers check this to abort without writing credentials. + "cancel_event": threading.Event(), } with _oauth_sessions_lock: _oauth_sessions[sid] = sess @@ -11263,6 +11266,17 @@ def _codex_full_login_worker(session_id: str) -> None: single function — we need to surface the user_code to the dashboard the moment we receive it, well before polling completes. """ + with _oauth_sessions_lock: + sess = _oauth_sessions.get(session_id) + if sess is None: + return + cancel_event: threading.Event = sess["cancel_event"] + # Pin the target profile now, while the session entry still exists. + # Cancellation pops the session from _oauth_sessions, so resolving + # the profile lazily at write time would silently fall back to + # whatever profile happens to be "current" (see _oauth_session_profile). + target_profile = sess.get("profile") + try: import httpx from hermes_cli.auth import ( @@ -11303,7 +11317,11 @@ def _codex_full_login_worker(session_id: str) -> None: code_resp = None with httpx.Client(timeout=httpx.Timeout(15.0)) as client: while time.monotonic() < deadline: + if cancel_event.is_set(): + return time.sleep(poll_interval) + if cancel_event.is_set(): + return poll = client.post( f"{issuer}/api/accounts/deviceauth/token", json={"device_auth_id": device_auth_id, "user_code": user_code}, @@ -11316,6 +11334,9 @@ def _codex_full_login_worker(session_id: str) -> None: continue # user hasn't authorized yet raise RuntimeError(f"deviceauth/token poll returned {poll.status_code}") + if cancel_event.is_set(): + return + if code_resp is None: with _oauth_sessions_lock: sess["status"] = "expired" @@ -11349,7 +11370,14 @@ def _codex_full_login_worker(session_id: str) -> None: from hermes_cli.auth import _save_codex_tokens - with _profile_scope(_oauth_session_profile(session_id)): + # Refuse to write if cancellation raced past the checks above. + if cancel_event.is_set(): + _log.info( + "oauth/device: openai-codex session %s cancelled before token write", session_id, + ) + return + + with _profile_scope(target_profile): _save_codex_tokens({ "access_token": access_token, "refresh_token": refresh_token, @@ -11456,6 +11484,10 @@ async def cancel_oauth_session( _require_token(request) with _oauth_sessions_lock: sess = _oauth_sessions.pop(session_id, None) + if sess is not None: + cancel_event = sess.get("cancel_event") + if cancel_event is not None: + cancel_event.set() if sess is None: return {"ok": False, "message": "session not found"} return {"ok": True, "session_id": session_id} diff --git a/tests/hermes_cli/test_web_oauth_dispatch.py b/tests/hermes_cli/test_web_oauth_dispatch.py index f4725f069228..d6af812f8624 100644 --- a/tests/hermes_cli/test_web_oauth_dispatch.py +++ b/tests/hermes_cli/test_web_oauth_dispatch.py @@ -341,6 +341,92 @@ def post(self, url, **kwargs): ws._oauth_sessions.pop(sid, None) +def test_codex_dashboard_worker_aborts_after_cancel(tmp_path, monkeypatch): + """DELETE /api/providers/oauth/sessions/{id} must stop the codex poll + worker before it exchanges the device code and writes tokens. + + Regression for issue #74308: cancelling mid-poll used to leave the + background worker running to completion, and — since the session dict + entry had already been popped — the eventual token write resolved its + target profile as ``None`` and fell back to whatever profile happened + to be "current" instead of the profile the session actually targeted. + """ + from hermes_cli import auth as auth_mod + from hermes_cli import web_server as ws + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + + save_calls = [] + monkeypatch.setattr( + auth_mod, + "_save_codex_tokens", + lambda tokens: save_calls.append(tokens), + ) + + class _Resp: + def __init__(self, status_code, payload): + self.status_code = status_code + self._payload = payload + + def json(self): + return self._payload + + class _Client: + def __init__(self, *args, **kwargs): + pass + + def __enter__(self): + return self + + def __exit__(self, *args): + return False + + def post(self, url, **kwargs): + if url.endswith("/deviceauth/usercode"): + return _Resp(200, { + "device_auth_id": "device-auth-id", + "interval": 3, + "user_code": "CODEX-1234", + }) + if url.endswith("/deviceauth/token"): + # The user "approves" on the very first poll — if the worker + # doesn't check cancellation, this looks like a success. + return _Resp(200, { + "authorization_code": "authorization-code", + "code_verifier": "code-verifier", + }) + return _Resp(200, { + "access_token": "codex-access-should-not-be-saved", + "refresh_token": "codex-refresh-should-not-be-saved", + }) + + monkeypatch.setattr(httpx, "Client", _Client) + + sid, _ = ws._new_oauth_session("openai-codex", "device_code", profile="coder") + + # Cancel the session the moment the worker calls time.sleep() inside the + # poll loop — simulates the user clicking "Cancel" while the worker is + # paused between poll attempts, published device code already in hand. + def fake_sleep(_seconds): + resp = client.delete( + f"/api/providers/oauth/sessions/{sid}", headers=HEADERS, + ) + assert resp.status_code == 200, resp.text + assert resp.json()["ok"] is True + + monkeypatch.setattr(ws.time, "sleep", fake_sleep) + + try: + ws._codex_full_login_worker(sid) + finally: + ws._oauth_sessions.pop(sid, None) + + # Cancellation must have removed the session before the worker exits. + assert sid not in ws._oauth_sessions + # And the worker must have aborted BEFORE persisting any credentials. + assert save_calls == [] + + def test_codex_dashboard_start_rewords_device_authorization_error(monkeypatch): from hermes_cli import web_server as ws