feat(plugins): support config-driven external paths - #38698
Conversation
a463338 to
8148005
Compare
fe63afe to
db27223
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing a real plugin-development gap. The premise is confirmed on current main: discovery has no external-root source, and its unconditional winner assignment at hermes_cli/plugins.py:1382 can let an unenabled later manifest suppress an earlier bundled winner before the enable gate at :1452-1466.
Problems
HERMES_PLUGIN_PATHSat PR-headhermes_cli/plugins.py:253-255is a new non-secret, user-facing behavior setting.AGENTS.md:102-107requires this configuration to live inconfig.yaml; retainplugins.extra_pathsand remove the environment-variable interface.- The new scan at PR-head
hermes_cli/plugins.py:1154-1163only reachesPluginManager. Model-provider manifests are deliberately skipped at:1236-1244, butproviders/__init__.py:91-171only scans bundled and$HERMES_HOMEprovider directories. Memory, context-engine, and dashboard paths likewise have independent discovery, so the advertised external source is incomplete for those plugin categories.
Suggested changes
- Define whether external roots support all plugin categories. Wire each independent loader to the configured roots with focused tests, or narrow the feature/docs to general PluginManager plugins.
- Rebase the collision work onto the current loader, which has moved substantially since the PR parent.
Automated hermes-sweeper review.
f891903 to
d87f9af
Compare
b0396fd to
0085688
Compare
|
Rebased onto current upstream main ( |
86c4b8b to
0fa4d3e
Compare
|
Final freshness update: this branch is now rebased directly onto captured The review requirements remain fully addressed: there is no Post-rebase verification is clean: |
0fa4d3e to
596af0e
Compare
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Three PRs cover two distinct plugin improvements: #33363 and #34471 contain the same fix for a curses renderer that ignored its computed scroll offset, while #38698 adds config-driven external general-plugin discovery and prevents unenabled later manifests from shadowing active bundled winners.
Related pull requests
- #33363 [closed]
related— (+184/-7) — superseded by #34471: fixes long-list scrolling by rendering fromscroll_offsetand adds plugin-list filters and plain/JSON output. It remains relevant because its commit was cherry-picked into the merged salvage PR with authorship preserved. - #34471 [merged]
related— (+184/-7) — merged reference implementation: carries the #33363 scroll-offset fix, navigation controls, list filters, compact outputs, and focused tests onto current main. - #38698
related— (+670/-17) — merge candidate: addsplugins.extra_pathsfor external general-plugin collections or direct checkouts, aligns runtime and CLI discovery, preserves opt-in activation, and handles collisions and invalid roots deterministically. The earlier keep_open review on #38698 identified an environment-variable interface and overstated category coverage; the current diff removes that interface and explicitly limits and documents support to the generalPluginManager, directly addressing both objections.
Duplicates
#33363 and #34471 are substantively identical; #34471 is the merged salvage of #33363.
Suggested consolidation
Merge #38698 after normal final CI/head verification: it addresses the separate external-path discovery gap, and its current diff resolves the contributor's keep_open review concerns without crossing the independent specialized loaders. No additional duplicate needs closing: #33363 is already closed as superseded by merged #34471, while #34471 should remain as the landed reference implementation.
Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 56 kB of PR diffs, 6 kB of issue/PR text, 3 kB of discussion (4 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
596af0e to
9cb676f
Compare
|
Thanks @GottZ — I completed the requested final current-main verification. This branch is now rebased onto upstream main 8defb9f at head 9cb676fcfd4495309917aa10cce90d3c901f9c37. The seven focused plugin runtime/control-plane files pass 120/120, Ruff and diff checks pass, and a fresh thread audit shows no unresolved review threads. The PR body now contains the exact current evidence. GitHub Actions for this fork head still require maintainer approval before jobs can start. |
9cb676f to
992ea0f
Compare
|
Refreshed #38698 onto current Scope remains the intended 6 files:
Local gates:
Review audit: both existing review threads are resolved; no unresolved review threads remain. The refreshed diff still has no Checks snapshot: GitHub status check rollup is empty/no CI result yet, so this comment does not claim CI green. Post-push readback shows PR open/non-draft, |
Add plugins.extra_paths discovery for general PluginManager plugins, aligned across runtime and plugin CLI listing while keeping activation opt-in. Relative entries resolve from the active Hermes home, external direct checkouts are supported, independent loader categories stay out of scope, and enabled external key collisions warn with source paths.
992ea0f to
27e2958
Compare
|
Refreshed #38698 onto current Scope remains the intended 6 files:
Local gates against the validated base:
Review audit: GitHub review-thread API reports 2 total review threads and 0 unresolved. The two pending empty reviews are authored by Checks snapshot: GitHub status check rollup is empty/no CI result yet, so this comment does not claim CI green. Post-push readback shows PR open/non-draft, Non-goals: no #38645/#40839 stack changes, no specialized loader path wiring, no generated/dependency/identity-file changes. |
What does this PR do?
Adds config-driven discovery for general plugins stored outside the Hermes source tree and
$HERMES_HOME/pluginsthroughplugins.extra_pathsinconfig.yaml.Configured roots may be collections or direct plugin checkouts. Relative roots resolve from the active Hermes home. External code remains opt-in through
plugins.enabled.Changes
plugins.extra_paths.plugin.yamlorplugin.yml.hermes plugins list/enable/disablewith runtime discovery.Verification
Rebased directly onto
upstream/mainat8defb9fd60bebe2802eaab7c57fa2ee6a4ff6281; current head9cb676fcfd4495309917aa10cce90d3c901f9c37.Result:
120 passedacross all seven relevant plugin runtime and control-plane files.Result: passed.
GitHub Actions for this fork head require maintainer approval before jobs can start; no pending check is represented as green.
Risks / mitigations
Checklist