fix(tools): dedupe bundled plugin toolsets with built-in entries (#15634)
`hermes tools` → "reconfigure existing" listed Spotify twice because the Apr 24 refactor that moved Spotify into plugins/spotify/ (PR #15174) left the entry in CONFIGURABLE_TOOLSETS. _get_effective_configurable_toolsets() unconditionally appended get_plugin_toolsets() on top, so the same 'spotify' key showed up from both sources. Dedupe by key — built-in CONFIGURABLE_TOOLSETS entry wins (it has the nicer label and description). Also guards against future bundled plugins that share a toolset key with a built-in.
This commit is contained in:
@ -103,13 +103,23 @@ def _get_effective_configurable_toolsets():
|
||||
"""Return CONFIGURABLE_TOOLSETS + any plugin-provided toolsets.
|
||||
|
||||
Plugin toolsets are appended at the end so they appear after the
|
||||
built-in toolsets in the TUI checklist.
|
||||
built-in toolsets in the TUI checklist. A plugin whose toolset key
|
||||
already appears in ``CONFIGURABLE_TOOLSETS`` is skipped — bundled
|
||||
plugins (e.g. ``plugins/spotify``) share their toolset key with the
|
||||
built-in entry, and we want the built-in label/description to win.
|
||||
Without the dedupe, ``hermes tools`` → "reconfigure existing" would
|
||||
list the same toolset twice.
|
||||
"""
|
||||
result = list(CONFIGURABLE_TOOLSETS)
|
||||
seen = {ts_key for ts_key, _, _ in result}
|
||||
try:
|
||||
from hermes_cli.plugins import discover_plugins, get_plugin_toolsets
|
||||
discover_plugins() # idempotent — ensures plugins are loaded
|
||||
result.extend(get_plugin_toolsets())
|
||||
for entry in get_plugin_toolsets():
|
||||
if entry[0] in seen:
|
||||
continue
|
||||
seen.add(entry[0])
|
||||
result.append(entry)
|
||||
except Exception:
|
||||
pass
|
||||
return result
|
||||
|
||||
@ -766,3 +766,24 @@ def test_get_platform_tools_feishu_tools_not_on_other_platforms():
|
||||
enabled = _get_platform_tools({}, plat)
|
||||
assert "feishu_doc" not in enabled, f"feishu_doc leaked onto {plat}"
|
||||
assert "feishu_drive" not in enabled, f"feishu_drive leaked onto {plat}"
|
||||
|
||||
|
||||
def test_get_effective_configurable_toolsets_dedupes_bundled_plugins():
|
||||
"""Bundled plugins (plugins/spotify) share their toolset key with the
|
||||
built-in CONFIGURABLE_TOOLSETS entry. The effective list must not list
|
||||
them twice — otherwise `hermes tools` → "reconfigure existing" shows
|
||||
the same toolset two rows in a row.
|
||||
"""
|
||||
from hermes_cli.tools_config import _get_effective_configurable_toolsets
|
||||
|
||||
all_ts = _get_effective_configurable_toolsets()
|
||||
keys = [ts_key for ts_key, _, _ in all_ts]
|
||||
assert len(keys) == len(set(keys)), (
|
||||
f"duplicate toolset keys in effective list: "
|
||||
f"{[k for k in keys if keys.count(k) > 1]}"
|
||||
)
|
||||
# Spotify specifically — the bug that motivated the dedupe.
|
||||
spotify_rows = [t for t in all_ts if t[0] == "spotify"]
|
||||
assert len(spotify_rows) == 1, spotify_rows
|
||||
# Built-in label wins over the plugin label.
|
||||
assert spotify_rows[0][1] == "🎵 Spotify"
|
||||
|
||||
Reference in New Issue
Block a user