fix(web_server): stop Codex OAuth worker from finishing after cancel

[JoaoMarcos44]

Summary

Fixes IA-01 (closes #73912): cancelling a pending OpenAI Codex device-code OAuth login did not stop the background worker. If the user approved the device code at OpenAI after clicking Cancel, the worker still exchanged the code and saved tokens — and could save them into the wrong profile, since profile resolution fell back to the caller's current active profile once the session entry was gone.

Root cause

• DELETE /api/providers/oauth/sessions/{session_id} (cancel_oauth_session) only popped the session from _oauth_sessions — it never signalled the worker.
• _codex_full_login_worker never re-checked cancellation during its poll loop.
• The save-time profile lookup (_oauth_session_profile(session_id)) ran after the session was gone, returning None → _profile_scope(None) silently fell back to the current active profile.

Fix

• cancel_oauth_session now sets sess["cancelled"] = True on the shared session dict before popping it, so the worker (which holds a reference to that same dict object) can observe it.
• _codex_full_login_worker captures session_profile = sess.get("profile") once, right after publishing the user_code — never re-derived later, so a cancelled/popped session can no longer fall back to the caller's current profile scope.
• The worker checks sess.get("cancelled") before/after each poll sleep, before the authorization-code exchange, and immediately before calling _save_codex_tokens(). _save_codex_tokens() itself is unchanged — the guard lives entirely in the worker's call sites.

Infographic

Test plan

• test_codex_dashboard_worker_stops_polling_after_cancel — new: simulates a concurrent DELETE firing mid-poll, asserts _save_codex_tokens is never called and status stays "pending".
• test_cancel_oauth_session_marks_dict_cancelled_before_popping — new: asserts the DELETE endpoint mutates the shared dict's cancelled flag before removing it from _oauth_sessions.
• Full suite: pytest tests/hermes_cli/test_web_oauth_dispatch.py — 26/26 passing.
• Confirmed the one unrelated failure in tests/hermes_cli/test_web_server_oauth_write.py::test_dashboard_oauth_write_uses_owner_only_permissions pre-exists on main (Windows doesn't enforce POSIX chmod 0o600) via git stash + re-run — unrelated to this change.

Out of scope

This PR only addresses IA-01. Other findings from the same investigation (fallback secret-scope isolation, Azure hostname classification, compression race, etc.) are tracked separately and not part of this change.

[teknium1]

Thanks for targeting a real current-main OAuth/profile-isolation bug.

Problems

• The final guard at hermes_cli/web_server.py:11392 is not synchronized with _save_codex_tokens() at :11398-11402. DELETE can set cancellation and return success after that read but before persistence, so the stated no-write-after-cancel guarantee still does not hold. This is also the gap identified in the #73912 discussion.
• The endpoint pops before it sets cancelled at hermes_cli/web_server.py:11516-11518, despite the new docstring claiming the reverse. The worker reads that field outside the lock.
• The new worker test directly mutates the dict at tests/hermes_cli/test_web_oauth_dispatch.py:443-446; it does not exercise DELETE/removal or force the final-check-to-save interleaving.
• The shared endpoint leaves analogous profile re-resolution paths in the Nous, MiniMax, and xAI pollers (hermes_cli/web_server.py:10410, :10500, :10550).

Suggested changes

• Define an atomic cancellation/persistence boundary and add a deterministic real-DELETE regression test at that boundary.
• Audit or separately track the sibling device-code pollers.

Automated hermes-sweeper review.

[egilewski]

suggesting changes

The PR closes the user-initiated DELETE race for the OpenAI Codex device-code worker: it preserves the originating profile, exposes cancellation through the retained session object, and makes the final cancellation check and credential save atomic. A source-extracted runtime probe reproduced the current-main token save after DELETE and showed that PR head suppresses it. However, the same session-removal invariant is still bypassed by TTL garbage collection: _gc_oauth_sessions() pops an expired session without marking the retained dict cancelled, and the PR-head worker can subsequently exchange and persist tokens for that removed/expired session. This is an in-scope residual lifecycle path and should be fixed before merge.

• [P3] TTL cleanup still lets an expired Codex OAuth worker persist credentials (hermes_cli/web_server.py:9939)
  _gc_oauth_sessions() removes stale entries with a bare pop, unlike the corrected DELETE path which first sets sess["cancelled"] = True. The Codex worker retains the same dict after initialization and its new checks only inspect that flag. Because the worker's 15-minute polling deadline starts after the initial device-code request while GC ages from session creation, there is a real interval in which a later authenticated /start call can GC the session while its worker is still polling. If authorization completes afterward, the final locked check sees no cancellation flag and _save_codex_tokens() persists credentials even though the session was removed as expired. A source-extracted PR-head probe forced that timing and observed saved=True with the session absent from _oauth_sessions.
  Remediation: Under _oauth_sessions_lock, mark every stale session dict cancelled before removing it, preferably through one helper shared by DELETE and GC. Add a regression test that expires a Codex session, invokes _gc_oauth_sessions() while the worker is between polls, and asserts that neither token exchange nor _save_codex_tokens() occurs afterward.

Security evidence:

• trust boundary: The boundary is between a token-protected dashboard request that starts or cancels an in-memory OAuth session, untrusted OpenAI device/token endpoint responses processed by a daemon worker, and the persistent profile-scoped auth.json credential sink. Session IDs and mutable session dicts coordinate the request thread and worker; _oauth_sessions_lock is the concurrency validator. Profile names enter at /start, are validated by _validate_oauth_profile(), normalized into the session, and must remain bound to the eventual credential write.
• source/sink/invariant: After a session is cancelled or expired and removed, its retained worker reference must not poll, exchange, or persist OAuth tokens. Any credential save must use the profile captured from the validated originating session, and the last cancellation/expiry decision must be serialized with _save_codex_tokens(). PR head establishes this for DELETE by flagging before pop and checking under the same lock as save, but GC violates the same invariant by popping without flagging.
• current-main reproduction: A standard-library AST harness loaded the actual _codex_full_login_worker, cancel_oauth_session, and session helpers from current main 4b60979dc188655eb4fb81abf292890147ec2d4c, supplied deterministic successful provider responses, and invoked the actual cancel handler during the poll sleep. It observed saved=True after the session had been popped, reproducing the reported current-main race without network access.
• PR-head or patch-replay validation: The same harness loaded the actual functions from reviewed head 5aa7594993a4a2c475481c9f61c56651817ac250; DELETE during the poll sleep produced saved=False, confirming the intended fix. The PR patch from merge base ce6dd1a65f4b6b20b1f3b31f75184a3e26583488 was also applied to an archived current-main snapshot in the authorized scratch root, and both changed files compared byte-for-byte equal to reviewed head, establishing coherent replay on current main.
• positive/negative cases: Positive case: current main with successful device authorization after DELETE persisted tokens. Corrected negative case: PR head with DELETE during sleep removed the session and persisted nothing. Residual negative case: PR head with TTL GC during sleep removed the session but still persisted tokens (saved=True). Source tests also cover DELETE flag propagation and the legitimate point-of-no-return case where DELETE blocks behind an already-started atomic save, but the local environment lacked pytest dependencies so those repository tests were not executed.
• residual bypass search: Reviewed every _oauth_sessions mutation and the Codex poll/exchange/save checkpoints, plus profile resolution and auth-store locking. The search identified _gc_oauth_sessions() as a second removal source that bypasses the new cancellation marker. Races before worker initialization return safely because the worker re-reads the registry under lock; races after provider authorization are stopped at the final locked check for DELETE. The auth-store lock has a bounded timeout, and no source-backed lock-order inversion was found between _oauth_sessions_lock, _SKILLS_PROFILE_LOCK, and the profile auth-store lock.
• reviewer validation: Validated the diff against merge base and current main, inspected the exact save/profile/lock implementations and original worker lifecycle, ran git diff --check, performed byte-equal patch replay in scratch, and ran the source-extracted current-main/PR-head concurrency probe. Python compilation succeeded for both modified files. Focused pytest invocations were attempted but could not run because the leased checkout has no pytest-capable virtual environment; no external reviewer or network service was used, as required by the work order.

Uncertainty: The repository's pytest suite could not be executed locally because no pytest-capable virtual environment is available in the leased checkout or configured runner environment.; No live OpenAI OAuth exchange was performed; provider behavior was deterministically simulated because network access is prohibited.; The maximum real-world GC race window depends on initial device-code request latency and scheduler timing, although the source establishes that it can be non-zero.

Signed: GPT-5.6-sol-xhigh in Codex