From 134643a2fa80b4db1c4aa08cfece561f77b18e88 Mon Sep 17 00:00:00 2001 From: emozilla Date: Tue, 2 Jun 2026 03:25:46 -0400 Subject: [PATCH] fix(desktop): reflect active toolset provider in config panel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The toolset config panel highlighted the first keyless provider (e.g. Nous Portal) on load instead of the provider actually written to config. The /api/tools/toolsets/{name}/config endpoint never reported which provider was active, so the GUI's default-expand logic fell back to "first configured" — and keyless providers are always "configured". Backend now annotates each provider with is_active (via the same _is_provider_active helper the CLI 'hermes tools' picker uses) plus a top-level active_provider summary. The panel prefers that signal before falling back to first-configured/first. Adds a frontend regression test (active provider is expanded on load) and backend coverage (config reports is_active/active_provider; selecting a provider round-trips into the next config read). --- .../settings/toolset-config-panel.test.tsx | 57 ++++++++++++++++++- .../src/app/settings/toolset-config-panel.tsx | 17 ++++-- apps/desktop/src/types/hermes.ts | 5 ++ hermes_cli/web_server.py | 11 ++++ tests/hermes_cli/test_web_server.py | 39 +++++++++++++ 5 files changed, 122 insertions(+), 7 deletions(-) diff --git a/apps/desktop/src/app/settings/toolset-config-panel.test.tsx b/apps/desktop/src/app/settings/toolset-config-panel.test.tsx index 89fd5facd..a6dd99ae6 100644 --- a/apps/desktop/src/app/settings/toolset-config-panel.test.tsx +++ b/apps/desktop/src/app/settings/toolset-config-panel.test.tsx @@ -26,6 +26,7 @@ function config(overrides: Partial = {}): ToolsetConfig { return { name: 'tts', has_category: true, + active_provider: null, providers: [ { name: 'Microsoft Edge TTS', @@ -33,7 +34,8 @@ function config(overrides: Partial = {}): ToolsetConfig { tag: 'No API key needed', env_vars: [], post_setup: null, - requires_nous_auth: false + requires_nous_auth: false, + is_active: false }, { name: 'ElevenLabs', @@ -43,7 +45,8 @@ function config(overrides: Partial = {}): ToolsetConfig { { key: 'ELEVENLABS_API_KEY', prompt: 'ElevenLabs API key', url: 'https://x', default: null, is_set: false } ], post_setup: null, - requires_nous_auth: false + requires_nous_auth: false, + is_active: false } ], ...overrides @@ -99,4 +102,54 @@ describe('ToolsetConfigPanel', () => { await waitFor(() => expect(setEnvVar).toHaveBeenCalledWith('ELEVENLABS_API_KEY', 'sk-test-123')) }) + + it('expands the active provider on load, not just the first configured one', async () => { + // ElevenLabs is the active provider per config, even though the keyless + // Edge TTS provider sorts first and is also "configured". The panel must + // honor is_active and expand ElevenLabs (so its API-key field renders) + // rather than defaulting to the first keyless provider. Regression test + // for the GUI showing the wrong provider selected after relaunch. + getToolsetConfig.mockResolvedValue( + config({ + active_provider: 'ElevenLabs', + providers: [ + { + name: 'Microsoft Edge TTS', + badge: 'free', + tag: 'No API key needed', + env_vars: [], + post_setup: null, + requires_nous_auth: false, + is_active: false + }, + { + name: 'ElevenLabs', + badge: 'paid', + tag: 'Most natural voices', + env_vars: [ + { + key: 'ELEVENLABS_API_KEY', + prompt: 'ElevenLabs API key', + url: 'https://x', + default: null, + is_set: true + } + ], + post_setup: null, + requires_nous_auth: false, + is_active: true + } + ] + }) + ) + + const { ToolsetConfigPanel } = await import('./toolset-config-panel') + render() + + // The active provider's env-var field only renders when it's the expanded + // one — so finding it proves ElevenLabs (not Edge TTS) was auto-expanded. + expect(await screen.findByText('ELEVENLABS_API_KEY')).toBeTruthy() + // No provider selection was triggered — this is purely reflecting state. + expect(selectToolsetProvider).not.toHaveBeenCalled() + }) }) diff --git a/apps/desktop/src/app/settings/toolset-config-panel.tsx b/apps/desktop/src/app/settings/toolset-config-panel.tsx index 27381004f..aeb5ac81a 100644 --- a/apps/desktop/src/app/settings/toolset-config-panel.tsx +++ b/apps/desktop/src/app/settings/toolset-config-panel.tsx @@ -195,16 +195,23 @@ export function ToolsetConfigPanel({ toolset, onConfiguredChange }: ToolsetConfi const providers = useMemo(() => cfg?.providers ?? [], [cfg]) - // Default the expanded provider to the first one that is fully configured, - // else the first provider. + // Default the expanded provider to the one actually active in config + // (`is_active` / `cfg.active_provider`, mirroring the CLI picker), then the + // first fully-configured provider, else the first provider. Without this the + // panel highlighted the first keyless provider (e.g. Nous Portal) even when + // the user had already selected another (e.g. DuckDuckGo). useEffect(() => { if (activeProvider || providers.length === 0) { return } - const configured = providers.find(p => providerConfigured(p, envState)) - setActiveProvider((configured ?? providers[0]).name) - }, [activeProvider, providers, envState]) + const selected = + providers.find(p => p.is_active) ?? + (cfg?.active_provider ? providers.find(p => p.name === cfg.active_provider) : undefined) ?? + providers.find(p => providerConfigured(p, envState)) ?? + providers[0] + setActiveProvider(selected.name) + }, [activeProvider, providers, envState, cfg]) async function handleSelect(provider: ToolProvider) { setActiveProvider(provider.name) diff --git a/apps/desktop/src/types/hermes.ts b/apps/desktop/src/types/hermes.ts index f3c1c0c93..550b66deb 100644 --- a/apps/desktop/src/types/hermes.ts +++ b/apps/desktop/src/types/hermes.ts @@ -471,12 +471,17 @@ export interface ToolProvider { env_vars: ToolEnvVar[] post_setup: string | null requires_nous_auth: boolean + /** True when this is the provider currently written to config (mirrors the + * CLI `hermes tools` active-provider detection). */ + is_active: boolean } export interface ToolsetConfig { name: string has_category: boolean providers: ToolProvider[] + /** Name of the currently active provider, or null if none is configured. */ + active_provider: string | null } export interface SessionSearchResult { diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 1757cd2c2..2b97d2b13 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -5183,6 +5183,7 @@ async def get_toolset_config(name: str): from hermes_cli.tools_config import ( TOOL_CATEGORIES, _get_effective_configurable_toolsets, + _is_provider_active, _visible_providers, ) from hermes_cli.config import get_env_value @@ -5194,6 +5195,7 @@ async def get_toolset_config(name: str): config = load_config() cat = TOOL_CATEGORIES.get(name) providers = [] + active_provider = None if cat: for prov in _visible_providers(cat, config, force_fresh=True): env_vars = [ @@ -5206,6 +5208,13 @@ async def get_toolset_config(name: str): } for e in prov.get("env_vars", []) ] + # Surface the same active-provider determination the CLI picker + # uses (``_is_provider_active``) so the GUI highlights the provider + # actually written to config (e.g. web.backend), not just the first + # keyless one in the list. + is_active = _is_provider_active(prov, config, force_fresh=True) + if is_active and active_provider is None: + active_provider = prov["name"] providers.append({ "name": prov["name"], "badge": prov.get("badge", ""), @@ -5213,11 +5222,13 @@ async def get_toolset_config(name: str): "env_vars": env_vars, "post_setup": prov.get("post_setup"), "requires_nous_auth": bool(prov.get("requires_nous_auth")), + "is_active": is_active, }) return { "name": name, "has_category": cat is not None, "providers": providers, + "active_provider": active_provider, } diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index ce885f1b2..8dd39fa1f 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -1523,13 +1523,52 @@ class TestNewEndpoints: assert data["has_category"] is True assert isinstance(data["providers"], list) assert data["providers"], "tts always has at least the built-in providers" + # active_provider is part of the contract so the GUI can highlight the + # provider actually written to config (else it falls back to the first + # keyless one). It's either None or the name of one listed provider. + assert "active_provider" in data + names = {p["name"] for p in data["providers"]} + assert data["active_provider"] is None or data["active_provider"] in names for prov in data["providers"]: assert "name" in prov + assert "is_active" in prov assert "env_vars" in prov assert isinstance(prov["env_vars"], list) for ev in prov["env_vars"]: assert "key" in ev assert "is_set" in ev + # active_provider summarizes the first provider flagged is_active + # (some catalogs list two rows backed by the same config value, e.g. + # Firecrawl cloud + self-hosted both map to web.backend=firecrawl). + active = [p["name"] for p in data["providers"] if p["is_active"]] + if active: + assert data["active_provider"] == active[0] + else: + assert data["active_provider"] is None + + def test_get_toolset_config_reflects_selected_provider(self): + """Selecting a provider is reflected in the next /config read. + + Regression: the GUI's provider panel highlighted the first keyless + provider on relaunch because /config never reported which provider was + actually active. After selecting one, is_active / active_provider must + point at it. + """ + sel = self.client.put( + "/api/tools/toolsets/web/provider", + json={"provider": "Firecrawl Self-Hosted"}, + ) + assert sel.status_code == 200 + + resp = self.client.get("/api/tools/toolsets/web/config") + assert resp.status_code == 200 + data = resp.json() + assert data["active_provider"] == "Firecrawl Self-Hosted" + active = [p["name"] for p in data["providers"] if p["is_active"]] + # The first active row is what the GUI highlights; it must be the + # selected provider. + assert active, "expected at least one provider flagged active" + assert active[0] == "Firecrawl Self-Hosted" def test_get_toolset_config_no_category_toolset(self): """A toolset without a TOOL_CATEGORIES entry returns has_category False."""