diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 8c81b21ba0621..1180d904e57dc 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -11325,13 +11325,23 @@ def _codex_full_login_worker(session_id: str) -> None: sess["interval"] = poll_interval sess["expires_in"] = 15 * 60 # OpenAI's effective limit sess["expires_at"] = time.time() + sess["expires_in"] + # Captured now (not re-derived after cancel pops the session) so a + # cancelled session can never fall back to the caller's current + # profile scope at save time. + session_profile = sess.get("profile") # Step 2: poll until authorized deadline = time.monotonic() + sess["expires_in"] code_resp = None with httpx.Client(timeout=httpx.Timeout(15.0)) as client: while time.monotonic() < deadline: + if sess.get("cancelled"): + _log.info("oauth/device: openai-codex login cancelled (session=%s)", session_id) + return time.sleep(poll_interval) + if sess.get("cancelled"): + _log.info("oauth/device: openai-codex login cancelled (session=%s)", session_id) + return poll = client.post( f"{issuer}/api/accounts/deviceauth/token", json={"device_auth_id": device_auth_id, "user_code": user_code}, @@ -11350,6 +11360,10 @@ def _codex_full_login_worker(session_id: str) -> None: sess["error_message"] = "Device code expired before approval" return + if sess.get("cancelled"): + _log.info("oauth/device: openai-codex login cancelled before token exchange (session=%s)", session_id) + return + # Step 3: exchange authorization_code for tokens authorization_code = code_resp.get("authorization_code", "") code_verifier = code_resp.get("code_verifier", "") @@ -11375,9 +11389,13 @@ def _codex_full_login_worker(session_id: str) -> None: if not access_token: raise RuntimeError("token exchange did not return access_token") + if sess.get("cancelled"): + _log.info("oauth/device: openai-codex login cancelled before token save (session=%s)", session_id) + return + from hermes_cli.auth import _save_codex_tokens - with _profile_scope(_oauth_session_profile(session_id)): + with _profile_scope(session_profile): _save_codex_tokens({ "access_token": access_token, "refresh_token": refresh_token, @@ -11485,10 +11503,19 @@ async def cancel_oauth_session( request: Request, profile: Optional[str] = None, ): - """Cancel a pending OAuth session. Token-protected.""" + """Cancel a pending OAuth session. Token-protected. + + Marks the session dict ``cancelled`` before popping it so any + background worker still holding a reference to that same dict (e.g. + the Codex device-code poller) observes the cancellation and stops + polling/exchanging/saving instead of completing the login after the + user believed it was aborted. + """ _require_token(request) with _oauth_sessions_lock: sess = _oauth_sessions.pop(session_id, None) + if sess is not None: + sess["cancelled"] = True 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 a8ec8c669c98c..ed06ba3b1f01f 100644 --- a/tests/hermes_cli/test_web_oauth_dispatch.py +++ b/tests/hermes_cli/test_web_oauth_dispatch.py @@ -391,6 +391,103 @@ def test_codex_dashboard_start_rewords_device_authorization_error(monkeypatch): ws._oauth_sessions.pop(sid, None) +def test_codex_dashboard_worker_stops_polling_after_cancel(tmp_path, monkeypatch): + """DELETE mid-poll must stop the worker before it exchanges/saves tokens. + + Regression for IA-01: cancelling only popped the session dict; the + background worker kept polling/exchanging/saving regardless, and once + the session was gone `_oauth_session_profile()` fell back to the + caller's current profile scope instead of the one the login started + in. The fix marks the dict `cancelled` before popping, and the worker + checks that flag before every remaining step. + """ + from hermes_cli import auth as auth_mod + from hermes_cli import web_server as ws + + 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", + }) + raise AssertionError( + f"worker must stop before calling {url} once cancelled" + ) + + saved = [] + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setattr(httpx, "Client", _Client) + monkeypatch.setattr(auth_mod, "_save_codex_tokens", lambda tokens: saved.append(tokens)) + + sid, _ = ws._new_oauth_session("openai-codex", "device_code", profile="coder") + + def fake_sleep(_interval): + # Simulate a concurrent DELETE /api/providers/oauth/sessions/{sid} + # firing while the worker is asleep between polls. + ws._oauth_sessions[sid]["cancelled"] = True + + monkeypatch.setattr(ws.time, "sleep", fake_sleep) + + try: + ws._codex_full_login_worker(sid) + + assert saved == [] + assert ws._oauth_sessions[sid]["status"] == "pending" + finally: + ws._oauth_sessions.pop(sid, None) + + +def test_cancel_oauth_session_marks_dict_cancelled_before_popping(): + """The DELETE endpoint must flag the session dict before removing it. + + A background worker holds its own reference to the same dict object; + it can only observe cancellation if the flag is set on that shared + object prior to (or instead of) removal from the global session map. + """ + from hermes_cli import web_server as ws + + session_id = "cancel-flag-test" + ws._oauth_sessions[session_id] = { + "session_id": session_id, + "provider": "openai-codex", + "flow": "device_code", + "profile": "coder", + "created_at": time.time(), + "status": "pending", + "error_message": None, + } + worker_ref = ws._oauth_sessions[session_id] + + resp = client.delete( + f"/api/providers/oauth/sessions/{session_id}", + headers=HEADERS, + ) + + assert resp.status_code == 200, resp.text + assert resp.json() == {"ok": True, "session_id": session_id} + assert session_id not in ws._oauth_sessions + assert worker_ref["cancelled"] is True + + def test_nous_dashboard_poller_preserves_effective_scope_when_token_omits_scope(monkeypatch): from hermes_cli import auth as auth_mod from hermes_cli import web_server as ws