From 12c39830f0f491cb97b7519eed1faa9fd2df3483 Mon Sep 17 00:00:00 2001 From: xxxigm <54813621+xxxigm@users.noreply.github.com> Date: Tue, 19 May 2026 00:14:33 -0700 Subject: [PATCH] fix(doctor): attach codex CLI hint to OpenAI Codex auth warning for #27975 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `hermes doctor` printed 'codex CLI not installed (optional — ...)' as a generic info line at the bottom of the auth section, several rows below 'OpenAI Codex auth (not logged in)' and after MiniMax/Gemini auth checks. Users reading sequentially mistook it for MiniMax-related advice. Move the hint up under the Codex auth warning so it's adjacent to the row it actually pertains to. Behavior unchanged when the codex CLI is installed (success path keeps its 'codex CLI ✓' row at the bottom). Tests cover both placement and suppression cases. Salvage of @xxxigm's 3-commit stack (#27986). Closes #27975. --- hermes_cli/doctor.py | 22 ++++----- tests/hermes_cli/test_doctor.py | 84 +++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 12 deletions(-) diff --git a/hermes_cli/doctor.py b/hermes_cli/doctor.py index dab22e2640a..61381502511 100644 --- a/hermes_cli/doctor.py +++ b/hermes_cli/doctor.py @@ -799,6 +799,16 @@ def run_doctor(args): check_warn("OpenAI Codex auth", "(not logged in)") if codex_status.get("error"): check_info(codex_status["error"]) + # Native OAuth uses Hermes' own device-code flow — the Codex CLI is + # only needed to import existing tokens from ~/.codex/auth.json. + # Attach the hint to the Codex auth row so it doesn't read as + # remediation for whichever provider happens to print next (#27975). + if not _safe_which("codex"): + check_info( + "codex CLI not installed " + "(optional — only required to import tokens " + "from an existing Codex CLI login)" + ) gemini_status = get_gemini_oauth_auth_status() if gemini_status.get("logged_in"): @@ -837,18 +847,6 @@ def run_doctor(args): except Exception: pass - if _safe_which("codex"): - check_ok("codex CLI") - else: - # Native OAuth uses Hermes' own device-code flow — the Codex CLI is - # only needed if you want to import existing tokens from - # ~/.codex/auth.json. Downgrade to info so users running - # `hermes auth openai-codex` aren't told they're missing something. - check_info( - "codex CLI not installed " - "(optional — only required to import tokens from an existing Codex CLI login)" - ) - _section("Directory Structure") hermes_home = HERMES_HOME if hermes_home.exists(): diff --git a/tests/hermes_cli/test_doctor.py b/tests/hermes_cli/test_doctor.py index be8c35239b3..3fcb845366a 100644 --- a/tests/hermes_cli/test_doctor.py +++ b/tests/hermes_cli/test_doctor.py @@ -1223,3 +1223,87 @@ class TestDoctorXaiOAuthStatus: # None → {} → logged_in falsy → shows not-logged-in warn assert "xAI OAuth" in out assert "(not logged in)" in out + + +# --------------------------------------------------------------------------- +# ◆ Auth Providers — codex CLI import hint placement (issue #27975) +# --------------------------------------------------------------------------- + + +class TestDoctorCodexCliHintPlacement: + """The `codex CLI not installed` hint belongs under OpenAI Codex auth. + + Regression for #27975: the hint used to be emitted as a standalone block + after all auth-provider rows, so it visually attached to whichever + provider happened to print last (MiniMax OAuth in the reported repro), + reading as remediation for an unrelated provider. + """ + + def _run(self, monkeypatch, tmp_path, *, codex_logged_in: bool, codex_cli_present: bool) -> str: + home = tmp_path / ".hermes" + home.mkdir(parents=True, exist_ok=True) + (home / "config.yaml").write_text("memory: {}\n", encoding="utf-8") + project = tmp_path / "project" + project.mkdir(exist_ok=True) + + monkeypatch.setattr(doctor_mod, "HERMES_HOME", home) + monkeypatch.setattr(doctor_mod, "PROJECT_ROOT", project) + monkeypatch.setattr(doctor_mod, "_DHH", str(home)) + + fake_model_tools = types.SimpleNamespace( + check_tool_availability=lambda *a, **kw: ([], []), + TOOLSET_REQUIREMENTS={}, + ) + monkeypatch.setitem(sys.modules, "model_tools", fake_model_tools) + + from hermes_cli import auth as _auth_mod + monkeypatch.setattr(_auth_mod, "get_nous_auth_status", lambda: {"logged_in": False}) + monkeypatch.setattr(_auth_mod, "get_codex_auth_status", lambda: {"logged_in": codex_logged_in}) + monkeypatch.setattr(_auth_mod, "get_gemini_oauth_auth_status", lambda: {"logged_in": False}) + monkeypatch.setattr(_auth_mod, "get_minimax_oauth_auth_status", lambda: {"logged_in": False}) + monkeypatch.setattr(_auth_mod, "get_xai_oauth_auth_status", lambda: {"logged_in": False}) + + real_which = doctor_mod.shutil.which + monkeypatch.setattr( + doctor_mod.shutil, + "which", + lambda cmd: ("/usr/local/bin/codex" if codex_cli_present else None) if cmd == "codex" else real_which(cmd), + ) + + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + doctor_mod.run_doctor(Namespace(fix=False)) + return buf.getvalue() + + @staticmethod + def _hint_line() -> str: + return "codex CLI not installed" + + def test_hint_appears_under_codex_auth_when_missing(self, monkeypatch, tmp_path): + out = self._run(monkeypatch, tmp_path, codex_logged_in=False, codex_cli_present=False) + lines = out.splitlines() + codex_idx = next(i for i, l in enumerate(lines) if "OpenAI Codex auth" in l) + hint_idx = next(i for i, l in enumerate(lines) if self._hint_line() in l) + minimax_idx = next(i for i, l in enumerate(lines) if "MiniMax OAuth" in l) + # Hint must sit between Codex auth and the next provider row (#27975). + assert codex_idx < hint_idx < minimax_idx + + def test_hint_suppressed_when_codex_cli_present(self, monkeypatch, tmp_path): + out = self._run(monkeypatch, tmp_path, codex_logged_in=False, codex_cli_present=True) + assert "OpenAI Codex auth" in out + assert self._hint_line() not in out + + def test_hint_suppressed_when_codex_logged_in(self, monkeypatch, tmp_path): + out = self._run(monkeypatch, tmp_path, codex_logged_in=True, codex_cli_present=False) + assert "OpenAI Codex auth" in out + assert "(logged in)" in out + assert self._hint_line() not in out + + def test_hint_never_attaches_to_minimax_row(self, monkeypatch, tmp_path): + out = self._run(monkeypatch, tmp_path, codex_logged_in=False, codex_cli_present=False) + # The MiniMax OAuth row and the hint must not be adjacent — the hint + # belongs to the Codex auth row directly above it. + lines = [l for l in out.splitlines() if l.strip()] + minimax_idx = next(i for i, l in enumerate(lines) if "MiniMax OAuth" in l) + assert self._hint_line() not in lines[minimax_idx - 1] + assert minimax_idx + 1 >= len(lines) or self._hint_line() not in lines[minimax_idx + 1]