fix(cli): resolve deferred platform plugin for its top-level CLI command (salvage #54717) - #73498
Merged
Conversation
Closes #54678 `hermes photon ...` could fail with argparse `invalid choice: 'photon'` even when the bundled Photon platform plugin is present. Photon registers its top-level CLI command from the platform adapter module via `ctx.register_cli_command(name="photon", ...)`, but bundled platform plugins are cheap-registered as *deferred* entries to avoid importing every gateway SDK during normal startup. On the unknown-top-level-command slow path, `discover_plugins()` records the deferred loader but never imports the matching platform module, so the CLI registration side effect doesn't run and `photon` stays absent from `_cli_commands` — argparse then rejects it. Fix: after `discover_plugins()` on that slow path, resolve only the deferred platform whose name matches the first positional token (via `platform_registry.get(name)`) before reading `_cli_commands`. This imports exactly the targeted platform, leaving normal startup cheap (a bare `hermes` or flags-only invocation has no positional token and touches nothing). The resolution is best-effort: registry/import failures are logged at debug and never crash startup. Added 3 tests in tests/hermes_cli/test_startup_plugin_gating.py: resolves the matching platform, ignores empty/None command, and swallows registry errors. Fails without the fix (symbol absent).
Responds to hermes-sweeper review on #54717: existing tests only mocked platform_registry.get(). Add a hermetic fake deferred-loader test that runs real PlatformRegistry resolution → PluginContext.register_cli_command → argparse subparser/choices visibility, without Photon SDK imports.
Contributor
૮ >ﻌ< ა ci reviewran on be31d2f all good! |
This was referenced Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
hermes photon(and any future platform plugin's top-level CLI command) works again — the deferred platform lazy-loader now fires for the one platform whose name matches the invoked command, so itsregister_cli_command()side effect runs before argparse builds the subparser table.Root cause: PR #54448 made bundled
kind: platformplugins lazy-load (deferred registry loaders) to cut startup time. Photon is the only platform that registers a top-levelhermes <name>CLI command inside its pluginregister(), so deferral silently unregisteredhermes photon— argparse rejects it withinvalid choice(issue #54678, reported again by TheSameCat on X, July 28). The docs were correct; the command had vanished.Salvages PR #54717 by @Bartok9 (earliest fix, June 29) onto current main with authorship preserved.
Changes
hermes_cli/main.py: new_resolve_deferred_platform_cli_command()— on the unknown-top-level-command slow path, afterdiscover_plugins(), resolve the deferred platform matching the first positional argv token viaplatform_registry.get()so its loader imports the module and registers the CLI command. Only the matched platform loads; errors are logged-and-swallowed.tests/hermes_cli/test_startup_plugin_gating.py: 4 regression tests, including a hermetic full-chain test (realPlatformRegistry+ realPluginManager+ fake deferred loader → CLI command visible in argparse choices) — no Photon SDK import.Validation
hermes photon --helpinvalid choice: 'photon'hermes photon status(temp HERMES_HOME)hermes versionfast pathtest_startup_plugin_gating.pytest_plugin_cli_registration.py+tests/plugins/platforms/photon/Fixes #54678.
Infographic