[codex] fix Photon platform plugin CLI loading - #55824
Conversation
Load explicitly enabled bundled platform plugins so Photon CLI commands remain available. Respect explicit disabled platform config even when env credentials are present. Add regressions for Photon env auto-enable and bundled platform CLI loading. (cherry picked from commit f5ecd7f5f02f89bdd0aa438745f29d327de44bda) (cherry picked from commit c5d4a299c47657f08e812531dec1a1e20edd5c72)
|
c5d4a29 to
d310e8c
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing the deferred-platform CLI regression. The current-main premise is real: bundled platforms are deferred at hermes_cli/plugins.py:1444-1446, while Photon registers hermes photon only when its adapter loads at plugins/platforms/photon/adapter.py:1789-1795.
Problems
- The proposed eager-load branch only applies when the bundled platform is in
plugins.enabled. It therefore does not materialize Photon for the normal deferredhermes photonpath;main()still reads_cli_commandsafter discovery athermes_cli/main.py:13228-13240without resolving the matching deferred loader. - The
gateway/config.pychange removes the intentional_enabled_explicitdistinction. Current YAML loading records that marker atgateway/config.py:1125-1129, and the registry pass deliberately probes non-explicit disabled entries atgateway/config.py:2181-2217for env configuration.
Suggested changes
- Use the targeted deferred-platform resolution approach identified in the discussion of #54717, then test the loader → CLI registration → argparse path.
- Keep the explicit-disable marker guard unless a file-loaded configuration repro establishes otherwise.
Automated hermes-sweeper review.
| @@ -1955,11 +1955,7 @@ def _enable_from_env(platform: Platform) -> PlatformConfig: | |||
| # explicitly disabled it, never re-enable here just because | |||
There was a problem hiding this comment.
_enabled_explicit is the intentional boundary here: load_gateway_config() sets it when YAML supplied enabled (gateway/config.py:1125-1129), while the registry path later probes an existing disabled config for env-only setup (gateway/config.py:2181-2217). Removing the marker makes every non-explicit disabled entry suppress env enablement; please retain it unless a file-loaded config repro demonstrates the marker is absent for a user-authored disable.
Summary
Keep explicitly enabled bundled platform plugins loadable during plugin CLI discovery so plugin-owned commands such as
hermes photon setupremain available.Also tighten gateway env auto-enable behavior so an explicitly disabled platform stays disabled even when credentials are present in the environment.
Root Cause
Bundled platform plugins are normally deferred to avoid importing every platform SDK on ordinary CLI startup. That is correct for the common path, but it also meant an explicitly enabled bundled platform could appear enabled while its plugin-owned top-level CLI command was never registered.
Separately, gateway env auto-enable only skipped disabled platform configs when an internal
_enabled_explicitmarker was present. User config that disabled a platform without that marker could be re-enabled by environment credentials.Changes
Validation
scripts/run_tests.sh tests/gateway/test_plugin_env_enablement.py tests/plugins/test_bundled_platform_plugin_loading.pyuv pip check --python /home/fnoor1/.hermes/hermes-agent/venv/bin/pythonhermes photon --help