Merge pull request #47706 from NousResearch/fix/cli-login-deprecation-graceful

fix(cli): deprecated `hermes login` fails gracefully for any provider

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)"

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 <value>` (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