feat(gateway): add gateway.persist_model_override flag (opt-out for sticky session /model overrides) - #68861
feat(gateway): add gateway.persist_model_override flag (opt-out for sticky session /model overrides)#68861hilmanraz wants to merge 1 commit into
Conversation
Adds an opt-out for per-session /model override persistence, addressing the silent desync reported in NousResearch#68826 where an in-chat /model switch (even once, even weeks prior) is persisted per-session and rehydrated on every gateway restart, permanently outranking config.yaml's model.default/model.provider until manually cleared. Behavior: - gateway.persist_model_override: true (default) — existing behavior unchanged: /model writes {model, provider, base_url} to the session store and the override is rehydrated after a restart. - gateway.persist_model_override: false — /model still switches the model for the current process lifetime (in-memory map is updated by the slash-command caller), but the switch is never written to disk and never rehydrated. Sessions always fall back to model.default / model.provider on the next gateway boot. Clearing (set_model_override None) still works with the flag off, so an operator can wipe a previously-persisted override after flipping the flag. Touches: - gateway/config.py: new persist_model_override field on GatewayConfig, threaded through to_dict / from_dict / top-level-vs-nested precedence (mirrors write_sessions_json). - gateway/session.py: SessionStore caches _persist_model_override; set_model_override skips non-clearing writes when the flag is off. - gateway/run.py: _rehydrate_session_model_override early-returns when the store has persistence disabled, so a pre-existing persisted override (from before the flag was flipped) does not resurrect. - hermes_cli/config.py: DEFAULT_CONFIG['gateway']['persist_model_override'] so the flag shows up in generated config.yaml. - tests/gateway/test_session_model_override_persist_flag.py: 7 new tests covering flag-off no-persist, restart survival, clear-after-flip, runner rehydrate skip, default-on sanity, live in-memory override still applies, and config round-trip. Closes NousResearch#68826
|
Thanks for the triage note and for flagging #67113. They're complementary, not competing — different layers and different use cases: #67113 ( This PR ( The two solve the same underlying desync symptom (#68826) but for different operators:
Happy to adjust if the maintainers prefer only one of the two to ship. A couple of notes on the design choice in case it helps the decision:
|
teknium1
left a comment
There was a problem hiding this comment.
Thanks for separating the operator-wide policy from the merged /model --once path. The stale-session premise is still present on current main: gateway/run.py:4478 rehydrates before resolution, and gateway/session.py:2523 persists the override.
Problems
gateway/session.py:2185returns beforesanitize_model_override(). Existing method semantics say a dict with no persistable values clears the override (gateway/session.py:2526-2532on current main); with the new flag,{}instead leaves a prior persisted override intact. Normalize first and gate only a non-Nonecleaned value.- The default-config hunk is stale: current main moved
DEFAULT_CONFIGtohermes_cli/config_defaults.pyin1fe06115d1. GitHub reports this PR asdirty, so a salvage must relocate that hunk rather than apply it tohermes_cli/config.py. - The added round-trip test does not exercise
load_gateway_config()YAML loading and its top-level-vs-gateway.*precedence convention (tests/gateway/test_config.py:1059-1076).
Suggested changes
- Add loader-level tests for
gateway.persist_model_override: falseand precedence, plus the empty-dict clearing case.
Automated hermes-sweeper review.
| entry = self._entries.get(session_key) | ||
| if entry is None: | ||
| return | ||
| if override is not None and not self._persist_model_override: |
There was a problem hiding this comment.
This gates before sanitize_model_override(), so {} cannot clear a previously persisted override when the flag is off, despite the existing method contract accepting a dict with no persistable values as a clear. Compute cleaned first and skip only when cleaned is not None.
SummaryOne open PR addresses #68826. #68861 adds an operator-wide configuration flag that gates both persistence and restart rehydration of session-scoped /model overrides, directly targeting the reported precedence of stale session state over config.yaml defaults. Related pull requests
Suggested consolidationAuthor action: rebase #68861 onto main, or split out the part that can merge. Preserve the operator-wide persistence policy as the salvage path, but first normalize before gating so {} still clears persisted state, relocate the default-config change, add load_gateway_config() tests for top-level-versus-gateway.* precedence, and obtain the documented maintainer precedence decision; keep the PR open while those contributor review findings remain unresolved. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I68826(["issue #68826 (open)"])
P68861["PR #68861 (open)"]
P68861 -->|best fix| I68826
class I68826 open
class P68861 open
class P68861 best
class P68861 target
click I68826 "https://github.com/NousResearch/hermes-agent/issues/68826"
click P68861 "https://github.com/NousResearch/hermes-agent/pull/68861"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 16 kB of PR diffs, 10 kB of issue/PR text, 4 kB of discussion (4 comments), 3 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
Summary
Adds an opt-out for the per-session
/modeloverride persistence that silently desyncs config.yaml-managed models from active gateway sessions (#68826).When a user runs
/model <name>inside a gateway chat (Telegram/Discord/etc — even once, even weeks prior), Hermes persists{model, provider, base_url}to the session store viaSessionStore.set_model_override(). That override is rehydrated on every gateway restart (_rehydrate_session_model_overrideingateway/run.py) and always outranksconfig.yaml'smodel.default/model.provider. There was previously no way to opt out — operators who manage models exclusively viahermes model(SSH) / config.yaml edits never get their config change to take effect on any session that has ever used/modelin-chat, with no user-facing signal that the session is pinned away from config.This PR adds
gateway.persist_model_override(defaulttrue, preserves existing behavior). When set tofalse:/modelstill switches the model for the current process lifetime (the in-memory_session_model_overridesmap is updated by the slash-command caller — the feature only gates the write-through to the session store and the rehydrate path).model.default/model.providerfrom config.yaml on the next gateway boot.set_model_override(None)) still works with the flag off, so an operator can wipe a previously-persisted override after flipping the flag without needing a Python one-liner.Motivation
#68826 reported the symptom; this is the concrete fix. Users who switch models 2–3× a day via SSH (
hermes config set model.default …+ gateway restart) and occasionally use/modelin the bot get permanently pinned to the stale in-chat override, with the only diagnostic being a grep for"Rehydrated persisted /model override"in~/.hermes/logs/gateway.log. After this change, settinggateway.persist_model_override: falsemakes config.yaml the permanent source of truth — no more silent pinning.Changes
gateway/config.py: newpersist_model_overridefield onGatewayConfig, threaded throughto_dict/from_dict/ the top-level-vs-nestedgateway.*precedence block (mirrors the existingwrite_sessions_jsonpattern).gateway/session.py:SessionStorecaches_persist_model_overridefrom the config;set_model_overrideskips non-clearing writes when the flag is off. Clearing (None) is always honored so a pre-existing persisted override can still be wiped.gateway/run.py:_rehydrate_session_model_overrideearly-returns when the store has persistence disabled, so a pre-existing persisted override (from before the flag was flipped) does not resurrect on restart.hermes_cli/config.py:DEFAULT_CONFIG['gateway']['persist_model_override'] = Trueso the flag shows up in generatedconfig.yaml.tests/gateway/test_session_model_override_persist_flag.py: 7 new tests — flag-off no-persist, restart survival, clear-after-flip wipes prior persisted override, runner rehydrate skip, default-on sanity (existing behavior unchanged), live in-memory override still applies during process lifetime, and config round-trip. All pass locally.Backward compatibility
Default is
true→ existing behavior is unchanged. Existing persisted overrides remain valid and continue to rehydrate. Operators who want the new contract opt in by settinggateway.persist_model_override: false.Testing
Closes #68826.
Checklist
write_sessions_jsonconfig-field pattern (field + to_dict + from_dict + top-level-vs-nested precedence)hermes config set gateway.persist_model_override falseworks out of the box once this lands