fix(cli): deprecated hermes login fails gracefully for any provider
`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).
This commit is contained in:
@@ -10,20 +10,40 @@ from typing import Callable
|
|||||||
|
|
||||||
|
|
||||||
def build_login_parser(subparsers, *, cmd_login: Callable) -> None:
|
def build_login_parser(subparsers, *, cmd_login: Callable) -> None:
|
||||||
"""Attach the ``login`` subcommand to ``subparsers``."""
|
"""Attach the deprecated ``login`` subcommand to ``subparsers``.
|
||||||
# =========================================================================
|
|
||||||
# login command
|
``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_parser = subparsers.add_parser(
|
||||||
"login",
|
"login",
|
||||||
help="Authenticate with an inference provider",
|
description=(
|
||||||
description="Run OAuth device authorization flow for Hermes CLI",
|
"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(
|
login_parser.add_argument(
|
||||||
"--provider",
|
"--provider",
|
||||||
choices=["nous", "openai-codex", "xai-oauth"],
|
|
||||||
default=None,
|
default=None,
|
||||||
help="Provider to authenticate with (default: nous)",
|
help="(deprecated) Provider name; ignored — see `hermes model`",
|
||||||
)
|
)
|
||||||
login_parser.add_argument(
|
login_parser.add_argument(
|
||||||
"--portal-url", help="Portal base URL (default: production portal)"
|
"--portal-url", help="Portal base URL (default: production portal)"
|
||||||
|
|||||||
@@ -95,3 +95,52 @@ def test_dashboard_builder_two_handlers():
|
|||||||
assert parser.parse_args(["dashboard"]).func is dash
|
assert parser.parse_args(["dashboard"]).func is dash
|
||||||
# dashboard register -> register handler
|
# dashboard register -> register handler
|
||||||
assert parser.parse_args(["dashboard", "register"]).func is reg
|
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
|
||||||
|
|||||||
Reference in New Issue
Block a user