From d5fc8a5e00dfd396cd188f605ff2abc76fce3c2e Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 19 Apr 2026 05:19:57 -0700 Subject: [PATCH] fix(tui): reject /model and agent-mutating slash passthroughs while running (#12548) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit agent.switch_model() mutates self.model, self.provider, self.base_url, self.api_key, self.api_mode, and rebuilds self.client / self._anthropic_client in place. The worker thread running agent.run_conversation reads those fields on every iteration. A concurrent config.set key=model or slash- worker-mirrored /model / /personality / /prompt / /compress can send an HTTP request with mismatched model + base_url (or the old client keeps running against a new endpoint) — 400/404s the user never asked for. Fix: same pattern as the session.undo / session.compress guards (PR #12416) and the gateway runner's running-agent /model guard (PR #12334). Reject with 4009 'session busy' when session.running is True. Two call sites guarded: - config.set with key=model: primary /model entry point from Ink - _mirror_slash_side_effects for model / personality / prompt / compress: slash-worker passthrough path that applies live-agent side effects Idle sessions still switch models normally — regression guard test verifies this. Tests (tests/test_tui_gateway_server.py): 4 new cases. - test_config_set_model_rejects_while_running - test_config_set_model_allowed_when_idle (regression guard) - test_mirror_slash_side_effects_rejects_mutating_commands_while_running - test_mirror_slash_side_effects_allowed_when_idle (regression guard) Validated: against unpatched server.py, the two 'rejects_while_running' tests fail with the exact race they assert against. With the fix all 4 pass. Live E2E against the live Python environment confirmed both guards enforce 4009 / 'session busy' exactly as designed. --- tests/test_tui_gateway_server.py | 121 +++++++++++++++++++++++++++++++ tui_gateway/server.py | 24 ++++++ 2 files changed, 145 insertions(+) diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 07a68ac9e..c0f523903 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -828,3 +828,124 @@ def test_respond_unpacks_sid_tuple_correctly(): server._pending.pop("rid-x", None) server._answers.pop("rid-x", None) + + +# --------------------------------------------------------------------------- +# /model switch and other agent-mutating commands must reject while the +# session is running. agent.switch_model() mutates self.model, self.provider, +# self.base_url, self.client etc. in place — the worker thread running +# agent.run_conversation is reading those on every iteration. Same class of +# bug as the session.undo / session.compress mid-run silent-drop; same fix +# pattern: reject with 4009 while running. +# --------------------------------------------------------------------------- + + +def test_config_set_model_rejects_while_running(monkeypatch): + """/model via config.set must reject during an in-flight turn.""" + seen = {"called": False} + + def _fake_apply(sid, session, raw): + seen["called"] = True + return {"value": raw, "warning": ""} + + monkeypatch.setattr(server, "_apply_model_switch", _fake_apply) + + server._sessions["sid"] = _session(running=True) + try: + resp = server.handle_request({ + "id": "1", "method": "config.set", + "params": {"session_id": "sid", "key": "model", "value": "anthropic/claude-sonnet-4.6"}, + }) + assert resp.get("error") + assert resp["error"]["code"] == 4009 + assert "session busy" in resp["error"]["message"] + assert not seen["called"], ( + "_apply_model_switch was called mid-turn — would race with " + "the worker thread reading agent.model / agent.client" + ) + finally: + server._sessions.pop("sid", None) + + +def test_config_set_model_allowed_when_idle(monkeypatch): + """Regression guard: idle sessions can still switch models.""" + seen = {"called": False} + + def _fake_apply(sid, session, raw): + seen["called"] = True + return {"value": "newmodel", "warning": ""} + + monkeypatch.setattr(server, "_apply_model_switch", _fake_apply) + + server._sessions["sid"] = _session(running=False) + try: + resp = server.handle_request({ + "id": "1", "method": "config.set", + "params": {"session_id": "sid", "key": "model", "value": "newmodel"}, + }) + assert resp.get("result") + assert resp["result"]["value"] == "newmodel" + assert seen["called"] + finally: + server._sessions.pop("sid", None) + + +def test_mirror_slash_side_effects_rejects_mutating_commands_while_running(monkeypatch): + """Slash worker passthrough (e.g. /model, /personality, /prompt, + /compress) must reject during an in-flight turn. Same race as + config.set — mutates live agent state while run_conversation is + reading it.""" + import types + + applied = {"model": False, "compress": False} + + def _fake_apply_model(sid, session, arg): + applied["model"] = True + return {"value": arg, "warning": ""} + + def _fake_compress(session, focus): + applied["compress"] = True + return (0, {}) + + monkeypatch.setattr(server, "_apply_model_switch", _fake_apply_model) + monkeypatch.setattr(server, "_compress_session_history", _fake_compress) + + session = _session(running=True) + session["agent"] = types.SimpleNamespace(model="x") + + for cmd, expected_name in [ + ("/model new/model", "model"), + ("/personality default", "personality"), + ("/prompt", "prompt"), + ("/compress", "compress"), + ]: + warning = server._mirror_slash_side_effects("sid", session, cmd) + assert "session busy" in warning, ( + f"{cmd} should have returned busy warning, got: {warning!r}" + ) + assert f"/{expected_name}" in warning + + # None of the mutating side-effect helpers should have fired. + assert not applied["model"], "model switch fired despite running session" + assert not applied["compress"], "compress fired despite running session" + + +def test_mirror_slash_side_effects_allowed_when_idle(monkeypatch): + """Regression guard: idle session still runs the side effects.""" + import types + + applied = {"model": False} + + def _fake_apply_model(sid, session, arg): + applied["model"] = True + return {"value": arg, "warning": ""} + + monkeypatch.setattr(server, "_apply_model_switch", _fake_apply_model) + + session = _session(running=False) + session["agent"] = types.SimpleNamespace(model="x") + + warning = server._mirror_slash_side_effects("sid", session, "/model foo") + # Should NOT contain "session busy" — the switch went through. + assert "session busy" not in warning + assert applied["model"] diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 921f868a3..00f834619 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -1743,6 +1743,19 @@ def _(rid, params: dict) -> dict: if not value: return _err(rid, 4002, "model value required") if session: + # Reject during an in-flight turn. agent.switch_model() + # mutates self.model / self.provider / self.base_url / + # self.client in place; the worker thread running + # agent.run_conversation is reading those on every + # iteration. A mid-turn swap can send an HTTP request + # with the new base_url but old model (or vice versa), + # producing 400/404s the user never asked for. Parity + # with the gateway's running-agent /model guard. + if session.get("running"): + return _err( + rid, 4009, + "session busy — /interrupt the current turn before switching models", + ) result = _apply_model_switch(params.get("session_id", ""), session, value) else: result = _apply_model_switch("", {"agent": None}, value) @@ -2446,6 +2459,17 @@ def _mirror_slash_side_effects(sid: str, session: dict, command: str) -> str: return "" name, arg, agent = parts[0], (parts[1].strip() if len(parts) > 1 else ""), session.get("agent") + # Reject agent-mutating commands during an in-flight turn. These + # all do read-then-mutate on live agent/session state that the + # worker thread running agent.run_conversation is using. Parity + # with the session.compress / session.undo guards and the gateway + # runner's running-agent /model guard. + _MUTATES_WHILE_RUNNING = {"model", "personality", "prompt", "compress"} + if name in _MUTATES_WHILE_RUNNING and session.get("running"): + return ( + f"session busy — /interrupt the current turn before running /{name}" + ) + try: if name == "model" and arg and agent: result = _apply_model_switch(sid, session, arg)