From 91ce8fc000deaa4b4bbf1edb5b3d9f6dd1668f09 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 5 May 2026 04:07:49 -0700 Subject: [PATCH] fix(setup): offer Keep/Replace/Clear when API key already exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hermes setup / hermes model used to silently skip the key prompt when any value was present in .env — even a malformed paste — leaving users with a stuck '✓' and no way to recover without hand-editing .env. Replace the silent acknowledgement at all three API-key provider flows (Kimi, Stepfun, generic) with a single [K]eep / [R]eplace / [C]lear menu via a shared `_prompt_api_key` helper. - K / Enter / Ctrl-C / unknown input → keep (never destroys the key) - R → getpass for new key; empty input cancels and preserves existing - C → clears the env var, tells user to rerun hermes setup, aborts flow LM Studio's no-auth-placeholder substitution stays on first-time entry only; on Replace an empty input means 'cancel', not 'overwrite with dummy key'. 11 unit tests cover all branches incl. garbage-input-keeps-key, Ctrl-C at the choice prompt, Replace-cancel preserving the old key, Clear wiping only the target env var, and lmstudio placeholder semantics. Fixes #16394 Reshapes #18355 — original PR pasted the menu inline at 3 sites with no tests; this consolidates to one helper (+88/-66) with coverage. Co-authored-by: Feranmi10 <89228157+Feranmi10@users.noreply.github.com> --- hermes_cli/main.py | 154 +++++++++++++---------- tests/hermes_cli/test_prompt_api_key.py | 157 ++++++++++++++++++++++++ 2 files changed, 245 insertions(+), 66 deletions(-) create mode 100644 tests/hermes_cli/test_prompt_api_key.py diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 2f10d3f471..4e709a8f83 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -3974,6 +3974,85 @@ def _model_flow_copilot_acp(config, current_model=""): print(f"Default model set to: {selected} (via {pconfig.name})") +def _prompt_api_key(pconfig, existing_key: str, provider_id: str = "") -> tuple: + """Shared API-key entry point for ``hermes setup`` / ``hermes model``. + + Handles both first-time entry and the already-configured case. When a key + is already present, offers [K]eep / [R]eplace / [C]lear so the user can + recover from a malformed paste without editing ``~/.hermes/.env`` by hand. + + Returns ``(resolved_key, abort)``. ``abort=True`` means the caller should + ``return`` immediately — the user cancelled entry, declined to replace, or + cleared the key and is now unconfigured. + """ + import getpass + + from hermes_cli.auth import LMSTUDIO_NOAUTH_PLACEHOLDER + from hermes_cli.config import save_env_value + + key_env = pconfig.api_key_env_vars[0] if pconfig.api_key_env_vars else "" + + def _prompt_new_key(*, allow_lmstudio_default: bool) -> str: + if provider_id == "lmstudio" and allow_lmstudio_default: + prompt = f"{key_env} (Enter for no-auth default {LMSTUDIO_NOAUTH_PLACEHOLDER!r}): " + else: + prompt = f"{key_env} (or Enter to cancel): " + try: + entered = getpass.getpass(prompt).strip() + except (KeyboardInterrupt, EOFError): + print() + return "" + if not entered and provider_id == "lmstudio" and allow_lmstudio_default: + return LMSTUDIO_NOAUTH_PLACEHOLDER + return entered + + # First-time entry ──────────────────────────────────────────────────── + if not existing_key: + print(f"No {pconfig.name} API key configured.") + if not key_env: + return "", True + new_key = _prompt_new_key(allow_lmstudio_default=True) + if not new_key: + print("Cancelled.") + return "", True + save_env_value(key_env, new_key) + print("API key saved.") + print() + return new_key, False + + # Already configured — offer K / R / C ──────────────────────────────── + print(f" {pconfig.name} API key: {existing_key[:8]}... ✓") + if not key_env: + # Nothing we can rewrite; just acknowledge and move on. + print() + return existing_key, False + try: + choice = input(" [K]eep / [R]eplace / [C]lear (default K): ").strip().lower() + except (KeyboardInterrupt, EOFError): + print() + choice = "k" + + if choice.startswith("r"): + new_key = _prompt_new_key(allow_lmstudio_default=False) + if not new_key: + print(" No change.") + print() + return existing_key, False + save_env_value(key_env, new_key) + print(" API key updated.") + print() + return new_key, False + + if choice.startswith("c"): + save_env_value(key_env, "") + print(f" API key cleared. Re-run `hermes setup` to configure {pconfig.name} again.") + return "", True + + # Keep (default, or any other input) + print() + return existing_key, False + + def _model_flow_kimi(config, current_model=""): """Kimi / Moonshot model selection with automatic endpoint routing. @@ -4008,26 +4087,9 @@ def _model_flow_kimi(config, current_model=""): if existing_key: break - if not existing_key: - print(f"No {pconfig.name} API key configured.") - if key_env: - try: - import getpass - - new_key = getpass.getpass(f"{key_env} (or Enter to cancel): ").strip() - except (KeyboardInterrupt, EOFError): - print() - return - if not new_key: - print("Cancelled.") - return - save_env_value(key_env, new_key) - existing_key = new_key - print("API key saved.") - print() - else: - print(f" {pconfig.name} API key: {existing_key[:8]}... ✓") - print() + existing_key, abort = _prompt_api_key(pconfig, existing_key, provider_id=provider_id) + if abort: + return # Step 2: Auto-detect endpoint from key prefix is_coding_plan = existing_key.startswith("sk-kimi-") @@ -4128,25 +4190,9 @@ def _model_flow_stepfun(config, current_model=""): if existing_key: break - if not existing_key: - print(f"No {pconfig.name} API key configured.") - if key_env: - try: - import getpass - new_key = getpass.getpass(f"{key_env} (or Enter to cancel): ").strip() - except (KeyboardInterrupt, EOFError): - print() - return - if not new_key: - print("Cancelled.") - return - save_env_value(key_env, new_key) - existing_key = new_key - print("API key saved.") - print() - else: - print(f" {pconfig.name} API key: {existing_key[:8]}... ✓") - print() + existing_key, abort = _prompt_api_key(pconfig, existing_key, provider_id=provider_id) + if abort: + return current_base = "" if base_url_env: @@ -4522,33 +4568,9 @@ def _model_flow_api_key_provider(config, provider_id, current_model=""): if existing_key: break - if not existing_key: - print(f"No {pconfig.name} API key configured.") - if key_env: - try: - import getpass - - if provider_id == "lmstudio": - prompt = f"{key_env} (Enter for no-auth default {LMSTUDIO_NOAUTH_PLACEHOLDER!r}): " - else: - prompt = f"{key_env} (or Enter to cancel): " - new_key = getpass.getpass(prompt).strip() - except (KeyboardInterrupt, EOFError): - print() - return - if not new_key: - if provider_id == "lmstudio": - new_key = LMSTUDIO_NOAUTH_PLACEHOLDER - else: - print("Cancelled.") - return - save_env_value(key_env, new_key) - existing_key = new_key - print("API key saved.") - print() - else: - print(f" {pconfig.name} API key: {existing_key[:8]}... ✓") - print() + existing_key, abort = _prompt_api_key(pconfig, existing_key, provider_id=provider_id) + if abort: + return # Gemini free-tier gate: free-tier daily quotas (<= 250 RPD for Flash) # are exhausted in a handful of agent turns, so refuse to wire up the diff --git a/tests/hermes_cli/test_prompt_api_key.py b/tests/hermes_cli/test_prompt_api_key.py new file mode 100644 index 0000000000..39be8faa91 --- /dev/null +++ b/tests/hermes_cli/test_prompt_api_key.py @@ -0,0 +1,157 @@ +"""Tests for ``_prompt_api_key`` — the shared Keep/Replace/Clear menu used by +``hermes setup`` / ``hermes model`` when an API key already exists in ``.env``. + +Regression coverage for #16394: the wizard used to silently skip the key prompt +when any value was present (even malformed junk), leaving users stuck. +""" +from __future__ import annotations + +from pathlib import Path +from unittest.mock import patch + +import pytest + + +@pytest.fixture +def profile_env(tmp_path, monkeypatch): + home = tmp_path / ".hermes" + home.mkdir() + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.setenv("HERMES_HOME", str(home)) + (home / ".env").write_text("") + return home + + +def _pconfig(name="deepseek"): + from hermes_cli.auth import PROVIDER_REGISTRY + return PROVIDER_REGISTRY[name] + + +def _run_prompt(existing_key, choice, new_key="", provider_id="", pconfig_name="deepseek"): + """Invoke _prompt_api_key with mocked input()/getpass() responses.""" + from hermes_cli import main as m + + pconfig = _pconfig(pconfig_name) + with patch("builtins.input", return_value=choice), \ + patch("getpass.getpass", return_value=new_key): + return m._prompt_api_key(pconfig, existing_key, provider_id=provider_id) + + +# First-time entry ──────────────────────────────────────────────────────────── + +def test_first_time_save_new_key(profile_env): + from hermes_cli.config import get_env_value + + key, abort = _run_prompt(existing_key="", choice="", new_key="sk-abcdef") + assert key == "sk-abcdef" + assert abort is False + assert get_env_value("DEEPSEEK_API_KEY") == "sk-abcdef" + + +def test_first_time_cancelled(profile_env): + key, abort = _run_prompt(existing_key="", choice="", new_key="") + assert key == "" + assert abort is True + + +# Already configured — K / R / C ─────────────────────────────────────────────── + +def test_keep_default_empty_input(profile_env): + from hermes_cli.config import save_env_value + save_env_value("DEEPSEEK_API_KEY", "sk-existing") + + key, abort = _run_prompt(existing_key="sk-existing", choice="") + assert key == "sk-existing" + assert abort is False + + +def test_keep_letter_k(profile_env): + key, abort = _run_prompt(existing_key="sk-existing", choice="k") + assert key == "sk-existing" + assert abort is False + + +def test_keep_on_unrecognised_input(profile_env): + """Garbage input falls through to keep — never destroys the user's key.""" + key, abort = _run_prompt(existing_key="sk-existing", choice="xyz") + assert key == "sk-existing" + assert abort is False + + +def test_replace_saves_new_key(profile_env): + from hermes_cli.config import get_env_value, save_env_value + save_env_value("DEEPSEEK_API_KEY", "sk-malformed-junk") + + key, abort = _run_prompt( + existing_key="sk-malformed-junk", choice="r", new_key="sk-fresh" + ) + assert key == "sk-fresh" + assert abort is False + assert get_env_value("DEEPSEEK_API_KEY") == "sk-fresh" + + +def test_replace_cancelled_preserves_key(profile_env): + """Empty entry to the Replace prompt means cancel — keeps the old key intact.""" + from hermes_cli.config import get_env_value, save_env_value + save_env_value("DEEPSEEK_API_KEY", "sk-existing") + + key, abort = _run_prompt( + existing_key="sk-existing", choice="r", new_key="" + ) + assert key == "sk-existing" + assert abort is False + assert get_env_value("DEEPSEEK_API_KEY") == "sk-existing" + + +def test_clear_wipes_env_and_aborts(profile_env): + from hermes_cli.config import get_env_value, save_env_value + save_env_value("DEEPSEEK_API_KEY", "sk-existing") + save_env_value("OTHER_VAR", "keep-me") + + key, abort = _run_prompt(existing_key="sk-existing", choice="c") + assert key == "" + assert abort is True + # Cleared, but sibling entries untouched. + assert not get_env_value("DEEPSEEK_API_KEY") + assert get_env_value("OTHER_VAR") == "keep-me" + + +def test_ctrl_c_at_choice_prompt_keeps(profile_env): + from hermes_cli import main as m + + pconfig = _pconfig("deepseek") + with patch("builtins.input", side_effect=KeyboardInterrupt): + key, abort = m._prompt_api_key(pconfig, "sk-existing") + assert key == "sk-existing" + assert abort is False + + +# LM Studio no-auth placeholder ──────────────────────────────────────────────── + +def test_lmstudio_first_time_empty_uses_placeholder(profile_env): + from hermes_cli.auth import LMSTUDIO_NOAUTH_PLACEHOLDER + from hermes_cli.config import get_env_value + + key, abort = _run_prompt( + existing_key="", choice="", new_key="", + provider_id="lmstudio", pconfig_name="lmstudio", + ) + assert key == LMSTUDIO_NOAUTH_PLACEHOLDER + assert abort is False + assert get_env_value("LM_API_KEY") == LMSTUDIO_NOAUTH_PLACEHOLDER + + +def test_lmstudio_replace_empty_does_not_overwrite_with_placeholder(profile_env): + """On REPLACE with empty input, preserve the user's existing key — do NOT + silently substitute the placeholder. The placeholder path only fires for + first-time configuration where the user has made no explicit choice yet.""" + from hermes_cli.config import get_env_value, save_env_value + save_env_value("LM_API_KEY", "my-real-lmstudio-key") + + key, abort = _run_prompt( + existing_key="my-real-lmstudio-key", choice="r", new_key="", + provider_id="lmstudio", pconfig_name="lmstudio", + ) + assert key == "my-real-lmstudio-key" + assert abort is False + assert get_env_value("LM_API_KEY") == "my-real-lmstudio-key"