fix(web_server): stop Codex OAuth worker from finishing after cancel
Cancelling a pending OpenAI Codex device-code login only popped the session dict; the background worker had no way to observe the cancellation and kept polling, exchanging the code, and saving tokens regardless. Once the session was gone, _oauth_session_profile() returned None and the save fell back to the caller's current profile scope instead of the profile the login was started in. Fix: cancel_oauth_session marks the dict cancelled=True before popping it, and _codex_full_login_worker (which holds a reference to the same dict object) checks that flag before every remaining sleep/poll, before the token exchange, and before saving. The profile is captured once up front so it can never be re-derived from a session that no longer exists.
This commit is contained in:
parent
0f64557c06
commit
a6c0803f59
|
|
@ -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}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue