diff --git a/apps/desktop/src/app/settings/memory/field-control.tsx b/apps/desktop/src/app/settings/memory/field-control.tsx index 75b9f4e21dc8..d4d943a66268 100644 --- a/apps/desktop/src/app/settings/memory/field-control.tsx +++ b/apps/desktop/src/app/settings/memory/field-control.tsx @@ -10,8 +10,7 @@ import { CONTROL_TEXT } from '../constants' // Fade the placeholder well below set values so example text never reads as data. const FIELD_INPUT = `font-mono ${CONTROL_TEXT} placeholder:text-muted-foreground/45` -/** Generic control for one declared provider field, dispatching on its kind. - * Values are edited as strings; the backend coerces them to native types. */ +// Values are edited as strings; the backend coerces them to native types. export function FieldControl({ field, value, diff --git a/apps/desktop/src/app/settings/memory/provider-config-modal.test.tsx b/apps/desktop/src/app/settings/memory/provider-config-modal.test.tsx index 7505dbb58064..91eea2e1e7e2 100644 --- a/apps/desktop/src/app/settings/memory/provider-config-modal.test.tsx +++ b/apps/desktop/src/app/settings/memory/provider-config-modal.test.tsx @@ -86,8 +86,7 @@ describe('ProviderConfigModal', () => { fireEvent.click(await screen.findByRole('switch')) fireEvent.click(screen.getByRole('button', { name: 'Save changes' })) - // Untouched fields stay unsubmitted so a save never ratifies rendered - // defaults the backend does not actually store. + // A save must never ratify rendered defaults the backend does not store. await waitFor(() => expect(saveMemoryProviderConfig).toHaveBeenCalledWith('honcho', { saveMessages: 'false' })) await waitFor(() => expect(onSaved).toHaveBeenCalled()) expect(onOpenChange).toHaveBeenCalledWith(false) diff --git a/apps/desktop/src/app/settings/memory/provider-config-modal.tsx b/apps/desktop/src/app/settings/memory/provider-config-modal.tsx index 6bc9ae216137..3be76ea29240 100644 --- a/apps/desktop/src/app/settings/memory/provider-config-modal.tsx +++ b/apps/desktop/src/app/settings/memory/provider-config-modal.tsx @@ -18,13 +18,12 @@ import type { MemoryProviderConfig, MemoryProviderField } from '@/types/hermes' import { FieldControl } from './field-control' import { ListRow } from '../primitives' -/** Seed every editable field (inline and modal-only). Secrets start blank — - * their value is never returned and submitting blank keeps the stored one. */ +// Secrets seed blank: values are write-only and blank keeps the stored one. function seedAll(config: MemoryProviderConfig): Record { return Object.fromEntries(config.fields.map(field => [field.key, field.kind === 'secret' ? '' : field.value])) } -/** Group fields in declared order, preserving first-seen group sequence. */ +// Group fields in declared order, preserving first-seen group sequence. function groupFields(fields: MemoryProviderField[]): [string, MemoryProviderField[]][] { const groups: [string, MemoryProviderField[]][] = [] for (const field of fields) { @@ -56,8 +55,7 @@ export function ProviderConfigModal({ const [seeded, setSeeded] = useState>({}) const [saving, setSaving] = useState(false) - // Reseed from the latest config each time the dialog opens so edits never - // start from a stale snapshot left over from a prior session. + // Reseed on open so edits never start from a stale prior-session snapshot. useEffect(() => { if (open) { const seed = seedAll(config) @@ -67,8 +65,7 @@ export function ProviderConfigModal({ }, [open, config]) const save = async () => { - // Unstored fields render their schema default; persisting untouched keys - // would pin values that runtime defaults (e.g. migration guards) still own. + // Untouched keys stay unsubmitted; runtime defaults still own their values. const edited = Object.fromEntries(Object.entries(values).filter(([key, value]) => value !== seeded[key])) setSaving(true) diff --git a/apps/desktop/src/app/settings/memory/provider-config-panel.tsx b/apps/desktop/src/app/settings/memory/provider-config-panel.tsx index 681325284b79..db8b49b227b6 100644 --- a/apps/desktop/src/app/settings/memory/provider-config-panel.tsx +++ b/apps/desktop/src/app/settings/memory/provider-config-panel.tsx @@ -11,9 +11,7 @@ import { FieldControl } from './field-control' import { ListRow, LoadingState, Pill } from '../primitives' import { ProviderConfigModal } from './provider-config-modal' -/** Seed editable values from the inline fields only, so saving the compact - * panel never re-writes fields owned by the full-config editor. Secret fields - * start blank (their value is never returned). */ +// Inline fields only: the compact panel must never re-write modal-owned keys. function seedValues(config: MemoryProviderConfig): Record { return Object.fromEntries( config.fields.filter(field => field.inline).map(field => [field.key, field.kind === 'secret' ? '' : field.value]) diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 7a37e700bb0a..8df2ff57bbeb 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -3875,10 +3875,7 @@ def _normalize_config_for_web(config: Dict[str, Any]) -> Dict[str, Any]: return config -# ── Memory provider config ────────────────────────────────────────────────── -# One generic GET/PUT pair drives every declared provider. Reads/writes dispatch -# on ``provider.storage`` so a new simple provider is pure declaration, while -# Honcho persists to its real host-scoped config without bespoke endpoints. +# ── Memory provider config: one generic GET/PUT pair, dispatching on storage ── def _provider_field_entry(field: ProviderField) -> Dict[str, Any]: @@ -3899,8 +3896,7 @@ def _provider_field_entry(field: ProviderField) -> Dict[str, Any]: } -# Sentinel: a coerced value of _UNSET means "remove this key" (blank text / -# number / json falls back to the host or built-in default). +# Sentinel: remove this key so it falls back to the host or built-in default. _UNSET: Any = object() @@ -4075,8 +4071,7 @@ def _memory_provider_payload(provider: ProviderConfigSchema) -> Dict[str, Any]: native = _read_field(field, sources, env) if is_honcho and not field.placeholder and field.key in {"workspace", "aiPeer"}: - # Surface the resolved host so the user sees the peer mapping - # Honcho will actually use when these fields are left blank. + # Blank fields surface the resolved host Honcho will actually use. entry["placeholder"] = host value = _serialize_field_value(field, native) @@ -4142,14 +4137,10 @@ def _write_provider_honcho(provider: ProviderConfigSchema, values: Dict[str, str resolve_active_host, resolve_config_path, host_block_of = _honcho_resolvers() host = resolve_active_host() - # Write the same file reads resolve — a hardcoded path would shadow a - # config living at one of resolve_config_path's fallbacks with a sparse - # copy holding only the submitted keys. + # Write the file reads resolve, or a save shadows it with a sparse copy. path = resolve_config_path() - # OAuth refresh rotates single-use tokens in this file; an unlocked - # read-modify-write racing it would persist a stale refresh token and - # revoke the grant, so serialize on the same advisory lock. + # OAuth rotation is single-use; an unlocked RMW here can revoke the grant. with _config_refresh_lock(path): cfg: Dict[str, Any] = {} if path.exists(): @@ -4161,8 +4152,7 @@ def _write_provider_honcho(provider: ProviderConfigSchema, values: Dict[str, str hosts = cfg.get("hosts") cfg["hosts"] = hosts = hosts if isinstance(hosts, dict) else {} - # Target the block reads resolve — including a legacy dot-form key — - # so a save updates the live block instead of shadowing it. + # Update the block reads resolve (legacy dot-form included), never shadow it. existing = host_block_of(cfg, host) host_key = next((k for k, v in hosts.items() if v is existing), host) if existing else host host_block = hosts.setdefault(host_key, existing) @@ -4175,9 +4165,7 @@ def _write_provider_honcho(provider: ProviderConfigSchema, values: Dict[str, str continue if field.env_key: save_env_value(field.env_key, submitted) - # The client reads the JSON-stored key before the env store, so - # persist where honcho setup does — but never overwrite an OAuth - # access token; the refresh loop owns that slot. + # Persist where the client reads first; an OAuth token owns that slot. stored = host_block.get(field.key) if not (isinstance(stored, str) and stored.startswith(ACCESS_TOKEN_PREFIX)): host_block[field.key] = submitted diff --git a/plugins/memory/config_schema.py b/plugins/memory/config_schema.py index 65c0260a8452..dffcb5b10380 100644 --- a/plugins/memory/config_schema.py +++ b/plugins/memory/config_schema.py @@ -35,9 +35,7 @@ KIND_BOOL = "bool" KIND_NUMBER = "number" KIND_JSON = "json" -# Storage backends understood by web_server. ``flat_json`` persists non-secret -# fields to ``//config.json``; ``honcho_host_block`` -# targets Honcho's real config (honcho.json, scoped to the active profile host). +# Storage backends understood by web_server (see its read/write dispatch). STORAGE_FLAT_JSON = "flat_json" STORAGE_HONCHO_HOST_BLOCK = "honcho_host_block" @@ -80,8 +78,7 @@ class ProviderField: env_fallbacks: tuple[str, ...] = () inline: bool = False group: str = "" - # Where a host-block backend stores the field: "host" (per-profile host - # block) or "root" (config root). Ignored by the flat-json backend. + # Host-block placement: "host" (per-profile) or "root"; flat-json ignores it. scope: str = "host" @property @@ -132,8 +129,7 @@ def get_provider_config_schema(name: str) -> ProviderConfigSchema | None: spec.loader.exec_module(module) schema = getattr(module, "CONFIG_SCHEMA", None) except Exception: - # A broken schema file must not cache: it would render as a silent - # empty panel until process restart even after the file is fixed. + # Never cache a failed load: it would pin an empty panel until restart. _log.exception("failed to load config schema for memory provider %r", name) return None diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index 451aae461897..862658c17e5e 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -581,8 +581,7 @@ class TestWebServerEndpoints: assert cfg["hosts"]["hermes"]["peerName"] == "eri" assert cfg["hosts"]["hermes"]["environment"] == "local" assert cfg["hosts"]["hermes"]["sessionStrategy"] == "per-repo" - # The key persists where the client actually reads it (the host block - # outranks the env store) — GET keeps it write-only regardless. + # The key lands where the client reads first; GET keeps it write-only. assert cfg["hosts"]["hermes"]["apiKey"] == "hch-test-key" def test_put_honcho_blank_text_clears_key(self, monkeypatch, tmp_path): @@ -703,9 +702,7 @@ class TestWebServerEndpoints: assert json.loads(fields["userPeerAliases"]["value"]) == {"telegram_1": "eri"} def test_put_honcho_first_save_merges_into_resolved_config(self, monkeypatch, tmp_path): - # No profile-local honcho.json: reads and writes both resolve to the - # global config, so a save merges instead of shadowing it with a - # sparse profile-local copy. + # With no profile-local file, a save merges into the resolved global config. monkeypatch.setenv("HOME", str(tmp_path)) from hermes_constants import get_hermes_home @@ -728,8 +725,7 @@ class TestWebServerEndpoints: assert cfg["hosts"]["hermes"] == {"workspace": "kept", "peerName": "eri"} def test_put_honcho_updates_legacy_dot_form_host_block(self, monkeypatch, tmp_path): - # A legacy 'hermes.' block that reads resolve must be updated - # in place, not shadowed by a fresh underscore-form block. + # The legacy dot-form block reads resolve is updated in place, not shadowed. monkeypatch.setenv("HOME", str(tmp_path)) monkeypatch.setenv("HERMES_HONCHO_HOST", "hermes_work")