From 694550e486a35484c04196a962a5ced7aea5fafe Mon Sep 17 00:00:00 2001 From: fangliquanflq Date: Mon, 24 Aug 2026 12:30:13 +0800 Subject: [PATCH] fix(cli): derive top-level value flags from parser (cherry picked from commit f2e5a1388115615fafde49a5f2144e5855a81d89) --- hermes_cli/_parser.py | 15 ++++++ hermes_cli/main.py | 47 ++++--------------- .../hermes_cli/test_startup_plugin_gating.py | 22 +++++++++ 3 files changed, 45 insertions(+), 39 deletions(-) diff --git a/hermes_cli/_parser.py b/hermes_cli/_parser.py index 13bcaa4703..696eb65b9c 100644 --- a/hermes_cli/_parser.py +++ b/hermes_cli/_parser.py @@ -11,6 +11,7 @@ because its dispatch is tightly coupled to module-level ``cmd_*`` functions. """ import argparse +from functools import lru_cache # `--profile` / `-p` is consumed by ``main._apply_profile_override`` before @@ -23,6 +24,20 @@ PRE_ARGPARSE_INHERITED_FLAGS: list[tuple[str, bool]] = [ ] +@lru_cache(maxsize=1) +def top_level_value_flag_sets() -> tuple[frozenset[str], frozenset[str]]: + """Return required- and optional-value flags from the live parser.""" + parser = build_top_level_parser()[0] + required: set[str] = set() + optional: set[str] = set() + for action in parser._actions: + if not action.option_strings or action.nargs == 0: + continue + target = optional if action.nargs == "?" else required + target.update(action.option_strings) + return frozenset(required), frozenset(optional) + + def _inherited_flag(parser, *args, **kwargs): """Register a flag that ``hermes_cli.relaunch`` should carry over when the CLI re-execs itself (e.g. after ``sessions browse`` picks a session, diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 268752b009..4e27e6768b 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -572,17 +572,9 @@ def _apply_profile_override() -> None: # 1. Check for explicit -p / --profile flag. Historically this worked even # after the subcommand (`hermes chat -p coder`), so keep scanning broadly. # The exception is command-argv passthrough regions such as `mcp add --args`. - value_flags = { - "-z", "--oneshot", - "-m", "--model", - "--provider", - "-t", "--toolsets", - "-r", "--resume", - "-s", "--skills", - "--usage-file", - "--in", - } - optional_value_flags = {"-c", "--continue"} + from hermes_cli._parser import top_level_value_flag_sets + + value_flags, optional_value_flags = top_level_value_flag_sets() i = 0 while i < len(argv): arg = argv[i] @@ -11908,33 +11900,6 @@ _BUILTIN_SUBCOMMANDS = frozenset( ) -# Top-level flags that take a value. Needed by ``_first_positional_argv`` -# so that in ``hermes -m gpt5 chat``, ``gpt5`` is correctly skipped as a -# flag value rather than misclassified as a subcommand. Kept in sync with -# the top-level flags declared in ``hermes_cli/_parser.py``. -# -# Correctness-safe either way: missing an entry here only makes the -# fast-path bail out too eagerly (we run plugin discovery when we didn't -# need to); extra entries would make us skip a real positional. -_TOP_LEVEL_VALUE_FLAGS = frozenset( - { - "-z", "--oneshot", - "-m", "--model", - "--provider", - "-t", "--toolsets", - "-r", "--resume", - "-s", "--skills", - "--usage-file", - "--in", - # ``-c / --continue`` is nargs='?' (optional value). Treat it as - # value-taking: if the next token is a subcommand-looking word - # the user almost certainly meant it as the session name, and - # either interpretation keeps us on the safe side. - "-c", "--continue", - } -) - - def _first_positional_argv() -> str | None: """Return the first non-flag, non-flag-value token in ``sys.argv[1:]``. @@ -11947,6 +11912,10 @@ def _first_positional_argv() -> str | None: bar`` flags degrade gracefully (``bar`` may be wrongly classified as a positional, which at worst forces a one-time plugin discovery). """ + from hermes_cli._parser import top_level_value_flag_sets + + required_value_flags, optional_value_flags = top_level_value_flag_sets() + value_flags = required_value_flags | optional_value_flags argv = sys.argv[1:] i = 0 while i < len(argv): @@ -11961,7 +11930,7 @@ def _first_positional_argv() -> str | None: if "=" in tok: i += 1 continue - if tok in _TOP_LEVEL_VALUE_FLAGS and i + 1 < len(argv): + if tok in value_flags and i + 1 < len(argv): i += 2 continue i += 1 diff --git a/tests/hermes_cli/test_startup_plugin_gating.py b/tests/hermes_cli/test_startup_plugin_gating.py index 5f89d06614..64449a2c41 100644 --- a/tests/hermes_cli/test_startup_plugin_gating.py +++ b/tests/hermes_cli/test_startup_plugin_gating.py @@ -28,6 +28,7 @@ from unittest.mock import patch import pytest +from hermes_cli._parser import build_top_level_parser, top_level_value_flag_sets from hermes_cli.main import ( _BUILTIN_SUBCOMMANDS, _first_positional_argv, @@ -69,6 +70,27 @@ def _live_subcommand_names() -> set[str]: # ── _first_positional_argv ───────────────────────────────────────────────── +def test_value_flag_sets_match_top_level_parser(): + required, optional = top_level_value_flag_sets() + + for action in build_top_level_parser()[0]._actions: + if not action.option_strings or action.nargs == 0: + continue + expected = optional if action.nargs == "?" else required + assert set(action.option_strings) <= expected + + +def test_reasoning_value_is_not_misclassified_as_subcommand(monkeypatch): + monkeypatch.setattr( + sys, + "argv", + ["hermes", "--reasoning", "high", "chat", "hello"], + ) + + assert _first_positional_argv() == "chat" + assert _plugin_cli_discovery_needed() is False + + # ── _plugin_cli_discovery_needed ───────────────────────────────────────────