Skip to content

fix(plugins): include platforms/ namespace in bundled platform plugin keys#28160

Closed
Tranquil-Flow wants to merge 1 commit into
NousResearch:mainfrom
Tranquil-Flow:fix/27548-platform-plugin-discovery-drops-platforms-namespace-from-k
Closed

fix(plugins): include platforms/ namespace in bundled platform plugin keys#28160
Tranquil-Flow wants to merge 1 commit into
NousResearch:mainfrom
Tranquil-Flow:fix/27548-platform-plugin-discovery-drops-platforms-namespace-from-k

Conversation

@Tranquil-Flow

@Tranquil-Flow Tranquil-Flow commented May 18, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a bug where bundled gateway platform plugins under plugins/platforms/<name>/ (e.g., plugins/platforms/teams/) were discovered with keys derived from the manifest name field (e.g., teams-platform) instead of the path-derived namespaced key (e.g., platforms/teams).

This broke plugins.enabled / plugins.disabled config entries that use canonical namespaced keys, and was inconsistent with the plugin manager's key model for namespaced plugins (image_gen/openai, web/tavily, etc.).

The fix passes prefix="platforms" to _scan_directory_level() when scanning the bundled platforms directory so plugin keys are computed as platforms/<dir_name>.

Related Issue

Fixes #27548

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_cli/plugins.py: Pass prefix="platforms" to _scan_directory_level() in the bundled platform plugin discovery path
  • tests/hermes_cli/test_plugins.py: Add regression test test_bundled_platform_plugins_have_platforms_namespace_key verifying platform plugin keys include the platforms/ namespace

How to Test

  1. Run python -m pytest tests/hermes_cli/test_plugins.py -q -o 'addopts='
  2. Verify the new test passes and existing tests are unaffected
  3. Alternatively, inspect plugin keys after discovery — platform plugins should show as platforms/teams, platforms/irc, etc. instead of teams-platform, irc-platform

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • N/A — no config keys, architecture, or tool behavior changed

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins labels May 18, 2026
@alt-glitch

alt-glitch commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Duplicate of #27240 — both fix #27548 (platform plugin discovery drops platforms/ namespace from keys) by passing prefix="platforms" to _scan_directory_level().

BoardJames-Bot
BoardJames-Bot reviewed May 18, 2026
View reviewed changes

@BoardJames-Bot BoardJames-Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed by Hermes Agent. The diff itself looks reasonable and the focused plugin tests pass locally (63 passed), but GitHub currently reports this PR as CONFLICTING against main. I am leaving a comment instead of approval/merge: please rebase or resolve the conflicts, then it can be re-reviewed. BoardJames-Bot also has pull-only repo permission, so I cannot merge even after conflicts are resolved.

@outsourc-e outsourc-e mentioned this pull request May 18, 2026
Open
@outsourc-e

outsourc-e commented May 18, 2026

Copy link
Copy Markdown
Contributor

Rebased/salvaged this locally. The fix itself is good, but the current PR is non-mergeable/conflicting against main. I opened a clean replacement here: #28178. Verification on the replacement branch: scripts/run_tests.sh tests/hermes_cli/test_plugins.py -> 73 passed.

@Tranquil-Flow
fix(plugins): include platforms/ namespace in bundled platform plugin…
22c79a0 
… keys

Bundled gateway platform plugins under plugins/platforms/<name>/ were
discovered with keys derived from the manifest name (e.g. teams-platform)
instead of the path-derived namespaced key (e.g. platforms/teams).

This broke config entries in plugins.enabled / plugins.disabled that use
canonical namespaced keys, because the discovery key did not match.

Pass prefix='platforms' to _scan_directory_level() when scanning the
platforms directory so keys are computed as platforms/<dir_name>.

Closes NousResearch#27548
@alt-glitch

alt-glitch commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Superseded by #28179, which applies the same fix cleanly on current main.

@Tranquil-Flow Tranquil-Flow force-pushed the fix/27548-platform-plugin-discovery-drops-platforms-namespace-from-k branch from a0cede1 to 22c79a0 Compare May 18, 2026 20:15
@Tranquil-Flow

Tranquil-Flow commented May 18, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto latest main

This PR had merge conflicts against main (the platforms discovery code gained debug logging and a bundled_platforms variable since the branch was created). I've resolved the conflicts, keeping both:

  • Upstream's debug logging (logger.debug("bundled/platforms: %d manifest(s)", ...))
  • The actual fix — using _scan_directory_level() with prefix="platforms" so keys are computed as platforms/<name> instead of bare <name>-platform

Verification

python -m pytest tests/hermes_cli/test_plugins.py -q
73 passed in 4.04s

Diff is clean — 2 files changed (hermes_cli/plugins.py + tests/hermes_cli/test_plugins.py).

Ready for review/merge. ✅

@Tranquil-Flow

Tranquil-Flow commented May 18, 2026

Copy link
Copy Markdown
Contributor Author

Closing — this is a duplicate of #27240 by @felix-windsor, which was opened first and carries a cleaner focused diff. Apologies for the overlap; our duplicate-PR search missed it because #27240 doesn't reference the issue number in its title or body.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@BoardJames-Bot BoardJames-Bot BoardJames-Bot left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Platform plugin discovery drops platforms/ namespace from keys

4 participants

@Tranquil-Flow @alt-glitch @outsourc-e @BoardJames-Bot