fix(tools): prefer load_config() over stale CLI_CONFIG in delegate _load_config()#21405
Conversation
…oad_config() Fixes NousResearch#18946: hermes config set delegation.* silently has no effect on running process because _load_config() checks CLI_CONFIG first (stale in-memory dict) before load_config() (mtime-cached disk reads). Reverse priority so disk config with mtime invalidation wins.
Edge case: priority reversal can still trap on empty-string valuesThanks for the fix — inverting the priority to prefer However, the same truthy empty-string trap can still occur with this approach. If a user has: delegation:
model: ""
provider: ""
A merge-based approach avoids this entirely: def _load_config() -> dict:
result = {}
try:
from cli import CLI_CONFIG
result = dict(CLI_CONFIG.get("delegation") or {})
except Exception:
pass
try:
from hermes_cli.config import load_config
full = load_config()
persisted = full.get("delegation") or {}
for key, value in persisted.items():
if value or isinstance(value, (bool, int)):
result[key] = value
except Exception:
pass
return resultCLI_CONFIG provides defaults, persistent config overrides when non-empty. No truthy/empty-string ambiguity — This is the approach I proposed in #32671. Would you be open to adopting the merge strategy, or shall I submit a separate PR with it? |
Problem
hermes config set delegation.*writes to disk and reports success, but_load_config()indelegate_tool.pychecks the in-memoryCLI_CONFIGdict first. SinceCLI_CONFIGwas loaded once at startup and never invalidated, the stale cached value wins — the new disk config is never read until process restart.This affects CLI, gateway, and cron — any long-running process calling
delegate_task()after a config change viahermes config set.Root Cause
_load_config()prioritizedCLI_CONFIG(stale in-memory dict) overload_config()(mtime-cached disk reads with automatic invalidation). Theload_config()function inhermes_cli/config.pyalready handles cache invalidation correctly by checking(mtime_ns, size)— it just was never reached becauseCLI_CONFIGwas checked first.Fix
Reverse priority so
load_config()(mtime-aware, always fresh) is tried beforeCLI_CONFIG(stale startup snapshot). Addedif cfg:guard to the fallback path for consistency.Testing
delegation.model: google/gemini-2.5-flashhermes config set delegation.model google/gemini-3-flash-previewdelegate_task(...)from the original sessiongemini-2.5-flash(stale cache)gemini-3-flash-preview(fresh from disk)Notes
This replaces PR #18978 which was 692+ commits behind main and showed DIRTY merge state. Clean rebase onto current main.
Fixes #18946