Skip to content

fix(agent): persist complete model runtime state on /model switch - #25131

Closed
zccyman wants to merge 1 commit into
NousResearch:mainfrom
atyou2happy:fix/model-switch-persist-complete
Closed

zccyman wants to merge 1 commit into
NousResearch:mainfrom
atyou2happy:fix/model-switch-persist-complete

Conversation

@zccyman

@zccyman zccyman commented May 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Both gateway /model and CLI /model --global now persist the complete runtime tuple to config.yaml:

model.default
model.provider
model.base_url      # NEW: was missing or conditionally written
model.api_mode      # NEW: was never persisted

Before

Gateway /model (gateway/run.py):

model_cfg["default"] = result.new_model
model_cfg["provider"] = result.target_provider
if result.base_url:                    # ← only writes when truthy
    model_cfg["base_url"] = result.base_url
# api_mode: never persisted

CLI /model --global (cli.py, 2 locations):

save_config_value("model.default", result.new_model)
if result.provider_changed:
    save_config_value("model.provider", result.target_provider)
# base_url: never persisted
# api_mode: never persisted

After

Gateway — unconditional with empty-string clearing:

model_cfg["base_url"] = result.base_url or ""
model_cfg["api_mode"] = result.api_mode or ""

CLI — two additional save_config_value calls:

save_config_value("model.base_url", result.base_url or "")
save_config_value("model.api_mode", result.api_mode or "")

Impact

Fixes the class of bugs where switching models leaves stale endpoint/protocol state:

Scenario Before After
Switch from custom → built-in provider Old base_url survives Cleared to ""
Switch from anthropic_messages → openai Old api_mode survives Cleared to ""
Switch to a provider with explicit base_url Not persisted (CLI) Persisted correctly
Restart gateway after /model switch May use stale endpoint Uses correct state

Testing

  • 5 new tests covering all persistence paths
  • 29 existing model-switch tests passing (0 regressions)

Files changed

File Change
gateway/run.py Persist base_url + api_mode unconditionally
cli.py Add save_config_value for base_url + api_mode (2 locations)
tests/gateway/test_model_switch_config_persistence.py New: 5 tests

Closes #25107
Closes #25106
See also #25105 (kimi-for-coding alias normalization — separate issue)

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles P2 Medium — degraded but workaround exists labels May 13, 2026
Both gateway /model and CLI /model --global now persist the full
runtime tuple (model, provider, base_url, api_mode) to config.yaml.

Previously:
- Gateway: only persisted base_url when truthy (stale values survived)
- Gateway: api_mode was never persisted
- CLI: neither base_url nor api_mode was persisted via --global

This caused stale endpoint/protocol state to survive model switches,
leading to wrong routing after restart or new sessions.

Fix: unconditionally write all four fields, using empty-string to clear
stale values:
  model_cfg["base_url"] = result.base_url or ""
  model_cfg["api_mode"] = result.api_mode or ""

Closes NousResearch#25107
Closes NousResearch#25106
See also NousResearch#25105 (kimi-for-coding alias — separate normalization issue)
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for identifying the remaining CLI persistence gap. The CLI premise is still present on current main: cli.py:7945-7948 and cli.py:8257-8260 persist only model.default and conditionally model.provider.

Problems

  • The gateway hunk targets a path that has since moved. Live picker and typed persistence are now in gateway/slash_commands.py:1666-1673 and gateway/slash_commands.py:1916-1923; the PR is currently conflicting.
  • The proposed unconditional gateway writes conflict with the later endpoint-clearing contract. Both live paths call clear_model_endpoint_credentials(..., clear_base_url=True), which removes api_mode and base_url (hermes_cli/config.py:4566-4569), added by f6deabca0d8010d485d4d3a8744d0ceb797eacb9.
  • tests/gateway/test_model_switch_config_persistence.py simulates the intended assignments instead of exercising a production handler, so it cannot regress the actual CLI or gateway paths.

Suggested changes

  • Salvage the CLI fix at both live branches, and port gateway behavior to both current gateway/slash_commands.py sites while preserving non-custom clearing semantics.
  • Replace simulation tests with temp-HERMES_HOME tests driving both CLI paths and the gateway typed/picker paths; tests/gateway/test_model_picker_persist.py:15-18 is the existing callback-driven pattern.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for identifying this persistence-gap class early — you were 2 months ahead of the fixes that eventually landed. Closing as superseded on current main, in three pieces:

Nothing from this branch remains unlanded, and the gateway hunk targets a path that has since moved (per the earlier sweeper review). Appreciate the report and the diagnosis — the bug class was real and is now fully covered with regression tests on all paths.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

3 participants