fix: delete stale top-level route keys on /model persist
patch_session_model_config merges key-level and only deletes on explicit None. Dropping falsy values from the top-level patch let a previous switch's api_mode/base_url survive the next switch — TUI/desktop resume then restored e.g. openrouter with anthropic_messages wire mode, and a failed bare-custom heal produced a stale-provider/new-endpoint route. Write absent top-level values as explicit None so each switch fully replaces the persisted route. Regression test against a real SessionDB; mutation-checked. Also correct the heal comment (CLI is deliberately stricter than the TUI recovery, which keeps bare custom with a base_url).
This commit is contained in:
parent
dbe24dfc12
commit
d002167390
18
cli.py
18
cli.py
|
|
@ -7734,8 +7734,17 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin):
|
|||
try:
|
||||
db.update_session_model(sid, result.new_model)
|
||||
# Both shapes: nested for the CLI reader, top-level for the
|
||||
# TUI gateway's resume path.
|
||||
db.patch_session_model_config(sid, {"gateway_runtime": route, **route})
|
||||
# TUI gateway's resume path. Top-level keys are written as
|
||||
# explicit None when absent — _merge_model_config_json only
|
||||
# deletes on None, so omitting them would let a PREVIOUS
|
||||
# switch's provider/api_mode survive this one (stale wire
|
||||
# protocol / frankenroute on resume).
|
||||
db.patch_session_model_config(sid, {
|
||||
"gateway_runtime": route,
|
||||
"provider": provider or None,
|
||||
"base_url": result.base_url or None,
|
||||
"api_mode": result.api_mode or None,
|
||||
})
|
||||
except Exception:
|
||||
logger.debug(
|
||||
"Failed to persist model switch to session DB", exc_info=True
|
||||
|
|
@ -7782,8 +7791,9 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin):
|
|||
# Heal bare "custom" persisted by older builds / gateway turns: it's
|
||||
# the resolved billing class, not a routable identity. Recover the
|
||||
# durable custom:<name> menu key from the endpoint, else drop the
|
||||
# provider so resume keeps the ambient default (matches the TUI
|
||||
# gateway's _stored_session_runtime_overrides recovery).
|
||||
# provider so resume keeps the ambient default. (Stricter than the
|
||||
# TUI gateway's recovery, which keeps bare "custom" when a base_url
|
||||
# exists — the CLI's resolve path would hard-fail on it, #14676.)
|
||||
if str(stored_provider or "").strip().lower() == "custom":
|
||||
try:
|
||||
from hermes_cli.runtime_provider import canonical_custom_identity
|
||||
|
|
|
|||
|
|
@ -148,6 +148,46 @@ def test_persist_model_switch_writes_model_and_both_route_shapes():
|
|||
assert patch["provider"] == "custom:opencode-zen"
|
||||
assert patch["base_url"] == "https://oz/v1"
|
||||
assert "api_mode" not in patch["gateway_runtime"] # empty values dropped
|
||||
# Absent top-level values are explicit None so the merge DELETES stale
|
||||
# keys from a previous switch (merge only deletes on None).
|
||||
assert patch["api_mode"] is None
|
||||
|
||||
|
||||
def test_persist_model_switch_clears_stale_route_keys(tmp_path, monkeypatch):
|
||||
"""A later switch must not inherit the previous switch's api_mode/base_url.
|
||||
|
||||
patch_session_model_config merges key-level and only deletes on explicit
|
||||
None — dropping falsy values from the patch left the FIRST switch's
|
||||
api_mode (e.g. anthropic_messages) alive under the SECOND switch's
|
||||
provider, corrupting the wire protocol on TUI/desktop resume.
|
||||
"""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes"))
|
||||
db = SessionDB(db_path=tmp_path / "state.db")
|
||||
db.create_session(session_id="stale1", source="cli", model="m0")
|
||||
stub = _make_stub(_session_db=db, session_id="stale1")
|
||||
|
||||
class _First:
|
||||
new_model = "claude-x"
|
||||
target_provider = "custom:feather"
|
||||
base_url = "https://feather/v1"
|
||||
api_mode = "anthropic_messages"
|
||||
|
||||
class _Second:
|
||||
new_model = "gpt-5.4"
|
||||
target_provider = "openrouter"
|
||||
base_url = "https://openrouter.ai/api/v1"
|
||||
api_mode = "" # openrouter default — must ERASE the anthropic mode
|
||||
|
||||
stub._persist_model_switch_to_session(_First())
|
||||
stub._persist_model_switch_to_session(_Second())
|
||||
|
||||
meta = db.get_session("stale1")
|
||||
config = json.loads(meta["model_config"])
|
||||
assert config["provider"] == "openrouter"
|
||||
assert "api_mode" not in config, config # stale anthropic_messages deleted
|
||||
runtime = SessionDB.session_gateway_runtime(meta)
|
||||
assert runtime["provider"] == "openrouter"
|
||||
assert "api_mode" not in runtime
|
||||
|
||||
|
||||
def test_persist_model_switch_noop_without_db_or_session():
|
||||
|
|
@ -188,12 +228,13 @@ def test_persist_model_switch_heals_bare_custom(monkeypatch):
|
|||
stub._persist_model_switch_to_session(_BareResult())
|
||||
assert written["patch"]["provider"] == "custom:myendpoint"
|
||||
|
||||
# Healing fails -> provider dropped entirely, not persisted bare.
|
||||
# Healing fails -> provider dropped (explicit None deletes any stale
|
||||
# persisted provider), never persisted bare.
|
||||
monkeypatch.setattr(rp, "canonical_custom_identity",
|
||||
lambda base_url=None, model=None: None)
|
||||
written.clear()
|
||||
stub._persist_model_switch_to_session(_BareResult())
|
||||
assert "provider" not in written["patch"]
|
||||
assert written["patch"]["provider"] is None
|
||||
assert "provider" not in written["patch"]["gateway_runtime"]
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue