diff --git a/hermes_cli/subcommands/login.py b/hermes_cli/subcommands/login.py index efc91e8924e..c5837e9aa35 100644 --- a/hermes_cli/subcommands/login.py +++ b/hermes_cli/subcommands/login.py @@ -10,20 +10,40 @@ from typing import Callable def build_login_parser(subparsers, *, cmd_login: Callable) -> None: - """Attach the ``login`` subcommand to ``subparsers``.""" - # ========================================================================= - # login command - # ========================================================================= + """Attach the deprecated ``login`` subcommand to ``subparsers``. + + ``hermes login`` was removed in favor of ``hermes auth`` / ``hermes model`` + (the runtime handler in ``hermes_cli/auth.py::login_command`` just prints a + deprecation message and exits). The subparser is kept registered so that + old scripts/aliases invoking ``hermes login [--flags]`` still receive the + actionable deprecation message rather than an argparse ``invalid choice: + 'login'`` error — but: + + - The subparser is registered WITHOUT a ``help=`` kwarg so the row is + omitted from ``hermes --help`` (argparse only lists subcommands that + have a help string). This hides a command that no longer works (#24756) + without the ``help=argparse.SUPPRESS`` ``==SUPPRESS==`` leak that + argparse emits for a top-level subparser on Python 3.12+. + - ``--provider`` accepts ANY value (no ``choices=``) so that, e.g., + ``hermes login --provider anthropic`` reaches the deprecation handler and + gets pointed at ``hermes model`` instead of crashing in argparse with + ``invalid choice: 'anthropic'`` before the handler can run. + """ login_parser = subparsers.add_parser( "login", - help="Authenticate with an inference provider", - description="Run OAuth device authorization flow for Hermes CLI", + description=( + "Deprecated. Use `hermes auth` to manage credentials, " + "`hermes model` to select a provider, or `hermes setup` for full setup." + ), ) + # No ``choices=`` on purpose — the handler is a deprecation notice that + # ignores the value, and a restrictive list would reject providers the user + # legitimately wants (e.g. ``anthropic``) with an argparse error before the + # friendly redirect message is ever printed. login_parser.add_argument( "--provider", - choices=["nous", "openai-codex", "xai-oauth"], default=None, - help="Provider to authenticate with (default: nous)", + help="(deprecated) Provider name; ignored — see `hermes model`", ) login_parser.add_argument( "--portal-url", help="Portal base URL (default: production portal)" diff --git a/tests/hermes_cli/test_subcommands_batch.py b/tests/hermes_cli/test_subcommands_batch.py index 4fbba841fb2..f6af8d6848e 100644 --- a/tests/hermes_cli/test_subcommands_batch.py +++ b/tests/hermes_cli/test_subcommands_batch.py @@ -95,3 +95,52 @@ def test_dashboard_builder_two_handlers(): assert parser.parse_args(["dashboard"]).func is dash # dashboard register -> register handler assert parser.parse_args(["dashboard", "register"]).func is reg + + +# ── deprecated `hermes login` fails gracefully, not with argparse error ──── +# +# `hermes login` is a removed command; its handler (`login_command` in +# `hermes_cli/auth.py`) prints a deprecation notice pointing at `hermes auth` / +# `hermes model` and exits 0. Two behavior contracts guard the UX: +# 1. ANY `--provider ` (including ones the user actually wants, like +# `anthropic`) must parse and reach the handler — never crash in argparse +# with `invalid choice` before the friendly redirect is printed (#24756). +# 2. The subcommand must not advertise itself in the parser help row. + + +def _login_parser(): + parser = argparse.ArgumentParser(prog="hermes") + sub = parser.add_subparsers(dest="command") + build_login_parser(sub, cmd_login=_h("login")) + return parser + + +@pytest.mark.parametrize("provider", ["anthropic", "nous", "openai-codex", "totally-made-up"]) +def test_login_accepts_any_provider_value(provider): + """Deprecated `login` must route every `--provider` to the handler. + + A restrictive `choices=` list (the pre-fix behavior) rejected providers + like `anthropic` with an argparse error *before* the deprecation message + could run, so the user just saw `invalid choice: 'anthropic'` and assumed + the feature was broken rather than relocated. + """ + ns = _login_parser().parse_args(["login", "--provider", provider]) + assert ns.func.__name__ == "cmd_login" + assert ns.provider == provider + + +def test_login_subparser_help_is_suppressed(): + """The deprecated `login` row must not appear in `hermes --help`. + + Must hold without leaking argparse's literal `==SUPPRESS==` placeholder, + which `help=argparse.SUPPRESS` emits for a top-level subparser on 3.12+. + The fix omits the `help=` kwarg entirely instead. + """ + parser = argparse.ArgumentParser(prog="hermes") + sub = parser.add_subparsers(dest="command") + build_login_parser(sub, cmd_login=_h("login")) + help_text = parser.format_help() + # The misleading old help string must be gone from the top-level usage. + assert "Authenticate with an inference provider" not in help_text + # And no leaked SUPPRESS placeholder row. + assert "==SUPPRESS==" not in help_text