From a7ec3344488320eeb63fd1d0e669f6dfb44abcd3 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Wed, 17 Jun 2026 12:55:40 +0530 Subject: [PATCH] fix(cli): deprecated `hermes login` fails gracefully for any provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `hermes login` was removed in favor of `hermes auth` / `hermes model`, but the subparser still validated `--provider` against a hardcoded choices list (nous, openai-codex, xai-oauth). Running `hermes login --provider anthropic` therefore crashed in argparse with `invalid choice: 'anthropic'` *before* the deprecation handler could print the redirect to `hermes model` — so a user trying to authenticate a perfectly valid provider just saw a hard error and assumed the feature was broken rather than relocated. - Drop the restrictive `choices=` so every `--provider` value reaches the deprecation handler (which ignores the value and prints guidance). - Omit the subparser `help=` kwarg so the dead command no longer advertises itself in `hermes --help` (#24756). Avoids the `==SUPPRESS==` placeholder leak that `help=argparse.SUPPRESS` emits for a top-level subparser on 3.12+. - `hermes login [--flags]` still reaches the actionable deprecation message for old scripts/aliases; `hermes login --help` shows the redirect. Picks up the intent of the inactivity-closed #24902, rebased onto the post-refactor parser location (hermes_cli/subcommands/login.py) and extended to fix the whole bug class (any provider value), not just hiding from --help. Tests: parametrized provider acceptance + help-suppression (no SUPPRESS leak). --- hermes_cli/subcommands/login.py | 36 ++++++++++++---- tests/hermes_cli/test_subcommands_batch.py | 49 ++++++++++++++++++++++ 2 files changed, 77 insertions(+), 8 deletions(-) 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