mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(tui): reject /model and agent-mutating slash passthroughs while running (#12548)
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.
This commit is contained in:
parent
a3b76ae36d
commit
d5fc8a5e00
2 changed files with 145 additions and 0 deletions
|
|
@ -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"]
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue