Merge pull request #37283 from NousResearch/fix-toolset-provider-selection-display
fix(desktop): reflect active toolset provider in config panel
This commit is contained in:
@@ -26,6 +26,7 @@ function config(overrides: Partial<ToolsetConfig> = {}): ToolsetConfig {
|
|||||||
return {
|
return {
|
||||||
name: 'tts',
|
name: 'tts',
|
||||||
has_category: true,
|
has_category: true,
|
||||||
|
active_provider: null,
|
||||||
providers: [
|
providers: [
|
||||||
{
|
{
|
||||||
name: 'Microsoft Edge TTS',
|
name: 'Microsoft Edge TTS',
|
||||||
@@ -33,7 +34,8 @@ function config(overrides: Partial<ToolsetConfig> = {}): ToolsetConfig {
|
|||||||
tag: 'No API key needed',
|
tag: 'No API key needed',
|
||||||
env_vars: [],
|
env_vars: [],
|
||||||
post_setup: null,
|
post_setup: null,
|
||||||
requires_nous_auth: false
|
requires_nous_auth: false,
|
||||||
|
is_active: false
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: 'ElevenLabs',
|
name: 'ElevenLabs',
|
||||||
@@ -43,7 +45,8 @@ function config(overrides: Partial<ToolsetConfig> = {}): ToolsetConfig {
|
|||||||
{ key: 'ELEVENLABS_API_KEY', prompt: 'ElevenLabs API key', url: 'https://x', default: null, is_set: false }
|
{ key: 'ELEVENLABS_API_KEY', prompt: 'ElevenLabs API key', url: 'https://x', default: null, is_set: false }
|
||||||
],
|
],
|
||||||
post_setup: null,
|
post_setup: null,
|
||||||
requires_nous_auth: false
|
requires_nous_auth: false,
|
||||||
|
is_active: false
|
||||||
}
|
}
|
||||||
],
|
],
|
||||||
...overrides
|
...overrides
|
||||||
@@ -99,4 +102,54 @@ describe('ToolsetConfigPanel', () => {
|
|||||||
|
|
||||||
await waitFor(() => expect(setEnvVar).toHaveBeenCalledWith('ELEVENLABS_API_KEY', 'sk-test-123'))
|
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(<ToolsetConfigPanel onConfiguredChange={vi.fn()} toolset="tts" />)
|
||||||
|
|
||||||
|
// 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()
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -195,16 +195,23 @@ export function ToolsetConfigPanel({ toolset, onConfiguredChange }: ToolsetConfi
|
|||||||
|
|
||||||
const providers = useMemo(() => cfg?.providers ?? [], [cfg])
|
const providers = useMemo(() => cfg?.providers ?? [], [cfg])
|
||||||
|
|
||||||
// Default the expanded provider to the first one that is fully configured,
|
// Default the expanded provider to the one actually active in config
|
||||||
// else the first provider.
|
// (`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(() => {
|
useEffect(() => {
|
||||||
if (activeProvider || providers.length === 0) {
|
if (activeProvider || providers.length === 0) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
const configured = providers.find(p => providerConfigured(p, envState))
|
const selected =
|
||||||
setActiveProvider((configured ?? providers[0]).name)
|
providers.find(p => p.is_active) ??
|
||||||
}, [activeProvider, providers, envState])
|
(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) {
|
async function handleSelect(provider: ToolProvider) {
|
||||||
setActiveProvider(provider.name)
|
setActiveProvider(provider.name)
|
||||||
|
|||||||
@@ -471,12 +471,17 @@ export interface ToolProvider {
|
|||||||
env_vars: ToolEnvVar[]
|
env_vars: ToolEnvVar[]
|
||||||
post_setup: string | null
|
post_setup: string | null
|
||||||
requires_nous_auth: boolean
|
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 {
|
export interface ToolsetConfig {
|
||||||
name: string
|
name: string
|
||||||
has_category: boolean
|
has_category: boolean
|
||||||
providers: ToolProvider[]
|
providers: ToolProvider[]
|
||||||
|
/** Name of the currently active provider, or null if none is configured. */
|
||||||
|
active_provider: string | null
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface SessionSearchResult {
|
export interface SessionSearchResult {
|
||||||
|
|||||||
@@ -5792,6 +5792,7 @@ async def get_toolset_config(name: str):
|
|||||||
from hermes_cli.tools_config import (
|
from hermes_cli.tools_config import (
|
||||||
TOOL_CATEGORIES,
|
TOOL_CATEGORIES,
|
||||||
_get_effective_configurable_toolsets,
|
_get_effective_configurable_toolsets,
|
||||||
|
_is_provider_active,
|
||||||
_visible_providers,
|
_visible_providers,
|
||||||
)
|
)
|
||||||
from hermes_cli.config import get_env_value
|
from hermes_cli.config import get_env_value
|
||||||
@@ -5803,6 +5804,7 @@ async def get_toolset_config(name: str):
|
|||||||
config = load_config()
|
config = load_config()
|
||||||
cat = TOOL_CATEGORIES.get(name)
|
cat = TOOL_CATEGORIES.get(name)
|
||||||
providers = []
|
providers = []
|
||||||
|
active_provider = None
|
||||||
if cat:
|
if cat:
|
||||||
for prov in _visible_providers(cat, config, force_fresh=True):
|
for prov in _visible_providers(cat, config, force_fresh=True):
|
||||||
env_vars = [
|
env_vars = [
|
||||||
@@ -5815,6 +5817,13 @@ async def get_toolset_config(name: str):
|
|||||||
}
|
}
|
||||||
for e in prov.get("env_vars", [])
|
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({
|
providers.append({
|
||||||
"name": prov["name"],
|
"name": prov["name"],
|
||||||
"badge": prov.get("badge", ""),
|
"badge": prov.get("badge", ""),
|
||||||
@@ -5822,11 +5831,13 @@ async def get_toolset_config(name: str):
|
|||||||
"env_vars": env_vars,
|
"env_vars": env_vars,
|
||||||
"post_setup": prov.get("post_setup"),
|
"post_setup": prov.get("post_setup"),
|
||||||
"requires_nous_auth": bool(prov.get("requires_nous_auth")),
|
"requires_nous_auth": bool(prov.get("requires_nous_auth")),
|
||||||
|
"is_active": is_active,
|
||||||
})
|
})
|
||||||
return {
|
return {
|
||||||
"name": name,
|
"name": name,
|
||||||
"has_category": cat is not None,
|
"has_category": cat is not None,
|
||||||
"providers": providers,
|
"providers": providers,
|
||||||
|
"active_provider": active_provider,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -1523,13 +1523,52 @@ class TestNewEndpoints:
|
|||||||
assert data["has_category"] is True
|
assert data["has_category"] is True
|
||||||
assert isinstance(data["providers"], list)
|
assert isinstance(data["providers"], list)
|
||||||
assert data["providers"], "tts always has at least the built-in providers"
|
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"]:
|
for prov in data["providers"]:
|
||||||
assert "name" in prov
|
assert "name" in prov
|
||||||
|
assert "is_active" in prov
|
||||||
assert "env_vars" in prov
|
assert "env_vars" in prov
|
||||||
assert isinstance(prov["env_vars"], list)
|
assert isinstance(prov["env_vars"], list)
|
||||||
for ev in prov["env_vars"]:
|
for ev in prov["env_vars"]:
|
||||||
assert "key" in ev
|
assert "key" in ev
|
||||||
assert "is_set" 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):
|
def test_get_toolset_config_no_category_toolset(self):
|
||||||
"""A toolset without a TOOL_CATEGORIES entry returns has_category False."""
|
"""A toolset without a TOOL_CATEGORIES entry returns has_category False."""
|
||||||
|
|||||||
Reference in New Issue
Block a user