diff --git a/cli.py b/cli.py index c2c3ffcbc828b..ad793c5dfb651 100644 --- a/cli.py +++ b/cli.py @@ -7781,28 +7781,24 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin): ) or None except Exception: provider = None + # Both shapes use the same or-None discipline so stale keys from a + # previous switch are deleted (not merely omitted) in BOTH the + # nested gateway_runtime dict (CLI reader) and the top-level keys + # (TUI gateway reader). _merge_model_config_json only deletes on + # explicit None, so falsy values must be converted, not filtered. + # Deriving the top-level from **route guarantees the two shapes + # can never diverge — the asymmetry that caused the original + # stale-key bug (#85261 simplify-code review). route = { - k: v - for k, v in { - "provider": provider, - "base_url": result.base_url, - "api_mode": result.api_mode, - }.items() - if v + "provider": provider or None, + "base_url": result.base_url or None, + "api_mode": result.api_mode or None, } 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. 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, + **route, }) except Exception: logger.debug( diff --git a/hermes_state.py b/hermes_state.py index 4fe015d4f3a4d..0188e5d59bd12 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -6132,7 +6132,10 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin) return {} runtime = raw.get("gateway_runtime") if isinstance(runtime, dict) and runtime.get("provider"): - return dict(runtime) + # Filter None values: the persist path writes or-None to trigger + # deletion in the top-level merge, but gateway_runtime is replaced + # as a whole dict (not deep-merged), so None values survive here. + return {k: v for k, v in runtime.items() if v is not None} top_level = { key: raw.get(key) for key in ("provider", "base_url", "api_mode") diff --git a/tests/cli/test_resume_model_restore.py b/tests/cli/test_resume_model_restore.py index 33edbad8a1848..766490c54d88e 100644 --- a/tests/cli/test_resume_model_restore.py +++ b/tests/cli/test_resume_model_restore.py @@ -147,9 +147,10 @@ def test_persist_model_switch_writes_model_and_both_route_shapes(): # ...and top-level for the TUI gateway's _stored_session_runtime_overrides. 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). + # Both shapes use or-None so stale keys are deleted (not merely omitted) + # in BOTH gateway_runtime and top-level — the asymmetry that caused the + # original stale-key bug. + assert patch["gateway_runtime"]["api_mode"] is None assert patch["api_mode"] is None @@ -183,8 +184,16 @@ def test_persist_model_switch_clears_stale_route_keys(tmp_path, monkeypatch): meta = db.get_session("stale1") config = json.loads(meta["model_config"]) + # Top-level keys: stale values deleted. assert config["provider"] == "openrouter" assert "api_mode" not in config, config # stale anthropic_messages deleted + # Nested gateway_runtime: stale values replaced with None (the merge + # replaces the entire gateway_runtime dict, not deep-merging its keys). + # The reader's `or None` / `if v` filtering treats None the same as + # absent, so stale values are effectively erased. + gw = config.get("gateway_runtime", {}) + assert gw.get("provider") == "openrouter" + assert gw.get("api_mode") is None # stale anthropic_messages erased runtime = SessionDB.session_gateway_runtime(meta) assert runtime["provider"] == "openrouter" assert "api_mode" not in runtime @@ -235,7 +244,7 @@ def test_persist_model_switch_heals_bare_custom(monkeypatch): written.clear() stub._persist_model_switch_to_session(_BareResult()) assert written["patch"]["provider"] is None - assert "provider" not in written["patch"]["gateway_runtime"] + assert written["patch"]["gateway_runtime"]["provider"] is None def test_restore_session_model_heals_bare_custom_stored_rows(monkeypatch):