mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-22 16:25:58 +00:00
refactor: make memory.provider schema-driven instead of a 2nd fetch
Addresses review on #69077. The first pass added a second, heavier round-trip (`GET /api/memory` -> `_discover_memory_provider_statuses()`, which imports every provider module and probes install state) just to fill the desktop dropdown, and left `schema.options` for memory.provider dead — three sources of truth for one list. Root cause is narrower: the desktop schema *already* carried a discovery-driven `memory.provider` option list (`_SCHEMA_OVERRIDES` -> `_memory_provider_options()`), but `enumOptionsFor` returned the static `ENUM_OPTIONS['memory.provider']`, which shadowed `schema.options` in config-field.tsx. The only real gap was liveness: `_SCHEMA_OVERRIDES` is frozen at import time, so a provider installed mid-session never showed. Fix at the layer the rest of this stack already uses: - Backend: generalize `_schema_with_voice_provider_options` -> `_schema_with_dynamic_provider_options`, which now also recomputes `memory.provider` options per request (cheap plugin-dir scan via `_memory_provider_options`, plus current-value preservation). Fixes the same staleness for CLI + dashboard, not just desktop. - Frontend: drop the `memory.provider` entry from `ENUM_OPTIONS` so `enumOptionsFor` returns undefined and config-field consumes the discovery-driven `schema.options` directly. No new frontend round-trips. - Remove the now-unnecessary `getMemoryStatus()` fetch/state/wiring in config-settings.tsx (reverted to main). - Fix the stale `helpers.ts` comment ("schema omits memory.provider"). Tests: backend tests for the per-request merge (recomputes discovered providers; preserves a configured-but-undiscovered value); frontend test asserts enumOptionsFor no longer shadows the schema for memory.provider. Co-authored-by: brooklyn! <770929+OutThisLife@users.noreply.github.com>
This commit is contained in:
parent
3493a6c73c
commit
baa3bf3915
6 changed files with 98 additions and 82 deletions
|
|
@ -5,7 +5,7 @@ import { useEffect, useMemo, useRef, useState } from 'react'
|
|||
import { useSearchParams } from 'react-router-dom'
|
||||
|
||||
import { Button } from '@/components/ui/button'
|
||||
import { getElevenLabsVoices, getHermesConfigSchema, getMemoryStatus, saveHermesConfig } from '@/hermes'
|
||||
import { getElevenLabsVoices, getHermesConfigSchema, saveHermesConfig } from '@/hermes'
|
||||
import { useI18n } from '@/i18n'
|
||||
import { $keepAwake, setKeepAwake } from '@/store/keep-awake'
|
||||
import { notify, notifyError } from '@/store/notifications'
|
||||
|
|
@ -76,10 +76,6 @@ export function ConfigSettings({
|
|||
const schema = schemaResponse?.fields ?? null
|
||||
const [elevenLabsVoiceOptions, setElevenLabsVoiceOptions] = useState<string[] | null>(null)
|
||||
const [elevenLabsVoiceLabels, setElevenLabsVoiceLabels] = useState<Record<string, string>>({})
|
||||
// Discovered memory providers (bundled + user-installed + pip), so the
|
||||
// memory.provider dropdown reflects what the backend actually serves rather
|
||||
// than a hardcoded subset. null until the first fetch resolves.
|
||||
const [memoryProviderOptions, setMemoryProviderOptions] = useState<string[] | null>(null)
|
||||
const saveVersionRef = useRef(0)
|
||||
const savedDiscoverySignatureRef = useRef<string | undefined>(undefined)
|
||||
const [saveVersion, setSaveVersion] = useState(0)
|
||||
|
|
@ -130,27 +126,6 @@ export function ConfigSettings({
|
|||
return () => void (cancelled = true)
|
||||
}, [])
|
||||
|
||||
useEffect(() => {
|
||||
let cancelled = false
|
||||
|
||||
getMemoryStatus()
|
||||
.then(result => {
|
||||
if (cancelled) {
|
||||
return
|
||||
}
|
||||
|
||||
// Empty sentinel first (built-in only), then every discovered plugin.
|
||||
setMemoryProviderOptions(['', ...result.providers.map(provider => provider.name)])
|
||||
})
|
||||
.catch(() => {
|
||||
if (!cancelled) {
|
||||
setMemoryProviderOptions(null)
|
||||
}
|
||||
})
|
||||
|
||||
return () => void (cancelled = true)
|
||||
}, [])
|
||||
|
||||
useEffect(() => {
|
||||
if (!config || saveVersion === 0) {
|
||||
return
|
||||
|
|
@ -333,9 +308,7 @@ export function ConfigSettings({
|
|||
enumOptions={
|
||||
key === 'tts.elevenlabs.voice_id'
|
||||
? enumOptionsFor(key, getNested(config, key), config, elevenLabsVoiceOptions ?? undefined)
|
||||
: key === 'memory.provider'
|
||||
? enumOptionsFor(key, getNested(config, key), config, memoryProviderOptions ?? undefined)
|
||||
: enumOptionsFor(key, getNested(config, key), config)
|
||||
: enumOptionsFor(key, getNested(config, key), config)
|
||||
}
|
||||
onChange={value => updateConfig(setNested(config, key, value))}
|
||||
optionLabels={key === 'tts.elevenlabs.voice_id' ? elevenLabsVoiceLabels : undefined}
|
||||
|
|
|
|||
|
|
@ -247,12 +247,11 @@ export const ENUM_OPTIONS: Record<string, string[]> = {
|
|||
'code_execution.mode': ['project', 'strict'],
|
||||
'context.engine': ['compressor', 'default', 'custom'],
|
||||
'delegation.reasoning_effort': ['', 'minimal', 'low', 'medium', 'high', 'xhigh', 'max', 'ultra'],
|
||||
// Built-in memory is not a provider plugin: the empty sentinel renders as
|
||||
// "Built-in only" and a legacy literal `builtin` value is only kept visible
|
||||
// via enumOptionsFor's current-value passthrough (#49513). This static list
|
||||
// is only a pre-load fallback: config-settings.tsx feeds enumOptionsFor the
|
||||
// live discovered set (getMemoryStatus) so user-installed/pip providers show.
|
||||
'memory.provider': ['', 'honcho', 'hindsight'],
|
||||
// NOTE: memory.provider is intentionally NOT listed here. Its options are
|
||||
// discovery-driven and served by the backend config schema (merged
|
||||
// per-request in web_server._schema_with_dynamic_provider_options), so
|
||||
// config-field consumes schema.options directly — a static list here would
|
||||
// shadow that and hide user-installed/pip providers (#49513).
|
||||
// Terminal execution backends — kept in sync with the dispatch ladder in
|
||||
// tools/terminal_tool.py::_create_environment (local/docker/singularity/
|
||||
// modal/daytona/ssh). Remote backends need extra env (image, tokens, host).
|
||||
|
|
|
|||
|
|
@ -30,37 +30,12 @@ describe('settings helpers', () => {
|
|||
expect(fieldCopyForSchemaKey(FIELD_DESCRIPTIONS, 'desktop.repo_scan_exclude_paths')).toBeTruthy()
|
||||
})
|
||||
|
||||
it('lists the desktop memory provider options in their declared order', () => {
|
||||
const options = enumOptionsFor('memory.provider', '', {})
|
||||
|
||||
// Built-in memory is not a provider plugin; the empty sentinel is the
|
||||
// only built-in-shaped entry (#49513).
|
||||
expect(options).toEqual(['', 'honcho', 'hindsight'])
|
||||
})
|
||||
|
||||
it('keeps a legacy literal builtin value visible as the current selection', () => {
|
||||
const options = enumOptionsFor('memory.provider', 'builtin', {})
|
||||
|
||||
expect(options).toEqual(['', 'honcho', 'hindsight', 'builtin'])
|
||||
})
|
||||
|
||||
it('prefers the discovered provider set over the static fallback for memory.provider', () => {
|
||||
// config-settings.tsx passes the live getMemoryStatus() providers as
|
||||
// dynamicOptions; user-installed/pip providers must appear, not just the
|
||||
// hardcoded honcho/hindsight subset.
|
||||
const discovered = ['', 'honcho', 'hindsight', 'mem0', 'supermemory']
|
||||
const options = enumOptionsFor('memory.provider', '', {}, discovered)
|
||||
|
||||
expect(options).toEqual(discovered)
|
||||
})
|
||||
|
||||
it('keeps the current memory.provider value visible even if discovery omits it', () => {
|
||||
// A provider selected in config but not (yet) returned by discovery must
|
||||
// still render as the current selection rather than silently vanishing.
|
||||
const discovered = ['', 'honcho']
|
||||
const options = enumOptionsFor('memory.provider', 'hindsight', {}, discovered)
|
||||
|
||||
expect(options).toEqual(['', 'honcho', 'hindsight'])
|
||||
it('does not shadow the backend schema options for memory.provider', () => {
|
||||
// memory.provider options are discovery-driven and served by the backend
|
||||
// config schema (merged per-request); enumOptionsFor must return undefined
|
||||
// so config-field consumes schema.options instead of a stale static list.
|
||||
expect(enumOptionsFor('memory.provider', '', {})).toBeUndefined()
|
||||
expect(enumOptionsFor('memory.provider', 'honcho', {})).toBeUndefined()
|
||||
})
|
||||
|
||||
describe('isExternalMemoryProvider', () => {
|
||||
|
|
|
|||
|
|
@ -113,7 +113,7 @@ export function inferFieldSchema(value: unknown): ConfigFieldSchema {
|
|||
return { type: 'string' }
|
||||
}
|
||||
|
||||
// Backend schema omits some declared keys (e.g. memory.provider); config presence is the availability signal.
|
||||
// Backend schema omits some declared keys; config presence is the availability signal.
|
||||
export function sectionFieldEntries(
|
||||
schema: Record<string, ConfigFieldSchema>,
|
||||
config: HermesConfigRecord
|
||||
|
|
|
|||
|
|
@ -1136,28 +1136,59 @@ def _custom_provider_options(
|
|||
return names
|
||||
|
||||
|
||||
def _schema_with_voice_provider_options() -> Dict[str, Dict[str, Any]]:
|
||||
"""Return CONFIG_SCHEMA with per-request voice provider options merged.
|
||||
def _memory_provider_schema_options(cfg: Dict[str, Any]) -> List[str]:
|
||||
"""Discovered memory providers for a per-request schema merge.
|
||||
|
||||
Computed at request time (not import time) so options reflect the
|
||||
CURRENT config.yaml — including providers added after the server
|
||||
started, and the profile-scoped config when the request carries a
|
||||
``profile`` param. The module-level ``CONFIG_SCHEMA`` is never mutated;
|
||||
entries that change are shallow-copied onto a copied mapping.
|
||||
Reuses the cheap directory scan of :func:`_memory_provider_options` and
|
||||
additionally preserves the currently-configured provider, so a value
|
||||
selected in config but not (yet) discoverable — e.g. a plugin removed from
|
||||
disk — never silently vanishes from the dropdown.
|
||||
"""
|
||||
options = _memory_provider_options()
|
||||
current = _normalize_memory_provider_name(
|
||||
cfg.get("memory", {}).get("provider") if isinstance(cfg.get("memory"), dict) else None
|
||||
)
|
||||
if current and current not in options:
|
||||
options = [*options, current]
|
||||
return options
|
||||
|
||||
|
||||
def _schema_with_dynamic_provider_options() -> Dict[str, Dict[str, Any]]:
|
||||
"""Return CONFIG_SCHEMA with per-request discovery-driven options merged.
|
||||
|
||||
Some ``*.provider`` selects have options that are discovered at runtime
|
||||
(voice backends via the tts/stt registries + config.yaml command
|
||||
providers; memory providers via a plugin-dir scan). The module-level
|
||||
``_SCHEMA_OVERRIDES`` freezes those lists at import time, so a provider
|
||||
installed after the server started never appears. This recomputes them at
|
||||
request time — reflecting the CURRENT config.yaml, the profile-scoped
|
||||
config when the request carries a ``profile`` param, and mid-session
|
||||
plugin installs — for every surface that reads the schema (desktop, CLI,
|
||||
dashboard), with no extra frontend round-trips.
|
||||
|
||||
The module-level ``CONFIG_SCHEMA`` is never mutated; entries that change
|
||||
are shallow-copied onto a copied mapping.
|
||||
"""
|
||||
try:
|
||||
cfg = load_config()
|
||||
except Exception: # pragma: no cover - schema must survive config errors
|
||||
return CONFIG_SCHEMA
|
||||
overlay: Dict[str, Dict[str, Any]] = {}
|
||||
for kind in ("tts", "stt"):
|
||||
key = f"{kind}.provider"
|
||||
|
||||
def _merge(key: str, merged: List[str]) -> None:
|
||||
entry = CONFIG_SCHEMA.get(key)
|
||||
if not isinstance(entry, dict) or not isinstance(entry.get("options"), list):
|
||||
continue
|
||||
merged = _custom_provider_options(kind, list(entry["options"]), cfg)
|
||||
return
|
||||
if merged != entry["options"]:
|
||||
overlay[key] = {**entry, "options": merged}
|
||||
|
||||
for kind in ("tts", "stt"):
|
||||
entry = CONFIG_SCHEMA.get(f"{kind}.provider")
|
||||
if isinstance(entry, dict) and isinstance(entry.get("options"), list):
|
||||
_merge(f"{kind}.provider", _custom_provider_options(kind, list(entry["options"]), cfg))
|
||||
|
||||
_merge("memory.provider", _memory_provider_schema_options(cfg))
|
||||
|
||||
if not overlay:
|
||||
return CONFIG_SCHEMA
|
||||
fields = dict(CONFIG_SCHEMA)
|
||||
|
|
@ -6187,11 +6218,11 @@ async def get_defaults():
|
|||
|
||||
@app.get("/api/config/schema")
|
||||
async def get_schema(profile: Optional[str] = None):
|
||||
# Voice provider options are merged per-request so user-declared
|
||||
# command providers (tts.providers.* / stt.providers.*) added after
|
||||
# server start still show up, scoped to the requested profile's config.
|
||||
# Discovery-driven provider options (voice command providers + memory
|
||||
# provider plugins) are merged per-request so providers added after server
|
||||
# start still show up, scoped to the requested profile's config.
|
||||
with _config_profile_scope(profile):
|
||||
fields = _schema_with_voice_provider_options()
|
||||
fields = _schema_with_dynamic_provider_options()
|
||||
return {"fields": fields, "category_order": _CATEGORY_ORDER}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -4633,6 +4633,44 @@ class TestBuildSchemaFromConfig:
|
|||
missing = set(list_memory_provider_names()) - options
|
||||
assert missing == set(), f"discovered providers missing from schema options: {missing}"
|
||||
|
||||
def test_dynamic_merge_recomputes_memory_provider_options(self, monkeypatch):
|
||||
"""The per-request schema merge re-discovers memory providers.
|
||||
|
||||
The import-time _SCHEMA_OVERRIDES freezes the list at server start;
|
||||
_schema_with_dynamic_provider_options must recompute it so a provider
|
||||
installed mid-session is selectable without a restart.
|
||||
"""
|
||||
from hermes_cli import web_server
|
||||
|
||||
monkeypatch.setattr(web_server, "load_config", lambda: {"memory": {"provider": "honcho"}})
|
||||
monkeypatch.setattr(
|
||||
web_server,
|
||||
"_memory_provider_options",
|
||||
lambda: ["", "honcho", "hindsight", "freshly_installed"],
|
||||
)
|
||||
|
||||
fields = web_server._schema_with_dynamic_provider_options()
|
||||
|
||||
assert "freshly_installed" in fields["memory.provider"]["options"]
|
||||
# The entry is copied, not mutated in place, and keeps its select type.
|
||||
assert fields["memory.provider"]["type"] == "select"
|
||||
assert web_server.CONFIG_SCHEMA["memory.provider"] is not fields["memory.provider"]
|
||||
|
||||
def test_dynamic_merge_preserves_configured_memory_provider(self, monkeypatch):
|
||||
"""A configured-but-undiscovered provider stays visible as the selection.
|
||||
|
||||
e.g. the plugin dir was removed but config still points at it — the
|
||||
dropdown must not silently drop the active value.
|
||||
"""
|
||||
from hermes_cli import web_server
|
||||
|
||||
monkeypatch.setattr(web_server, "load_config", lambda: {"memory": {"provider": "gone_from_disk"}})
|
||||
monkeypatch.setattr(web_server, "_memory_provider_options", lambda: ["", "honcho"])
|
||||
|
||||
fields = web_server._schema_with_dynamic_provider_options()
|
||||
|
||||
assert "gone_from_disk" in fields["memory.provider"]["options"]
|
||||
|
||||
def test_approvals_mode_options_match_config_values(self):
|
||||
"""approvals.mode select options must match the values accepted by config.py.
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue