From a2b8e430e851bd7c77600fbafe3bc6cd5035e616 Mon Sep 17 00:00:00 2001 From: emozilla Date: Tue, 2 Jun 2026 05:11:52 -0400 Subject: [PATCH] refactor(desktop): consolidate skills + tools management into one pane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The left-nav Skills pane and Settings > Skills & Tools rendered the same getSkills()/getToolsets() data with the same helpers and toggles — genuine duplication that drifted (different default category labels, sort orders). Make the left pane the single home: it keeps its category-tabbed browsing and now gains the functional bits it lacked — a real toolset enable/disable switch (was a read-only pill) and the expandable ToolsetConfigPanel for provider selection + per-key credential config. Remove the Tools section from Settings (nav item, view branch, query slot, type union entries) and delete tools-settings.tsx, migrating its toggle coverage into the skills pane test. Relabel the entry point to 'Skills & Tools' in the sidebar and command center. --- apps/desktop/src/app/chat/sidebar/index.tsx | 2 +- apps/desktop/src/app/command-center/index.tsx | 2 +- apps/desktop/src/app/settings/constants.ts | 8 +- apps/desktop/src/app/settings/index.tsx | 17 +- .../src/app/settings/tools-settings.tsx | 229 ------------------ apps/desktop/src/app/settings/types.ts | 4 +- .../index.test.tsx} | 38 ++- apps/desktop/src/app/skills/index.tsx | 56 ++++- 8 files changed, 91 insertions(+), 265 deletions(-) delete mode 100644 apps/desktop/src/app/settings/tools-settings.tsx rename apps/desktop/src/app/{settings/tools-settings.test.tsx => skills/index.test.tsx} (60%) diff --git a/apps/desktop/src/app/chat/sidebar/index.tsx b/apps/desktop/src/app/chat/sidebar/index.tsx index 521d6d39ee4..53fb9688e3e 100644 --- a/apps/desktop/src/app/chat/sidebar/index.tsx +++ b/apps/desktop/src/app/chat/sidebar/index.tsx @@ -73,7 +73,7 @@ const SIDEBAR_NAV: SidebarNavItem[] = [ icon: props => , action: 'new-session' }, - { id: 'skills', label: 'Skills', icon: props => , route: SKILLS_ROUTE }, + { id: 'skills', label: 'Skills & Tools', icon: props => , route: SKILLS_ROUTE }, { id: 'messaging', label: 'Messaging', icon: props => , route: MESSAGING_ROUTE }, { id: 'artifacts', label: 'Artifacts', icon: props => , route: ARTIFACTS_ROUTE } ] diff --git a/apps/desktop/src/app/command-center/index.tsx b/apps/desktop/src/app/command-center/index.tsx index 588a84ea17a..c61ed8e5405 100644 --- a/apps/desktop/src/app/command-center/index.tsx +++ b/apps/desktop/src/app/command-center/index.tsx @@ -115,7 +115,7 @@ interface SectionSearchEntry { const NAVIGATION_SEARCH_ENTRIES: readonly NavigationSearchEntry[] = [ { id: 'nav-new-chat', route: NEW_CHAT_ROUTE, title: 'New session', detail: 'Start a fresh session' }, { id: 'nav-settings', route: SETTINGS_ROUTE, title: 'Settings', detail: 'Configure Hermes desktop' }, - { id: 'nav-skills', route: SKILLS_ROUTE, title: 'Skills', detail: 'Enable and inspect skills' }, + { id: 'nav-skills', route: SKILLS_ROUTE, title: 'Skills & Tools', detail: 'Enable skills, toolsets, and providers' }, { id: 'nav-messaging', route: MESSAGING_ROUTE, diff --git a/apps/desktop/src/app/settings/constants.ts b/apps/desktop/src/app/settings/constants.ts index 887bf256dbd..93c1e8374a3 100644 --- a/apps/desktop/src/app/settings/constants.ts +++ b/apps/desktop/src/app/settings/constants.ts @@ -311,15 +311,11 @@ export const MODE_OPTIONS: ModeOption[] = [ { id: 'system', label: 'System', description: 'Follow OS appearance', icon: Monitor } ] -export const SEARCH_PLACEHOLDER: Record< - 'about' | 'config' | 'gateway' | 'keys' | 'mcp' | 'sessions' | 'tools', - string -> = { +export const SEARCH_PLACEHOLDER: Record<'about' | 'config' | 'gateway' | 'keys' | 'mcp' | 'sessions', string> = { about: 'About Hermes Desktop', config: 'Search settings...', gateway: 'Gateway connection...', keys: 'Search API keys...', mcp: 'Search MCP servers...', - sessions: 'Search archived sessions...', - tools: 'Search skills and tools...' + sessions: 'Search archived sessions...' } diff --git a/apps/desktop/src/app/settings/index.tsx b/apps/desktop/src/app/settings/index.tsx index a865186be6f..edad8ca623b 100644 --- a/apps/desktop/src/app/settings/index.tsx +++ b/apps/desktop/src/app/settings/index.tsx @@ -3,7 +3,7 @@ import { useEffect, useRef, useState } from 'react' import { getHermesConfigDefaults, getHermesConfigRecord, saveHermesConfig } from '@/hermes' import { triggerHaptic } from '@/lib/haptics' -import { Archive, Globe, Info, KeyRound, Package, Wrench } from '@/lib/icons' +import { Archive, Globe, Info, KeyRound, Wrench } from '@/lib/icons' import { notifyError } from '@/store/notifications' import { useRouteEnumParam } from '../hooks/use-route-enum-param' @@ -20,7 +20,6 @@ import { GatewaySettings } from './gateway-settings' import { KeysSettings } from './keys-settings' import { McpSettings } from './mcp-settings' import { SessionsSettings } from './sessions-settings' -import { ToolsSettings } from './tools-settings' import type { SettingsPageProps, SettingsQueryKey, SettingsView as SettingsViewId } from './types' const SETTINGS_VIEWS: readonly SettingsViewId[] = [ @@ -29,7 +28,6 @@ const SETTINGS_VIEWS: readonly SettingsViewId[] = [ 'keys', 'mcp', 'sessions', - 'tools', 'about' ] @@ -42,8 +40,7 @@ export function SettingsView({ gateway, onClose, onConfigSaved }: SettingsPagePr gateway: '', keys: '', mcp: '', - sessions: '', - tools: '' + sessions: '' }) const searchInputRef = useRef(null) @@ -140,12 +137,6 @@ export function SettingsView({ gateway, onClose, onConfigSaved }: SettingsPagePr label="API Keys" onClick={() => setActiveView('keys')} /> - setActiveView('tools')} - /> ) : activeView === 'mcp' ? ( - ) : activeView === 'sessions' ? ( - ) : ( - + )} diff --git a/apps/desktop/src/app/settings/tools-settings.tsx b/apps/desktop/src/app/settings/tools-settings.tsx deleted file mode 100644 index 8e6d63c3a56..00000000000 --- a/apps/desktop/src/app/settings/tools-settings.tsx +++ /dev/null @@ -1,229 +0,0 @@ -import { useCallback, useEffect, useMemo, useState } from 'react' - -import { Switch } from '@/components/ui/switch' -import { getSkills, getToolsets, toggleSkill, toggleToolset } from '@/hermes' -import { Brain, Wrench } from '@/lib/icons' -import { notify, notifyError } from '@/store/notifications' -import type { SkillInfo, ToolsetInfo } from '@/types/hermes' - -import { asText, includesQuery, prettyName, toolNames } from './helpers' -import { ListRow, LoadingState, Pill, SectionHeading, SettingsContent } from './primitives' -import { ToolsetConfigPanel } from './toolset-config-panel' -import type { SearchProps } from './types' - -export function ToolsSettings({ query }: SearchProps) { - const [skills, setSkills] = useState(null) - const [toolsets, setToolsets] = useState(null) - const [savingSkill, setSavingSkill] = useState(null) - const [savingToolset, setSavingToolset] = useState(null) - const [expandedToolset, setExpandedToolset] = useState(null) - - useEffect(() => { - let cancelled = false - Promise.all([getSkills(), getToolsets()]) - .then(([s, t]) => { - if (cancelled) { - return - } - - setSkills(s) - setToolsets(t) - }) - .catch(err => notifyError(err, 'Capabilities failed to load')) - - return () => void (cancelled = true) - }, []) - - const refreshToolsets = useCallback(() => { - getToolsets() - .then(setToolsets) - .catch(err => notifyError(err, 'Toolsets failed to refresh')) - }, []) - - const filteredSkills = useMemo(() => { - if (!skills) { - return [] - } - - const q = query.trim().toLowerCase() - - return skills - .filter(s => !q || includesQuery(s.name, q) || includesQuery(s.description, q) || includesQuery(s.category, q)) - .sort( - (a, b) => asText(a.category).localeCompare(asText(b.category)) || asText(a.name).localeCompare(asText(b.name)) - ) - }, [query, skills]) - - const filteredToolsets = useMemo(() => { - if (!toolsets) { - return [] - } - - const q = query.trim().toLowerCase() - - return toolsets - .filter(t => { - if (!q) { - return true - } - - return ( - includesQuery(t.name, q) || - includesQuery(t.label, q) || - includesQuery(t.description, q) || - toolNames(t).some(n => includesQuery(n, q)) - ) - }) - .sort((a, b) => asText(a.label || a.name).localeCompare(asText(b.label || b.name))) - }, [query, toolsets]) - - const skillGroups = useMemo(() => { - const groups = new Map() - - for (const skill of filteredSkills) { - const cat = asText(skill.category) || 'other' - groups.set(cat, [...(groups.get(cat) ?? []), skill]) - } - - return Array.from(groups).sort(([a], [b]) => a.localeCompare(b)) - }, [filteredSkills]) - - async function handleToggleSkill(skill: SkillInfo, enabled: boolean) { - setSavingSkill(skill.name) - - try { - await toggleSkill(skill.name, enabled) - setSkills(c => c?.map(s => (s.name === skill.name ? { ...s, enabled } : s)) ?? c) - notify({ - kind: 'success', - title: enabled ? 'Skill enabled' : 'Skill disabled', - message: `${skill.name} applies to new sessions.` - }) - } catch (err) { - notifyError(err, `Failed to update ${skill.name}`) - } finally { - setSavingSkill(null) - } - } - - async function handleToggleToolset(toolset: ToolsetInfo, enabled: boolean) { - setSavingToolset(toolset.name) - - try { - await toggleToolset(toolset.name, enabled) - setToolsets(c => c?.map(t => (t.name === toolset.name ? { ...t, enabled, available: enabled } : t)) ?? c) - notify({ - kind: 'success', - title: enabled ? 'Toolset enabled' : 'Toolset disabled', - message: `${asText(toolset.label || toolset.name)} applies to new sessions.` - }) - } catch (err) { - notifyError(err, `Failed to update ${asText(toolset.label || toolset.name)}`) - } finally { - setSavingToolset(null) - } - } - - if (!skills || !toolsets) { - return - } - - return ( - -
- s.enabled).length} enabled`} title="Skills" /> - {skillGroups.map(([category, list]) => ( -
-
- {prettyName(category)} -
-
- {list.map(skill => ( - void handleToggleSkill(skill, c)} - /> - } - description={asText(skill.description)} - key={asText(skill.name)} - title={asText(skill.name)} - /> - ))} -
-
- ))} -
- -
- t.enabled).length} enabled`} - title="Toolsets" - /> -
- {filteredToolsets.map(toolset => { - const tools = toolNames(toolset) - const label = asText(toolset.label || toolset.name) - const expanded = expandedToolset === toolset.name - - return ( - - - void handleToggleToolset(toolset, c)} - /> -
- } - below={ - <> - {tools.length > 0 && ( -
- {tools.slice(0, 10).map(t => ( - - {t} - - ))} - {tools.length > 10 && ( - - +{tools.length - 10} more - - )} -
- )} - {expanded && ( - - )} - - } - description={asText(toolset.description)} - key={asText(toolset.name) || label} - title={label} - /> - ) - })} -
- - - ) -} diff --git a/apps/desktop/src/app/settings/types.ts b/apps/desktop/src/app/settings/types.ts index 076ed9327a9..b0f4c48657a 100644 --- a/apps/desktop/src/app/settings/types.ts +++ b/apps/desktop/src/app/settings/types.ts @@ -4,8 +4,8 @@ import type { HermesGateway } from '@/hermes' import type { IconComponent } from '@/lib/icons' import type { EnvVarInfo } from '@/types/hermes' -export type SettingsView = 'about' | 'gateway' | 'keys' | 'mcp' | 'sessions' | 'tools' | `config:${string}` -export type SettingsQueryKey = 'about' | 'config' | 'gateway' | 'keys' | 'mcp' | 'sessions' | 'tools' +export type SettingsView = 'about' | 'gateway' | 'keys' | 'mcp' | 'sessions' | `config:${string}` +export type SettingsQueryKey = 'about' | 'config' | 'gateway' | 'keys' | 'mcp' | 'sessions' export type EnvPatch = Partial> export interface SettingsPageProps { diff --git a/apps/desktop/src/app/settings/tools-settings.test.tsx b/apps/desktop/src/app/skills/index.test.tsx similarity index 60% rename from apps/desktop/src/app/settings/tools-settings.test.tsx rename to apps/desktop/src/app/skills/index.test.tsx index f160a70ab1a..1243cc1d87a 100644 --- a/apps/desktop/src/app/settings/tools-settings.test.tsx +++ b/apps/desktop/src/app/skills/index.test.tsx @@ -1,16 +1,24 @@ import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { MemoryRouter } from 'react-router-dom' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const getSkills = vi.fn() const getToolsets = vi.fn() const toggleSkill = vi.fn() const toggleToolset = vi.fn() +const getToolsetConfig = vi.fn() +const selectToolsetProvider = vi.fn() vi.mock('@/hermes', () => ({ getSkills: () => getSkills(), getToolsets: () => getToolsets(), toggleSkill: (name: string, enabled: boolean) => toggleSkill(name, enabled), - toggleToolset: (name: string, enabled: boolean) => toggleToolset(name, enabled) + toggleToolset: (name: string, enabled: boolean) => toggleToolset(name, enabled), + getToolsetConfig: (name: string) => getToolsetConfig(name), + selectToolsetProvider: (toolset: string, provider: string) => selectToolsetProvider(toolset, provider), + deleteEnvVar: vi.fn(), + revealEnvVar: vi.fn(), + setEnvVar: vi.fn() })) // Notifications hit nanostores/timers we don't care about here. @@ -32,10 +40,21 @@ function toolset(overrides: Record = {}) { } } +function renderSkills() { + return import('./index').then(({ SkillsView }) => + render( + + + + ) + ) +} + beforeEach(() => { getSkills.mockResolvedValue([]) getToolsets.mockResolvedValue([toolset()]) toggleToolset.mockResolvedValue({ ok: true, name: 'web', enabled: false }) + getToolsetConfig.mockResolvedValue({ has_category: false, active_provider: null, providers: [] }) }) afterEach(() => { @@ -43,10 +62,9 @@ afterEach(() => { vi.clearAllMocks() }) -describe('ToolsSettings toolset toggle', () => { +describe('SkillsView toolset management', () => { it('renders a switch for each toolset and toggles it off', async () => { - const { ToolsSettings } = await import('./tools-settings') - render() + await renderSkills() const sw = await screen.findByRole('switch', { name: 'Toggle Web Search toolset' }) expect(sw.getAttribute('aria-checked')).toBe('true') @@ -57,10 +75,18 @@ describe('ToolsSettings toolset toggle', () => { }) it('keeps the configured pill alongside the switch', async () => { - const { ToolsSettings } = await import('./tools-settings') - render() + await renderSkills() await screen.findByRole('switch', { name: 'Toggle Web Search toolset' }) expect(screen.getByText('Configured')).toBeTruthy() }) + + it('expands the provider config panel when the configured pill is clicked', async () => { + await renderSkills() + + const configureBtn = await screen.findByRole('button', { name: 'Configure Web Search' }) + fireEvent.click(configureBtn) + + await waitFor(() => expect(getToolsetConfig).toHaveBeenCalledWith('web')) + }) }) diff --git a/apps/desktop/src/app/skills/index.tsx b/apps/desktop/src/app/skills/index.tsx index f02bd5ffb97..ace615c28a4 100644 --- a/apps/desktop/src/app/skills/index.tsx +++ b/apps/desktop/src/app/skills/index.tsx @@ -6,7 +6,7 @@ import { Button } from '@/components/ui/button' import { Codicon } from '@/components/ui/codicon' import { Switch } from '@/components/ui/switch' import { TextTab, TextTabMeta } from '@/components/ui/text-tab' -import { getSkills, getToolsets, toggleSkill } from '@/hermes' +import { getSkills, getToolsets, toggleSkill, toggleToolset } from '@/hermes' import { cn } from '@/lib/utils' import { notify, notifyError } from '@/store/notifications' import type { SkillInfo, ToolsetInfo } from '@/types/hermes' @@ -14,6 +14,7 @@ import type { SkillInfo, ToolsetInfo } from '@/types/hermes' import { useRouteEnumParam } from '../hooks/use-route-enum-param' import { PageSearchShell } from '../page-search-shell' import { asText, includesQuery, prettyName, toolNames } from '../settings/helpers' +import { ToolsetConfigPanel } from '../settings/toolset-config-panel' import type { SetStatusbarItemGroup } from '../shell/statusbar-controls' const SKILLS_MODES = ['skills', 'toolsets'] as const @@ -73,6 +74,8 @@ export function SkillsView({ setStatusbarItemGroup: _setStatusbarItemGroup, ...p const [activeCategory, setActiveCategory] = useState(null) const [refreshing, setRefreshing] = useState(false) const [savingSkill, setSavingSkill] = useState(null) + const [savingToolset, setSavingToolset] = useState(null) + const [expandedToolset, setExpandedToolset] = useState(null) const refreshCapabilities = useCallback(async () => { setRefreshing(true) @@ -88,6 +91,12 @@ export function SkillsView({ setStatusbarItemGroup: _setStatusbarItemGroup, ...p } }, []) + const refreshToolsets = useCallback(() => { + getToolsets() + .then(setToolsets) + .catch(err => notifyError(err, 'Toolsets failed to refresh')) + }, []) + useEffect(() => { void refreshCapabilities() }, [refreshCapabilities]) @@ -148,6 +157,26 @@ export function SkillsView({ setStatusbarItemGroup: _setStatusbarItemGroup, ...p } } + async function handleToggleToolset(toolset: ToolsetInfo, enabled: boolean) { + setSavingToolset(toolset.name) + + try { + await toggleToolset(toolset.name, enabled) + setToolsets(current => + current?.map(row => (row.name === toolset.name ? { ...row, enabled, available: enabled } : row)) ?? current + ) + notify({ + kind: 'success', + title: enabled ? 'Toolset enabled' : 'Toolset disabled', + message: `${asText(toolset.label || toolset.name)} applies to new sessions.` + }) + } catch (err) { + notifyError(err, `Failed to update ${asText(toolset.label || toolset.name)}`) + } finally { + setSavingToolset(null) + } + } + return ( { const tools = toolNames(toolset) const label = asText(toolset.label || toolset.name) + const expanded = expandedToolset === toolset.name return (
{label}
-
- {toolset.enabled ? 'Enabled' : 'Disabled'} - - {toolset.configured ? 'Configured' : 'Needs keys'} - +
+ + void handleToggleToolset(toolset, checked)} + />

@@ -275,6 +318,7 @@ export function SkillsView({ setStatusbarItemGroup: _setStatusbarItemGroup, ...p ))}

)} + {expanded && }
) })}