From 1d275dd6dfb0fc326699064ae5dfc0a904c2375e Mon Sep 17 00:00:00 2001 From: briandevans <252620095+briandevans@users.noreply.github.com> Date: Fri, 31 Jul 2026 04:37:51 -0700 Subject: [PATCH 1/3] fix(tui_gateway): reconcile every build-relevant override with the deferred agent build _start_agent_build snapshots model_override, create_reasoning_override and create_service_tier_override into the build kwargs, then blocks for seconds inside _make_agent (MCP discovery, prompt/skill build) before installing the agent. Every writer of those three keys gates its live-apply on session["agent"] being set, so a pick made inside that window reaches neither the kwargs (already read) nor the agent (not yet installed). The user's choice is silently lost for the life of the session. The racy writers are config.set reasoning, config.set fast, _apply_model_switch, the pre-agent /moa branch, and the explicit-provider config.set model path -- which skips the initialization wait entirely, so it is the easiest to hit. Snapshot the three keys immediately before _make_agent and reconcile them once the agent is installed and wired, applying whatever changed through the same helpers the live paths use. Model is reconciled first: the switch rebuilds the agent's client and fast-mode overrides must resolve against the new model. A failed reconcile restores the previously built override instead of retaining the failed target. This matches the live path, which raises before committing model_override (#50163); a retained failure would otherwise be resurrected by the next /new or resume and rebuild the session onto a model that does not work. --- tests/test_tui_gateway_server.py | 295 +++++++++++++++++++++++++++++++ tui_gateway/methods_tools.py | 9 +- tui_gateway/server.py | 100 +++++++++++ 3 files changed, 401 insertions(+), 3 deletions(-) diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 2df2b124e37a4..029d61a5e0969 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -15022,6 +15022,301 @@ def fake_make_agent(sid, key, session_id=None, session_db=None, **kwargs): server._sessions.clear() +# --------------------------------------------------------------------------- +# Deferred agent build <-> override race. +# +# _start_agent_build snapshots model_override / create_reasoning_override / +# create_service_tier_override into the build kwargs and then blocks for +# seconds inside _make_agent (MCP discovery, prompt/skill build). Every writer +# of those keys gates its live-apply on session["agent"] being set, so a pick +# made inside that window reached neither the kwargs (already read) nor the +# agent (not yet installed) and was silently lost for the life of the session. +# --------------------------------------------------------------------------- + + +class _BuildRaceAgent: + """Stand-in for the agent _make_agent returns, recording in-place switches.""" + + def __init__(self): + self.model = "old/model" + self.provider = "openrouter" + self.base_url = "" + self.api_key = "sk-or" + self.api_mode = "chat_completions" + self.reasoning_config = None + self.service_tier = None + self.request_overrides: dict = {} + self.tools: list = [] + self.switch_calls: list = [] + + def switch_model(self, **kwargs): + self.switch_calls.append(kwargs) + self.model = kwargs.get("new_model", self.model) + self.provider = kwargs.get("new_provider", self.provider) + + +def _park_agent_build(monkeypatch, **session_extra): + """Start a deferred agent build parked inside _make_agent. + + Returns ``(sid, session, agent, release)``. Until ``release`` is set the + build thread sits inside _make_agent with ``session["agent"]`` still None — + the exact window every override writer used to mishandle. + """ + agent = _BuildRaceAgent() + entered = threading.Event() + release = threading.Event() + + def _slow_make_agent(_sid, _key, **_kwargs): + entered.set() + release.wait(timeout=5.0) + return agent + + monkeypatch.setattr(server, "_make_agent", _slow_make_agent) + monkeypatch.setattr(server, "_wire_callbacks", lambda _sid: None) + monkeypatch.setattr(server, "_emit", lambda *a, **kw: None) + monkeypatch.setattr(server, "_session_info", lambda _a, *a2: {"model": "x"}) + monkeypatch.setattr(server, "_probe_config_health", lambda _cfg: None) + monkeypatch.setattr(server, "_config_model_target", lambda: ("old/model", "")) + monkeypatch.setattr(server, "_start_notification_poller", lambda _sid, _s: None) + monkeypatch.setattr(server, "_notify_session_boundary", lambda *a, **kw: None) + monkeypatch.setattr(server, "_schedule_mcp_late_refresh", lambda *a, **kw: None) + monkeypatch.setattr(server, "_persist_live_session_runtime", lambda _s: None) + monkeypatch.setattr(server, "_persist_live_session_system_prompt", lambda _s: None) + monkeypatch.setattr(server, "_restart_slash_worker", lambda _sid, _s: None) + monkeypatch.setattr(server, "_append_model_switch_marker", lambda *a, **kw: None) + monkeypatch.setattr( + server, + "_get_db", + lambda: types.SimpleNamespace(create_session=lambda *a, **kw: None), + ) + + import tools.approval as _approval + + monkeypatch.setattr(_approval, "register_gateway_notify", lambda key, cb: None) + monkeypatch.setattr(_approval, "unregister_gateway_notify", lambda key: None) + monkeypatch.setattr(_approval, "load_permanent_allowlist", lambda: None) + + sid = "build-race-sid" + session = _session(agent_ready=threading.Event(), **session_extra) + session["agent"] = None + server._sessions[sid] = session + server._start_agent_build(sid, session) + assert entered.wait(timeout=2.0), "build thread never entered _make_agent" + assert session.get("agent") is None, "agent must not be installed yet" + return sid, session, agent, release + + +def _finish_build(session, release): + release.set() + assert session["agent_ready"].wait(timeout=5.0), "build never completed" + + +def _switch_result(model, provider, **extra): + return types.SimpleNamespace( + success=True, + new_model=model, + target_provider=provider, + api_key=extra.get("api_key", "sk-test"), + base_url=extra.get("base_url", "https://example.invalid"), + api_mode=extra.get("api_mode", "chat_completions"), + warning_message="", + model_info=None, + ) + + +def test_reasoning_pick_during_agent_build_reaches_installed_agent(monkeypatch): + """#63998's original shape: /reasoning during the build window was dropped. + + config.set reasoning writes create_reasoning_override and then gates the + live apply on session["agent"] — None mid-build — so the effort never + reached the agent the build was about to install. + """ + sid, session, agent, release = _park_agent_build(monkeypatch) + try: + resp = server.handle_request( + { + "id": "1", + "method": "config.set", + "params": {"session_id": sid, "key": "reasoning", "value": "high"}, + } + ) + assert resp.get("result"), f"got error: {resp.get('error')}" + assert session.get("agent") is None, "build must still be in flight" + + _finish_build(session, release) + + assert session["create_reasoning_override"] is not None + assert agent.reasoning_config == session["create_reasoning_override"], ( + "reasoning picked during the build window never reached the agent" + ) + finally: + release.set() + server._sessions.pop(sid, None) + + +def test_explicit_provider_model_switch_during_agent_build_reaches_agent(monkeypatch): + """config.set model with an explicit provider skips the initialization wait. + + That route calls _apply_model_switch with session["agent"] still None, so it + only records model_override; without reconciliation the built agent keeps + running the model the build snapshotted. + """ + result = _switch_result("claude-sonnet-4.6", "anthropic") + monkeypatch.setattr( + "hermes_cli.model_switch.switch_model", lambda **_kwargs: result + ) + + sid, session, agent, release = _park_agent_build(monkeypatch) + try: + resp = server.handle_request( + { + "id": "1", + "method": "config.set", + "params": { + "session_id": sid, + "key": "model", + "value": "claude-sonnet-4.6 --provider anthropic", + "confirm_expensive_model": True, + }, + } + ) + assert resp.get("result"), f"got error: {resp.get('error')}" + # The explicit-provider path returned without ever waiting for a build. + assert session.get("agent") is None, "build must still be in flight" + assert session["model_override"]["model"] == "claude-sonnet-4.6" + + _finish_build(session, release) + + assert agent.switch_calls, ( + "model picked during the build window never reached the agent" + ) + assert agent.model == "claude-sonnet-4.6" + assert agent.provider == "anthropic" + finally: + release.set() + server._sessions.pop(sid, None) + + +def test_moa_one_shot_during_agent_build_reaches_agent(monkeypatch): + """The pre-agent /moa branch writes model_override and returns. + + Its comment claimed "the override is consumed by the first build", which is + false once the build is already in flight — the MoA turn silently ran on the + old model. + """ + monkeypatch.setattr( + "hermes_cli.moa_config.normalize_moa_config", + lambda _cfg: {"default_preset": "balanced"}, + ) + result = _switch_result("balanced", "moa", base_url="moa://local") + monkeypatch.setattr( + "hermes_cli.model_switch.switch_model", lambda **_kwargs: result + ) + + sid, session, agent, release = _park_agent_build(monkeypatch) + try: + resp = server.handle_request( + { + "id": "1", + "method": "command.dispatch", + "params": {"session_id": sid, "name": "moa", "arg": "summarize this"}, + } + ) + assert resp.get("result"), f"got error: {resp.get('error')}" + assert session["model_override"]["provider"] == "moa" + + _finish_build(session, release) + + assert agent.switch_calls, ( + "/moa preset picked during the build window never reached the agent" + ) + assert agent.provider == "moa" + assert agent.model == "balanced" + finally: + release.set() + server._sessions.pop(sid, None) + + +def test_failed_reconcile_does_not_leave_failed_model_override_pinned(monkeypatch): + """A failed reconcile must not persist the model it failed to switch to. + + The live switch path raises *before* committing model_override (#50163). + The reconcile must match it: a retained failed target would be resurrected + by the next /new or resume and rebuild the session onto a broken model. + """ + result = _switch_result("broken/model", "anthropic") + monkeypatch.setattr( + "hermes_cli.model_switch.switch_model", lambda **_kwargs: result + ) + + sid, session, agent, release = _park_agent_build(monkeypatch) + try: + resp = server.handle_request( + { + "id": "1", + "method": "config.set", + "params": { + "session_id": sid, + "key": "model", + "value": "broken/model --provider anthropic", + "confirm_expensive_model": True, + }, + } + ) + assert resp.get("result"), f"got error: {resp.get('error')}" + assert session["model_override"]["model"] == "broken/model" + + # The in-place swap fails only once the build thread reconciles. + def _boom(**_kwargs): + raise RuntimeError("provider rejected the model") + + agent.switch_model = _boom + + _finish_build(session, release) + + assert session.get("model_override") is None, ( + "a failed reconcile left the failed model pinned on the session; " + "the next /new or resume would rebuild onto it" + ) + finally: + release.set() + server._sessions.pop(sid, None) + + +def test_fast_mode_pick_during_agent_build_reaches_installed_agent(monkeypatch): + """create_service_tier_override is the third key the build snapshots. + + config.set fast pins it, then gates the live apply on the agent existing — + the same race as reasoning. + """ + monkeypatch.setattr( + "hermes_cli.models.resolve_fast_mode_overrides", + lambda _model: {"service_tier": "priority"}, + ) + + sid, session, agent, release = _park_agent_build(monkeypatch) + try: + resp = server.handle_request( + { + "id": "1", + "method": "config.set", + "params": {"session_id": sid, "key": "fast", "value": "fast"}, + } + ) + assert resp.get("result"), f"got error: {resp.get('error')}" + assert session.get("agent") is None, "build must still be in flight" + assert session["create_service_tier_override"] == "priority" + + _finish_build(session, release) + + assert agent.service_tier == "priority", ( + "fast mode picked during the build window never reached the agent" + ) + finally: + release.set() + server._sessions.pop(sid, None) + + # ── billing/subscription state + error serialization ───────────────── diff --git a/tui_gateway/methods_tools.py b/tui_gateway/methods_tools.py index dc12130c6fd30..8efcbf11fdc85 100644 --- a/tui_gateway/methods_tools.py +++ b/tui_gateway/methods_tools.py @@ -635,9 +635,12 @@ def _(rid, params: dict) -> dict: session.pop("moa_one_shot_restore", None) return _err(rid, 5030, f"moa unavailable: {exc}") else: - # No agent built yet (lazy/fresh session): the override is - # consumed by the first build, so the turn runs MoA without an - # in-place switch. + # No agent built yet (lazy/fresh session): the first build + # consumes the override, so the turn runs MoA without an + # in-place switch. If a build is already in flight it has + # already snapshotted the previous value, so + # _reconcile_deferred_build_overrides applies this one to the + # agent once it is installed. session["model_override"] = { "provider": "moa", "model": preset, diff --git a/tui_gateway/server.py b/tui_gateway/server.py index c33cba50ea977..f9a5f530d9b8b 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -2126,6 +2126,89 @@ def _wait_agent_for_prompt(session: dict, rid: str, sid: str) -> dict | None: return _err(rid, 5032, err) if err else None +def _reconcile_deferred_build_overrides( + sid: str, session: dict, agent, before: dict +) -> None: + """Apply build-relevant overrides written while the agent was being built. + + ``_start_agent_build`` reads ``model_override`` / + ``create_reasoning_override`` / ``create_service_tier_override`` into the + build kwargs and then spends seconds inside ``_make_agent`` (MCP discovery, + prompt/skill build). Every writer of those keys gates its live-apply on + ``session["agent"]`` being set — ``config.set reasoning``, + ``config.set fast``, ``_apply_model_switch``, the pre-agent ``/moa`` branch, + and the explicit-provider ``config.set model`` path (which skips the + initialization wait entirely). A pick made inside that window therefore + reaches neither the kwargs (already snapshotted) nor the agent (not yet + installed), so it is silently lost for the life of the session. + + Re-read the three keys after installation and apply whatever changed + through the same helpers the live paths use. Model goes first: the switch + rebuilds the agent's client, and fast-mode overrides must resolve against + the newly selected model. + """ + override = session.get("model_override") + if isinstance(override, dict) and override != before.get("model"): + model = str(override.get("model") or "").strip() + provider = str(override.get("provider") or "").strip() + if model: + raw = f"{model} --provider {provider}" if provider else model + try: + _apply_model_switch( + sid, + session, + raw, + # The pick is already committed to the session, so its + # cost confirmation (if any) was answered by whoever wrote + # it. Re-prompting here would strand the switch: nothing is + # listening to this background build, so a confirm_required + # return would drop the user's model on the floor. + confirm_expensive_model=True, + pin_session_override=True, + # Reconciling a session-scoped pick — never write config.yaml. + persist_override=False, + ) + except Exception as exc: + # The live path raises *before* committing the override + # (#50163), so a failed reconcile must not leave the failed + # target pinned: a later /new or resume rebuilds from + # model_override and would resurrect a model that does not + # work. Put back exactly what the build baked in. + logger.warning( + "Deferred-build model override reconcile failed: %s", exc + ) + prior = before.get("model") + if prior is None: + session.pop("model_override", None) + else: + session["model_override"] = prior + + changed = False + + tier = session.get("create_service_tier_override") + if tier is not None and tier != before.get("tier"): + agent.service_tier = "priority" if tier == "priority" else None + request_overrides = dict(getattr(agent, "request_overrides", {}) or {}) + request_overrides.pop("service_tier", None) + request_overrides.pop("speed", None) + if tier == "priority": + from hermes_cli.models import resolve_fast_mode_overrides + + fast_overrides = resolve_fast_mode_overrides(getattr(agent, "model", None)) + if fast_overrides: + request_overrides.update(fast_overrides) + agent.request_overrides = request_overrides + changed = True + + reasoning = session.get("create_reasoning_override") + if reasoning is not None and reasoning != before.get("reasoning"): + agent.reasoning_config = reasoning + changed = True + + if changed: + _persist_live_session_runtime(session) + + def _start_agent_build(sid: str, session: dict) -> None: """Start building the real AIAgent for a TUI session, once. @@ -2229,6 +2312,17 @@ def _build() -> None: kw["reasoning_config_override"] = reasoning if (tier := current.get("create_service_tier_override")) is not None: kw["service_tier_override"] = tier + # Snapshot what the build is about to bake in. _make_agent + # blocks for seconds and every writer of these keys skips its + # live-apply while session["agent"] is None, so anything the + # user picks from here until installation must be reconciled + # afterwards or it is lost — see + # _reconcile_deferred_build_overrides. + pre_build_overrides = { + "model": current.get("model_override"), + "reasoning": current.get("create_reasoning_override"), + "tier": current.get("create_service_tier_override"), + } agent = _make_agent(sid, key, **kw) finally: _clear_session_context(tokens) @@ -2265,6 +2359,12 @@ def _build() -> None: pass _wire_callbacks(sid) + # The agent is installed and wired, so writers now take their live + # branch. Adopt anything picked during the build window before the + # session.info below publishes the session's runtime identity. + _reconcile_deferred_build_overrides( + sid, current, agent, pre_build_overrides + ) # Surface the self-improvement review's "💾 …" summary as an event # the TUI/desktop render in-transcript, honoring # display.memory_notifications. _init_session wires this for the From 655eae4376947a2dc99cda47d911a16166d6268c Mon Sep 17 00:00:00 2001 From: briandevans <252620095+briandevans@users.noreply.github.com> Date: Fri, 31 Jul 2026 04:49:31 -0700 Subject: [PATCH 2/3] fix(tui_gateway): reconcile a cleared reasoning override with the deferred build config.set reasoning with scope=global writes agent.reasoning_effort and pops create_reasoning_override, applying the new effort live only when an agent already exists. Both halves are skipped during the deferred build window, so the reconcile treated the removal as "nothing changed" and the freshly installed agent kept the effort the build had snapshotted. Treat an override removal as a reconciled transition and apply the effective global parsed setting after installation, with a barrier regression test through the real global-scope route. --- tests/test_tui_gateway_server.py | 48 ++++++++++++++++++++++++++++++++ tui_gateway/server.py | 23 +++++++++++++-- 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 029d61a5e0969..ea28877688cb0 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -15154,6 +15154,54 @@ def test_reasoning_pick_during_agent_build_reaches_installed_agent(monkeypatch): server._sessions.pop(sid, None) +def test_global_reasoning_clear_during_agent_build_reaches_installed_agent( + tmp_path, monkeypatch +): + """A *cleared* override is a reconciled transition too. + + config.set reasoning with scope=global writes agent.reasoning_effort and + pops create_reasoning_override, applying it live only when an agent already + exists. During the build window neither happens, so the installed agent + would otherwise keep the effort the build snapshotted. + """ + monkeypatch.setattr(server, "_hermes_home", tmp_path) + (tmp_path / "config.yaml").write_text( + "agent:\n reasoning_effort: medium\n", encoding="utf-8" + ) + + sid, session, agent, release = _park_agent_build( + monkeypatch, + create_reasoning_override={"enabled": True, "effort": "low"}, + ) + try: + resp = server.handle_request( + { + "id": "1", + "method": "config.set", + "params": { + "session_id": sid, + "key": "reasoning", + "value": "high", + "scope": "global", + }, + } + ) + assert resp.get("result"), f"got error: {resp.get('error')}" + # The session pin is gone and the agent does not exist yet. + assert "create_reasoning_override" not in session + assert session.get("agent") is None, "build must still be in flight" + + _finish_build(session, release) + + assert agent.reasoning_config == {"enabled": True, "effort": "high"}, ( + "a global reasoning change during the build window left the agent " + "on the stale snapshotted effort" + ) + finally: + release.set() + server._sessions.pop(sid, None) + + def test_explicit_provider_model_switch_during_agent_build_reaches_agent(monkeypatch): """config.set model with an explicit provider skips the initialization wait. diff --git a/tui_gateway/server.py b/tui_gateway/server.py index f9a5f530d9b8b..5ddd7c6823b9e 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -2201,9 +2201,26 @@ def _reconcile_deferred_build_overrides( changed = True reasoning = session.get("create_reasoning_override") - if reasoning is not None and reasoning != before.get("reasoning"): - agent.reasoning_config = reasoning - changed = True + before_reasoning = before.get("reasoning") + if reasoning is not None: + if reasoning != before_reasoning: + agent.reasoning_config = reasoning + changed = True + elif before_reasoning is not None: + # The session pin was *cleared* during the build: `config.set reasoning` + # with scope=global writes agent.reasoning_effort and pops the session + # key, then applies it live only when an agent already exists. A + # removal is just as much a reconciled transition as a write — without + # this the freshly installed agent keeps the stale snapshotted effort + # even though the user moved the setting globally. + from hermes_constants import parse_reasoning_effort + + cfg = _load_cfg() + agent_cfg = cfg.get("agent") if isinstance(cfg.get("agent"), dict) else {} + global_reasoning = parse_reasoning_effort(agent_cfg.get("reasoning_effort")) + if global_reasoning is not None: + agent.reasoning_config = global_reasoning + changed = True if changed: _persist_live_session_runtime(session) From a13480d142fae988bc6fafa94a1cd49acd2de690 Mon Sep 17 00:00:00 2001 From: briandevans <252620095+briandevans@users.noreply.github.com> Date: Sun, 2 Aug 2026 16:39:21 -0700 Subject: [PATCH 3/3] fix(tui_gateway): keep the deferred-build reconcile faithful to the resolver it mirrors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two divergences in _reconcile_deferred_build_overrides, both stemming from it comparing against the pre-build snapshot rather than the agent's live state. Fast tier: the tier branch set agent.service_tier unconditionally and only applied resolve_fast_mode_overrides when it returned something. The live config.set fast path does the opposite — it resolves first and returns 4002 ("fast mode is not available for this model") before pinning anything, so create_service_tier_override == "priority" has always implied the overrides resolved. The model switch this function reconciles first can land on a model without fast support, at which point the session advertised service_tier="priority" in session.info with no request overrides behind it. The reconcile cannot error (nothing is listening to a background build), so mirror the resolver's precedence instead: drop the pin and leave the tier unset. The docstring already said fast-mode overrides must resolve against the newly selected model; the code did not finish the thought. Model switch: the guard is `override != before.get("model")`, and before is the pre-build snapshot, not live state. A writer taking its live branch after session["agent"] is set but before this runs applies the switch itself and pins the same model_override read back here, leaving the guard true and re-running the switch — a second slash-worker restart, a second switch marker in the transcript, a redundant session.info. Adopt the state instead when the agent already runs the requested model and provider, the same baseline check _sync_agent_model_with_config makes. --- tests/test_tui_gateway_server.py | 131 +++++++++++++++++++++++++++++++ tui_gateway/server.py | 40 ++++++++-- 2 files changed, 164 insertions(+), 7 deletions(-) diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index ea28877688cb0..8763846bb0287 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -15365,6 +15365,137 @@ def test_fast_mode_pick_during_agent_build_reaches_installed_agent(monkeypatch): server._sessions.pop(sid, None) +def test_reconcile_drops_the_fast_pin_when_the_switched_model_cannot_honor_it( + monkeypatch, +): + """Reconciling both keys must not break the resolver's own invariant. + + ``config.set fast`` returns 4002 ("fast mode is not available for this + model") *before* pinning anything, so ``create_service_tier_override == + "priority"`` has always implied that ``resolve_fast_mode_overrides`` + resolved for the session's model. A model picked in the same build window + is reconciled first and can land on a model without fast support, at which + point setting the tier unconditionally leaves the session advertising + ``service_tier="priority"`` with no request overrides behind it — a state + the live path cannot produce. + """ + monkeypatch.setattr(server, "_resolve_model", lambda: "old/model") + monkeypatch.setattr( + "hermes_cli.models.resolve_fast_mode_overrides", + lambda model: None if model == "no-fast/model" else {"service_tier": "priority"}, + ) + monkeypatch.setattr( + "hermes_cli.model_switch.switch_model", + lambda **_kwargs: _switch_result("no-fast/model", "anthropic"), + ) + + sid, session, agent, release = _park_agent_build(monkeypatch) + try: + # Fast first: it validates against the model the session has *now*, so + # picking the unsupported model first would simply 4002 here. + resp = server.handle_request( + { + "id": "1", + "method": "config.set", + "params": {"session_id": sid, "key": "fast", "value": "fast"}, + } + ) + assert resp.get("result"), f"got error: {resp.get('error')}" + assert session["create_service_tier_override"] == "priority" + + resp = server.handle_request( + { + "id": "2", + "method": "config.set", + "params": { + "session_id": sid, + "key": "model", + "value": "no-fast/model --provider anthropic", + "confirm_expensive_model": True, + }, + } + ) + assert resp.get("result"), f"got error: {resp.get('error')}" + assert session.get("agent") is None, "build must still be in flight" + + _finish_build(session, release) + + assert agent.model == "no-fast/model", "the model reconcile must run first" + assert agent.service_tier is None, ( + "the session advertises priority service tier on a model whose fast " + "overrides do not resolve; config.set fast refuses to create that state" + ) + assert "service_tier" not in agent.request_overrides + assert "speed" not in agent.request_overrides + assert session.get("create_service_tier_override") is None, ( + "an unhonorable fast pin must not survive to the next rebuild" + ) + finally: + release.set() + server._sessions.pop(sid, None) + + +def test_reconcile_skips_a_model_switch_a_live_writer_already_applied(monkeypatch): + """``before`` is the pre-build snapshot, not the agent's live state. + + A writer that takes its live branch after ``session["agent"]`` is set but + before reconciliation runs applies the switch itself and pins the same + ``model_override`` this function then reads, so the ``!= before`` guard + stays true and the switch runs a second time. The duplicate is not + harmless: it restarts the slash worker, appends a second switch marker to + the transcript, and re-emits ``session.info``. + """ + switch_calls: list = [] + monkeypatch.setattr( + "hermes_cli.model_switch.switch_model", + lambda **kwargs: switch_calls.append(kwargs) + or _switch_result("new/model", "anthropic"), + ) + + side_effects: list[str] = [] + monkeypatch.setattr( + server, "_restart_slash_worker", lambda *_a: side_effects.append("slash_worker") + ) + monkeypatch.setattr( + server, + "_append_model_switch_marker", + lambda *_a, **_kw: side_effects.append("switch_marker"), + ) + monkeypatch.setattr( + server, "_emit", lambda kind, *_a, **_kw: side_effects.append(f"emit:{kind}") + ) + monkeypatch.setattr(server, "_session_info", lambda _a, *_a2: {"model": "x"}) + monkeypatch.setattr(server, "_persist_live_session_runtime", lambda _s: None) + monkeypatch.setattr(server, "_persist_live_session_system_prompt", lambda _s: None) + + agent = _BuildRaceAgent() + # The concurrent live-apply already landed: the agent runs the target model + # and the rich override it pinned is what the reconcile reads back. + agent.model = "new/model" + agent.provider = "anthropic" + session = _session(agent=agent) + session["model_override"] = { + "model": "new/model", + "provider": "anthropic", + "base_url": "https://example.invalid", + "api_key": "sk-test", + "api_mode": "chat_completions", + } + + server._reconcile_deferred_build_overrides( + "sid", session, agent, {"model": None, "reasoning": None, "tier": None} + ) + + assert switch_calls == [], "resolved a switch for a model already installed" + assert agent.switch_calls == [], "swapped the client for a no-op switch" + assert side_effects == [], ( + "duplicated the live writer's side effects: " f"{side_effects}" + ) + assert session["model_override"]["model"] == "new/model", ( + "the pin the live writer wrote must survive the skip" + ) + + # ── billing/subscription state + error serialization ───────────────── diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 5ddd7c6823b9e..3490dca6d8b9e 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -2151,7 +2151,18 @@ def _reconcile_deferred_build_overrides( if isinstance(override, dict) and override != before.get("model"): model = str(override.get("model") or "").strip() provider = str(override.get("provider") or "").strip() - if model: + # Already running the requested model: a writer that took its LIVE + # branch between the agent's installation and this call has applied the + # switch itself and pinned the ``model_override`` we just read, but + # ``before`` is the pre-build snapshot, so the guard above cannot see + # that. Re-running the switch would duplicate its side effects — a + # second slash-worker restart, a second switch marker in the + # transcript, a redundant ``session.info`` — for no change. Same + # baseline-adoption check ``_sync_agent_model_with_config`` makes. + already_live = model == getattr(agent, "model", "") and ( + not provider or provider == getattr(agent, "provider", "") + ) + if model and not already_live: raw = f"{model} --provider {provider}" if provider else model try: _apply_model_switch( @@ -2187,16 +2198,31 @@ def _reconcile_deferred_build_overrides( tier = session.get("create_service_tier_override") if tier is not None and tier != before.get("tier"): - agent.service_tier = "priority" if tier == "priority" else None - request_overrides = dict(getattr(agent, "request_overrides", {}) or {}) - request_overrides.pop("service_tier", None) - request_overrides.pop("speed", None) + fast_overrides = None if tier == "priority": from hermes_cli.models import resolve_fast_mode_overrides fast_overrides = resolve_fast_mode_overrides(getattr(agent, "model", None)) - if fast_overrides: - request_overrides.update(fast_overrides) + if fast_overrides is None: + # ``config.set fast`` refuses the pin outright (4002, "fast mode + # is not available for this model") rather than recording a tier + # the model cannot honor, so ``create_service_tier_override == + # "priority"`` has always implied resolvable overrides. The + # model switch reconciled above can invalidate that after the + # fact. Erroring is not available here — nothing is listening + # to a background build — so mirror the resolver precedence the + # live path enforces and drop the pin instead of advertising + # ``service_tier="priority"`` in ``session.info`` with no + # matching request overrides behind it. + session.pop("create_service_tier_override", None) + # Priority iff the overrides actually resolved: the same invariant the + # live path gets from its early return. + agent.service_tier = "priority" if fast_overrides else None + request_overrides = dict(getattr(agent, "request_overrides", {}) or {}) + request_overrides.pop("service_tier", None) + request_overrides.pop("speed", None) + if fast_overrides: + request_overrides.update(fast_overrides) agent.request_overrides = request_overrides changed = True