From 8d12fb1e6bcf0144836cffb12b40edee9449eb64 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Fri, 24 Apr 2026 07:06:11 -0700 Subject: [PATCH] refactor(spotify): convert to built-in bundled plugin under plugins/spotify (#15174) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moves the Spotify integration from tools/ into plugins/spotify/, matching the existing pattern established by plugins/image_gen/ for third-party service integrations. Why: - tools/ should be reserved for foundational capabilities (terminal, read_file, web_search, etc.). tools/providers/ was a one-off directory created solely for spotify_client.py. - plugins/ is already the home for image_gen backends, memory providers, context engines, and standalone hook-based plugins. Spotify is a third-party service integration and belongs alongside those, not in tools/. - Future service integrations (eventually: Deezer, Apple Music, etc.) now have a pattern to copy. Changes: - tools/spotify_tool.py → plugins/spotify/tools.py (handlers + schemas) - tools/providers/spotify_client.py → plugins/spotify/client.py - tools/providers/ removed (was only used for Spotify) - New plugins/spotify/__init__.py with register(ctx) calling ctx.register_tool() × 7. The handler/check_fn wiring is unchanged. - New plugins/spotify/plugin.yaml (kind: backend, bundled, auto-load). - tests/tools/test_spotify_client.py: import paths updated. tools_config fix — _DEFAULT_OFF_TOOLSETS now wins over plugin auto-enable: - _get_platform_tools() previously auto-enabled unknown plugin toolsets for new platforms. That was fine for image_gen (which has no toolset of its own) but bad for Spotify, which explicitly requires opt-in (don't ship 7 tool schemas to users who don't use it). Added a check: if a plugin toolset is in _DEFAULT_OFF_TOOLSETS, it stays off until the user picks it in 'hermes tools'. Pre-existing test bug fix: - tests/hermes_cli/test_plugins.py::test_list_returns_sorted asserted names were sorted, but list_plugins() sorts by key (path-derived, e.g. image_gen/openai). With only image_gen plugins bundled, name and key order happened to agree. Adding plugins/spotify broke that coincidence (spotify sorts between openai-codex and xai by name but after xai by key). Updated test to assert key order, which is what the code actually documents. Validation: - scripts/run_tests.sh tests/hermes_cli/test_plugins.py \ tests/hermes_cli/test_tools_config.py \ tests/hermes_cli/test_spotify_auth.py \ tests/tools/test_spotify_client.py \ tests/tools/test_registry.py → 143 passed - E2E plugin load: 'spotify' appears in loaded plugins, all 7 tools register into the spotify toolset, check_fn gating intact. --- hermes_cli/tools_config.py | 8 ++- plugins/spotify/__init__.py | 66 +++++++++++++++++++ .../spotify/client.py | 0 plugins/spotify/plugin.yaml | 13 ++++ .../spotify/tools.py | 15 +---- tests/hermes_cli/test_plugins.py | 8 ++- tests/tools/test_spotify_client.py | 4 +- tools/providers/__init__.py | 1 - 8 files changed, 96 insertions(+), 19 deletions(-) create mode 100644 plugins/spotify/__init__.py rename tools/providers/spotify_client.py => plugins/spotify/client.py (100%) create mode 100644 plugins/spotify/plugin.yaml rename tools/spotify_tool.py => plugins/spotify/tools.py (93%) delete mode 100644 tools/providers/__init__.py diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index c18f6ab4e..09fc8ceec 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -607,7 +607,10 @@ def _get_platform_tools( default_off.remove(platform) enabled_toolsets -= default_off - # Plugin toolsets: enabled by default unless explicitly disabled. + # Plugin toolsets: enabled by default unless explicitly disabled, or + # unless the toolset is in _DEFAULT_OFF_TOOLSETS (e.g. spotify — + # shipped as a bundled plugin but user must opt in via `hermes tools` + # so we don't ship 7 Spotify tool schemas to users who don't use it). # A plugin toolset is "known" for a platform once `hermes tools` # has been saved for that platform (tracked via known_plugin_toolsets). # Unknown plugins default to enabled; known-but-absent = disabled. @@ -619,6 +622,9 @@ def _get_platform_tools( if pts in toolset_names: # Explicitly listed in config — enabled enabled_toolsets.add(pts) + elif pts in _DEFAULT_OFF_TOOLSETS: + # Opt-in plugin toolset — stay off until user picks it + continue elif pts not in known_for_platform: # New plugin not yet seen by hermes tools — default enabled enabled_toolsets.add(pts) diff --git a/plugins/spotify/__init__.py b/plugins/spotify/__init__.py new file mode 100644 index 000000000..0f68bba1f --- /dev/null +++ b/plugins/spotify/__init__.py @@ -0,0 +1,66 @@ +"""Spotify integration plugin — bundled, auto-loaded. + +Registers 7 tools (playback, devices, queue, search, playlists, albums, +library) into the ``spotify`` toolset. Each tool's handler is gated by +``_check_spotify_available()`` — when the user has not run ``hermes auth +spotify``, the tools remain registered (so they appear in ``hermes +tools``) but the runtime check prevents dispatch. + +Why a plugin instead of a top-level ``tools/`` file? + +- ``plugins/`` is where third-party service integrations live (see + ``plugins/image_gen/`` for the backend-provider pattern, ``plugins/ + disk-cleanup/`` for the standalone pattern). ``tools/`` is reserved + for foundational capabilities (terminal, read_file, web_search, etc.). +- Mirroring the image_gen plugin layout (``plugins///`` + for categories, flat ``plugins//`` for standalones) makes new + service integrations a pattern contributors can copy. +- Bundled + ``kind: backend`` auto-loads on startup just like image_gen + backends — no user opt-in needed, no ``plugins.enabled`` config. + +The Spotify auth flow (``hermes auth spotify``), CLI plumbing, and docs +are unchanged. This move is purely structural. +""" + +from __future__ import annotations + +from plugins.spotify.tools import ( + SPOTIFY_ALBUMS_SCHEMA, + SPOTIFY_DEVICES_SCHEMA, + SPOTIFY_LIBRARY_SCHEMA, + SPOTIFY_PLAYBACK_SCHEMA, + SPOTIFY_PLAYLISTS_SCHEMA, + SPOTIFY_QUEUE_SCHEMA, + SPOTIFY_SEARCH_SCHEMA, + _check_spotify_available, + _handle_spotify_albums, + _handle_spotify_devices, + _handle_spotify_library, + _handle_spotify_playback, + _handle_spotify_playlists, + _handle_spotify_queue, + _handle_spotify_search, +) + +_TOOLS = ( + ("spotify_playback", SPOTIFY_PLAYBACK_SCHEMA, _handle_spotify_playback, "🎵"), + ("spotify_devices", SPOTIFY_DEVICES_SCHEMA, _handle_spotify_devices, "🔈"), + ("spotify_queue", SPOTIFY_QUEUE_SCHEMA, _handle_spotify_queue, "📻"), + ("spotify_search", SPOTIFY_SEARCH_SCHEMA, _handle_spotify_search, "🔎"), + ("spotify_playlists", SPOTIFY_PLAYLISTS_SCHEMA, _handle_spotify_playlists, "📚"), + ("spotify_albums", SPOTIFY_ALBUMS_SCHEMA, _handle_spotify_albums, "💿"), + ("spotify_library", SPOTIFY_LIBRARY_SCHEMA, _handle_spotify_library, "❤️"), +) + + +def register(ctx) -> None: + """Register all Spotify tools. Called once by the plugin loader.""" + for name, schema, handler, emoji in _TOOLS: + ctx.register_tool( + name=name, + toolset="spotify", + schema=schema, + handler=handler, + check_fn=_check_spotify_available, + emoji=emoji, + ) diff --git a/tools/providers/spotify_client.py b/plugins/spotify/client.py similarity index 100% rename from tools/providers/spotify_client.py rename to plugins/spotify/client.py diff --git a/plugins/spotify/plugin.yaml b/plugins/spotify/plugin.yaml new file mode 100644 index 000000000..e9e1283e7 --- /dev/null +++ b/plugins/spotify/plugin.yaml @@ -0,0 +1,13 @@ +name: spotify +version: 1.0.0 +description: "Native Spotify integration — 7 tools (playback, devices, queue, search, playlists, albums, library) using Spotify Web API + PKCE OAuth. Auth via `hermes auth spotify`. Tools gate on `providers.spotify` in ~/.hermes/auth.json." +author: NousResearch +kind: backend +provides_tools: + - spotify_playback + - spotify_devices + - spotify_queue + - spotify_search + - spotify_playlists + - spotify_albums + - spotify_library diff --git a/tools/spotify_tool.py b/plugins/spotify/tools.py similarity index 93% rename from tools/spotify_tool.py rename to plugins/spotify/tools.py index b34b2b134..f6022ff5a 100644 --- a/tools/spotify_tool.py +++ b/plugins/spotify/tools.py @@ -1,11 +1,11 @@ -"""Native Spotify tools for Hermes.""" +"""Native Spotify tools for Hermes (registered via plugins/spotify).""" from __future__ import annotations from typing import Any, Dict, List from hermes_cli.auth import get_auth_status -from tools.providers.spotify_client import ( +from plugins.spotify.client import ( SpotifyAPIError, SpotifyAuthRequiredError, SpotifyClient, @@ -14,7 +14,7 @@ from tools.providers.spotify_client import ( normalize_spotify_uri, normalize_spotify_uris, ) -from tools.registry import registry, tool_error, tool_result +from tools.registry import tool_error, tool_result def _check_spotify_available() -> bool: @@ -452,12 +452,3 @@ SPOTIFY_LIBRARY_SCHEMA = { "required": ["kind", "action"], }, } - - -registry.register(name="spotify_playback", toolset="spotify", schema=SPOTIFY_PLAYBACK_SCHEMA, handler=_handle_spotify_playback, check_fn=_check_spotify_available, emoji="🎵") -registry.register(name="spotify_devices", toolset="spotify", schema=SPOTIFY_DEVICES_SCHEMA, handler=_handle_spotify_devices, check_fn=_check_spotify_available, emoji="🔈") -registry.register(name="spotify_queue", toolset="spotify", schema=SPOTIFY_QUEUE_SCHEMA, handler=_handle_spotify_queue, check_fn=_check_spotify_available, emoji="📻") -registry.register(name="spotify_search", toolset="spotify", schema=SPOTIFY_SEARCH_SCHEMA, handler=_handle_spotify_search, check_fn=_check_spotify_available, emoji="🔎") -registry.register(name="spotify_playlists", toolset="spotify", schema=SPOTIFY_PLAYLISTS_SCHEMA, handler=_handle_spotify_playlists, check_fn=_check_spotify_available, emoji="📚") -registry.register(name="spotify_albums", toolset="spotify", schema=SPOTIFY_ALBUMS_SCHEMA, handler=_handle_spotify_albums, check_fn=_check_spotify_available, emoji="💿") -registry.register(name="spotify_library", toolset="spotify", schema=SPOTIFY_LIBRARY_SCHEMA, handler=_handle_spotify_library, check_fn=_check_spotify_available, emoji="❤️") diff --git a/tests/hermes_cli/test_plugins.py b/tests/hermes_cli/test_plugins.py index 28d3abac7..157f967e5 100644 --- a/tests/hermes_cli/test_plugins.py +++ b/tests/hermes_cli/test_plugins.py @@ -635,7 +635,7 @@ class TestPluginManagerList: assert mgr.list_plugins() == [] def test_list_returns_sorted(self, tmp_path, monkeypatch): - """list_plugins() returns results sorted by name.""" + """list_plugins() returns results sorted by key.""" plugins_dir = tmp_path / "hermes_test" / "plugins" _make_plugin_dir(plugins_dir, "zulu") _make_plugin_dir(plugins_dir, "alpha") @@ -645,8 +645,10 @@ class TestPluginManagerList: mgr.discover_and_load() listing = mgr.list_plugins() - names = [p["name"] for p in listing] - assert names == sorted(names) + # list_plugins sorts by key (path-derived, e.g. ``image_gen/openai``), + # not by display name, so that category plugins group together. + keys = [p["key"] for p in listing] + assert keys == sorted(keys) def test_list_with_plugins(self, tmp_path, monkeypatch): """list_plugins() returns info dicts for each discovered plugin.""" diff --git a/tests/tools/test_spotify_client.py b/tests/tools/test_spotify_client.py index 61530f0aa..d22bc4480 100644 --- a/tests/tools/test_spotify_client.py +++ b/tests/tools/test_spotify_client.py @@ -4,8 +4,8 @@ import json import pytest -from tools.providers import spotify_client as spotify_mod -from tools import spotify_tool +from plugins.spotify import client as spotify_mod +from plugins.spotify import tools as spotify_tool class _FakeResponse: diff --git a/tools/providers/__init__.py b/tools/providers/__init__.py deleted file mode 100644 index 25dd42409..000000000 --- a/tools/providers/__init__.py +++ /dev/null @@ -1 +0,0 @@ -"""Provider-specific native tool clients."""