From 5089596685826ef2f63214f2fd184da88cc4cdb7 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Fri, 8 May 2026 16:07:23 -0700 Subject: [PATCH] perf(cli): skip eager plugin discovery on known built-in subcommands (#22120) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `hermes --help` drops from ~700ms to ~180ms; `hermes version` from ~950ms to ~240ms. ~4-5x startup speedup on inspection / diagnostic invocations. Changes: - hermes_cli/main.py: gate the argparse-setup `discover_plugins()` call behind `_plugin_cli_discovery_needed()`. Eager plugin imports (google.cloud.pubsub_v1, aiohttp, grpc, PIL) cost 500-650ms and are pure waste when the user is running a built-in subcommand that doesn't take plugin extensions (`--help`, `version`, `logs`, `config`, `sessions`, etc.). New `_BUILTIN_SUBCOMMANDS` frozenset + `_first_positional_argv` helper handle flag-value skipping (`-m gpt5 chat` → still fast). - hermes_cli/main.py: `cmd_version` now reads the OpenAI SDK version via `importlib.metadata` (~2ms) instead of `import openai` (~800ms of pydantic type-module loading). Agent-running paths (`hermes chat`, `hermes gateway run`) are unaffected — the second `discover_plugins()` call later in `main()` still runs so plugin hooks / tools wire up normally. Tests: - tests/hermes_cli/test_startup_plugin_gating.py: parity test guards the `_BUILTIN_SUBCOMMANDS` set against drift (every registered subparser must be declared; no phantom entries). Behavior tests for flag-value skipping, `--` terminator, inline `--flag=value` form. 37 tests. --- hermes_cli/main.py | 185 ++++++++++++++---- .../hermes_cli/test_startup_plugin_gating.py | 180 +++++++++++++++++ 2 files changed, 332 insertions(+), 33 deletions(-) create mode 100644 tests/hermes_cli/test_startup_plugin_gating.py diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 703d383485..b523c4491b 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -5363,11 +5363,16 @@ def cmd_version(args): # Show Python version print(f"Python: {sys.version.split()[0]}") - # Check for key dependencies + # Check for key dependencies. Use importlib.metadata rather than + # ``import openai`` — the SDK drags in ~800ms of pydantic-backed type + # modules just to expose ``__version__``. Metadata lookup is ~2ms. try: - import openai + from importlib.metadata import version as _pkg_version, PackageNotFoundError - print(f"OpenAI SDK: {openai.__version__}") + try: + print(f"OpenAI SDK: {_pkg_version('openai')}") + except PackageNotFoundError: + print("OpenAI SDK: Not installed") except ImportError: print("OpenAI SDK: Not installed") @@ -8793,6 +8798,113 @@ def _build_provider_choices() -> list[str]: ] +# Top-level subcommands that argparse knows about WITHOUT running plugin +# discovery. Used to short-circuit eager plugin imports (which can take +# 500ms+ pulling in google.cloud.pubsub_v1, aiohttp, grpc, etc.) when the +# user's invocation clearly doesn't need any plugin-registered subcommand. +# +# Keep this in sync with the ``subparsers.add_parser("NAME", ...)`` calls +# below in ``main()``. Missing an entry here only costs a one-time +# discovery; extra entries here would let a plugin command silently fail +# to parse. +_BUILTIN_SUBCOMMANDS = frozenset( + { + "acp", "auth", "backup", "checkpoints", "claw", "completion", + "config", "cron", "curator", "dashboard", "debug", "doctor", + "dump", "fallback", "gateway", "hooks", "import", "insights", + "kanban", "login", "logout", "logs", "mcp", "memory", "model", + "pairing", "plugins", "profile", "sessions", "setup", "skills", + "slack", "status", "tools", "uninstall", "update", "version", + "webhook", "whatsapp", "chat", + # Help-ish invocations — plugin commands not being listed in + # top-level --help is an acceptable trade-off for skipping an + # expensive eager import of every bundled plugin module. + "help", + } +) + + +# 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", + # ``-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:]``. + + Used by ``main()`` to decide whether plugin discovery has to run at + argparse-setup time. Handles common invocations like + ``hermes -m gpt5 --provider openai chat "msg"`` by skipping the + values attached to known top-level flags. + + Does NOT fully simulate argparse — unknown ``--foo=bar`` / ``--foo + bar`` flags degrade gracefully (``bar`` may be wrongly classified as + a positional, which at worst forces a one-time plugin discovery). + """ + argv = sys.argv[1:] + i = 0 + while i < len(argv): + tok = argv[i] + if tok == "--": + # Everything after ``--`` is positional. + if i + 1 < len(argv): + return argv[i + 1] + return None + if tok.startswith("-"): + # ``--flag=value`` carries its value inline — single token. + if "=" in tok: + i += 1 + continue + if tok in _TOP_LEVEL_VALUE_FLAGS and i + 1 < len(argv): + i += 2 + continue + i += 1 + continue + return tok + return None + + +def _plugin_cli_discovery_needed() -> bool: + """True when the CLI might be invoking a plugin-registered subcommand. + + Returning False lets ``main()`` skip plugin discovery entirely during + argparse setup, saving ~500-650ms per invocation for users whose + enabled plugins don't contribute any CLI command. + """ + first = _first_positional_argv() + if first is None: + # Bare ``hermes`` or only flags → defaults to ``chat``. + return False + if first in _BUILTIN_SUBCOMMANDS: + return False + # Unknown token — could be a plugin subcommand, OR a chat prompt + # starting with a non-flag word. Either way we need discovery: if it + # IS a plugin command, argparse needs the subparser; if it's a chat + # prompt, argparse will route it via positional handling and the + # extra discovery cost is amortized over a full agent run anyway. + return True + + def main(): """Main entry point for hermes CLI.""" # Force UTF-8 stdio on Windows before anything prints. No-op elsewhere. @@ -10077,39 +10189,46 @@ Examples: # Plugin CLI commands — dynamically registered by memory/general plugins. # Plugins provide a register_cli(subparser) function that builds their # own argparse tree. No hardcoded plugin commands in main.py. + # + # Skipped when the invocation is already targeting a known built-in + # subcommand — ``hermes --help``, ``hermes version``, ``hermes logs``, + # etc. This avoids eagerly importing every bundled plugin module + # (google.cloud.pubsub_v1, aiohttp, grpc, PIL …) which costs + # 500-650ms on typical installs. # ========================================================================= - try: - from plugins.memory import discover_plugin_cli_commands - from hermes_cli.plugins import discover_plugins, get_plugin_manager + if _plugin_cli_discovery_needed(): + try: + from plugins.memory import discover_plugin_cli_commands + from hermes_cli.plugins import discover_plugins, get_plugin_manager - seen_plugin_commands = set() - for cmd_info in discover_plugin_cli_commands(): - plugin_parser = subparsers.add_parser( - cmd_info["name"], - help=cmd_info["help"], - description=cmd_info.get("description", ""), - formatter_class=__import__("argparse").RawDescriptionHelpFormatter, - ) - cmd_info["setup_fn"](plugin_parser) - if cmd_info.get("handler_fn") is not None: - plugin_parser.set_defaults(func=cmd_info["handler_fn"]) - seen_plugin_commands.add(cmd_info["name"]) + seen_plugin_commands = set() + for cmd_info in discover_plugin_cli_commands(): + plugin_parser = subparsers.add_parser( + cmd_info["name"], + help=cmd_info["help"], + description=cmd_info.get("description", ""), + formatter_class=__import__("argparse").RawDescriptionHelpFormatter, + ) + cmd_info["setup_fn"](plugin_parser) + if cmd_info.get("handler_fn") is not None: + plugin_parser.set_defaults(func=cmd_info["handler_fn"]) + seen_plugin_commands.add(cmd_info["name"]) - discover_plugins() - for cmd_info in get_plugin_manager()._cli_commands.values(): - if cmd_info["name"] in seen_plugin_commands: - continue - plugin_parser = subparsers.add_parser( - cmd_info["name"], - help=cmd_info["help"], - description=cmd_info.get("description", ""), - formatter_class=__import__("argparse").RawDescriptionHelpFormatter, - ) - cmd_info["setup_fn"](plugin_parser) - if cmd_info.get("handler_fn") is not None: - plugin_parser.set_defaults(func=cmd_info["handler_fn"]) - except Exception as _exc: - logging.getLogger(__name__).debug("Plugin CLI discovery failed: %s", _exc) + discover_plugins() + for cmd_info in get_plugin_manager()._cli_commands.values(): + if cmd_info["name"] in seen_plugin_commands: + continue + plugin_parser = subparsers.add_parser( + cmd_info["name"], + help=cmd_info["help"], + description=cmd_info.get("description", ""), + formatter_class=__import__("argparse").RawDescriptionHelpFormatter, + ) + cmd_info["setup_fn"](plugin_parser) + if cmd_info.get("handler_fn") is not None: + plugin_parser.set_defaults(func=cmd_info["handler_fn"]) + except Exception as _exc: + logging.getLogger(__name__).debug("Plugin CLI discovery failed: %s", _exc) # ========================================================================= # curator command — background skill maintenance diff --git a/tests/hermes_cli/test_startup_plugin_gating.py b/tests/hermes_cli/test_startup_plugin_gating.py new file mode 100644 index 0000000000..6028b3ea2d --- /dev/null +++ b/tests/hermes_cli/test_startup_plugin_gating.py @@ -0,0 +1,180 @@ +"""Guards for CLI startup performance regression. + +``hermes_cli.main`` skips eager plugin discovery at argparse-setup time +when the invocation is clearly targeting a known built-in subcommand. +This saves 500-650ms on ``hermes --help``, ``hermes version``, +``hermes logs``, etc., by not importing ``google.cloud.pubsub_v1``, +``aiohttp``, ``grpc``, and friends. + +Two invariants: + +1. ``_BUILTIN_SUBCOMMANDS`` must contain every subcommand that is actually + registered by ``main()``. If an entry is missing, plugin discovery + runs unnecessarily for that command (correctness-safe, just slow). + If an entry is PRESENT but the subcommand doesn't exist, a plugin + could shadow the name — also bad. + +2. ``_plugin_cli_discovery_needed()`` returns the right answer for the + flag/positional parsing cases it's meant to handle. +""" + +from __future__ import annotations + +import io +import re +import sys +from contextlib import redirect_stdout +from unittest.mock import patch + +import pytest + +from hermes_cli.main import ( + _BUILTIN_SUBCOMMANDS, + _first_positional_argv, + _plugin_cli_discovery_needed, +) + + +# ── helper: grab the live set of top-level subcommands from argparse ─────── + + +def _live_subcommand_names() -> set[str]: + """Run ``hermes --help`` in-process and parse the subcommand block. + + We patch ``_plugin_cli_discovery_needed`` to always return False so + plugin-registered commands aren't included — we're validating the + built-in-only set. + """ + from hermes_cli import main as _main + + argv_backup = sys.argv[:] + sys.argv = ["hermes", "--help"] + buf = io.StringIO() + try: + with patch.object(_main, "_plugin_cli_discovery_needed", return_value=False): + with redirect_stdout(buf): + with pytest.raises(SystemExit): + _main.main() + finally: + sys.argv = argv_backup + + text = buf.getvalue() + # argparse prints "{chat,model,...}" somewhere in the help output + m = re.search(r"\{([a-zA-Z0-9_,\-]+)\}", text) + assert m, f"Could not find subcommand group in --help output:\n{text[:500]}" + return set(m.group(1).split(",")) + + +# ── _first_positional_argv ───────────────────────────────────────────────── + + +@pytest.mark.parametrize( + "argv,expected", + [ + (["hermes"], None), + (["hermes", "--help"], None), + (["hermes", "-h"], None), + (["hermes", "--version"], None), + (["hermes", "-w"], None), + # -p / --profile is stripped from sys.argv by + # _apply_profile_override() at import time, so it never reaches + # _first_positional_argv. We test with just -w / --tui here. + (["hermes", "-w", "--tui"], None), + (["hermes", "version"], "version"), + (["hermes", "--tui", "chat"], "chat"), + (["hermes", "-w", "logs"], "logs"), + (["hermes", "chat", "hello world"], "chat"), + (["hermes", "gateway", "run"], "gateway"), + # Top-level value-taking flags: the value should be skipped. + (["hermes", "-m", "gpt5", "chat"], "chat"), + (["hermes", "--model", "gpt5", "chat", "hi"], "chat"), + (["hermes", "-m", "gpt5", "--provider", "openai", "chat"], "chat"), + (["hermes", "-z", "hello world"], None), + (["hermes", "-z", "hello", "chat"], "chat"), + (["hermes", "--model=gpt5", "chat"], "chat"), # inline form + (["hermes", "--", "chat"], "chat"), # -- terminator + (["hermes", "-w", "--"], None), + # Unknown positional after skipped flags → plugin-cmd candidate. + (["hermes", "some-plugin-cmd"], "some-plugin-cmd"), + (["hermes", "-m", "gpt5", "some-plugin-cmd"], "some-plugin-cmd"), + ], +) +def test_first_positional_argv(argv, expected): + with patch.object(sys, "argv", argv): + assert _first_positional_argv() == expected + + +# ── _plugin_cli_discovery_needed ─────────────────────────────────────────── + + +@pytest.mark.parametrize( + "argv", + [ + ["hermes"], # bare → chat + ["hermes", "--help"], # top-level help + ["hermes", "-h"], + ["hermes", "version"], # known built-in + ["hermes", "logs"], + ["hermes", "gateway", "run"], + ["hermes", "--tui"], + ["hermes", "-w", "--tui"], + ["hermes", "chat", "hi"], + ["hermes", "help"], # accepted built-in-ish + ["hermes", "-m", "gpt5", "chat"], # flag-value-skipping + ], +) +def test_discovery_skipped_for_builtins(argv): + with patch.object(sys, "argv", argv): + assert _plugin_cli_discovery_needed() is False + + +@pytest.mark.parametrize( + "argv", + [ + ["hermes", "meet", "join"], # potential google_meet plugin + ["hermes", "honcho", "status"], # potential memory plugin + ["hermes", "unknown-subcmd"], + ], +) +def test_discovery_runs_for_unknown_positional(argv): + with patch.object(sys, "argv", argv): + assert _plugin_cli_discovery_needed() is True + + +# ── _BUILTIN_SUBCOMMANDS ↔ argparse registration parity ──────────────────── + + +def test_builtin_set_covers_every_registered_subcommand(): + """Every subcommand registered in main() must appear in the set. + + Missing entries cause a slow-path regression (correctness stays + fine — discovery just runs unnecessarily). + """ + live = _live_subcommand_names() + # "help" is synthetic — an argparse-implicit convenience we include + # in the set so ``hermes help `` skips discovery; it won't show + # up as a subparser in the --help output. + declared = _BUILTIN_SUBCOMMANDS - {"help"} + missing_from_declaration = live - declared + assert not missing_from_declaration, ( + f"_BUILTIN_SUBCOMMANDS is missing these live subcommands: " + f"{sorted(missing_from_declaration)}. Add them to " + f"hermes_cli/main.py::_BUILTIN_SUBCOMMANDS so plugin discovery " + f"can be skipped when the user targets them." + ) + + +def test_builtin_set_has_no_phantom_entries(): + """No entry in the set should refer to a subcommand that no longer exists. + + A phantom entry means plugin discovery gets incorrectly skipped for + a name that — if a plugin actually registered it — would fail to + parse. Keeps the set honest. + """ + live = _live_subcommand_names() + allowed_synthetic = {"help"} + phantom = _BUILTIN_SUBCOMMANDS - live - allowed_synthetic + assert not phantom, ( + f"_BUILTIN_SUBCOMMANDS has entries that are not registered as " + f"top-level subparsers: {sorted(phantom)}" + )