[verified] fix: scope model changes to the active profile
This commit is contained in:
@@ -29,6 +29,7 @@ from gateway.platforms.base import (
|
|||||||
BasePlatformAdapter,
|
BasePlatformAdapter,
|
||||||
SendResult,
|
SendResult,
|
||||||
)
|
)
|
||||||
|
from hermes_constants import get_hermes_home
|
||||||
|
|
||||||
from . import protocol as proto
|
from . import protocol as proto
|
||||||
from . import hermes_bridge
|
from . import hermes_bridge
|
||||||
@@ -59,6 +60,11 @@ class PhebyAdapter(BasePlatformAdapter):
|
|||||||
platform._value_ = "pheby"
|
platform._value_ = "pheby"
|
||||||
platform._name_ = "PHEBY"
|
platform._name_ = "PHEBY"
|
||||||
super().__init__(config=config, platform=platform)
|
super().__init__(config=config, platform=platform)
|
||||||
|
# WebSocket control requests run outside normal gateway turns. Capture
|
||||||
|
# the profile root while the adapter is constructed under its routed
|
||||||
|
# profile context so model changes can install the matching credential
|
||||||
|
# scope later instead of reading another profile's environment.
|
||||||
|
self.hermes_home = get_hermes_home()
|
||||||
self._pcfg: PhebyConfig = load_config(config.extra or {})
|
self._pcfg: PhebyConfig = load_config(config.extra or {})
|
||||||
self._server: Any = None
|
self._server: Any = None
|
||||||
self._loop: Optional[asyncio.AbstractEventLoop] = None
|
self._loop: Optional[asyncio.AbstractEventLoop] = None
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ import logging
|
|||||||
import threading
|
import threading
|
||||||
import time
|
import time
|
||||||
import uuid
|
import uuid
|
||||||
|
from pathlib import Path
|
||||||
from typing import Any, Dict, List, Optional, Tuple
|
from typing import Any, Dict, List, Optional, Tuple
|
||||||
|
|
||||||
from . import protocol as proto
|
from . import protocol as proto
|
||||||
@@ -682,7 +683,7 @@ async def models_snapshot(conversation_id: Optional[str] = None) -> Dict[str, An
|
|||||||
"""Providers + models Hermes currently exposes (credential-aware)."""
|
"""Providers + models Hermes currently exposes (credential-aware)."""
|
||||||
def _collect() -> Dict[str, Any]:
|
def _collect() -> Dict[str, Any]:
|
||||||
from hermes_cli.config import get_compatible_custom_providers
|
from hermes_cli.config import get_compatible_custom_providers
|
||||||
from hermes_cli.model_switch import list_picker_providers
|
from hermes_cli.model_switch_providers import list_picker_providers
|
||||||
cfg = _load_cfg()
|
cfg = _load_cfg()
|
||||||
model_cfg = (cfg.get("model") or {}) if isinstance(cfg, dict) else {}
|
model_cfg = (cfg.get("model") or {}) if isinstance(cfg, dict) else {}
|
||||||
current_model = str(model_cfg.get("default", "") or "")
|
current_model = str(model_cfg.get("default", "") or "")
|
||||||
@@ -751,9 +752,31 @@ async def set_model(model: str, provider: Optional[str],
|
|||||||
if not model:
|
if not model:
|
||||||
return {"ok": False, "code": proto.ERR_BAD_REQUEST,
|
return {"ok": False, "code": proto.ERR_BAD_REQUEST,
|
||||||
"message": "model is required"}
|
"message": "model is required"}
|
||||||
|
home_token = None
|
||||||
|
secret_token = None
|
||||||
try:
|
try:
|
||||||
|
from agent.secret_scope import (
|
||||||
|
build_profile_secret_scope, reset_secret_scope, set_secret_scope,
|
||||||
|
)
|
||||||
from hermes_cli.config import get_compatible_custom_providers
|
from hermes_cli.config import get_compatible_custom_providers
|
||||||
from hermes_cli.model_switch import switch_model
|
from hermes_cli.model_switch import switch_model
|
||||||
|
from hermes_constants import (
|
||||||
|
get_hermes_home, reset_hermes_home_override,
|
||||||
|
set_hermes_home_override,
|
||||||
|
)
|
||||||
|
|
||||||
|
adapter = _current_adapter()
|
||||||
|
profile_home = Path(
|
||||||
|
getattr(adapter, "hermes_home", None) or get_hermes_home())
|
||||||
|
# model.set is a WebSocket control request, not a gateway message turn,
|
||||||
|
# so the runner has not installed this profile's ContextVars for us.
|
||||||
|
# Bind them explicitly before load_config()/switch_model(); to_thread
|
||||||
|
# copies the current context into its worker.
|
||||||
|
home_token = set_hermes_home_override(str(profile_home))
|
||||||
|
secrets = await asyncio.to_thread(
|
||||||
|
build_profile_secret_scope, profile_home)
|
||||||
|
secret_token = set_secret_scope(secrets)
|
||||||
|
|
||||||
cfg = _load_cfg()
|
cfg = _load_cfg()
|
||||||
model_cfg = (cfg.get("model") or {}) if isinstance(cfg, dict) else {}
|
model_cfg = (cfg.get("model") or {}) if isinstance(cfg, dict) else {}
|
||||||
result = await asyncio.to_thread(
|
result = await asyncio.to_thread(
|
||||||
@@ -762,7 +785,7 @@ async def set_model(model: str, provider: Optional[str],
|
|||||||
str(model_cfg.get("provider", "openrouter") or "openrouter"),
|
str(model_cfg.get("provider", "openrouter") or "openrouter"),
|
||||||
str(model_cfg.get("default", "") or ""),
|
str(model_cfg.get("default", "") or ""),
|
||||||
str(model_cfg.get("base_url", "") or ""),
|
str(model_cfg.get("base_url", "") or ""),
|
||||||
"", # current_api_key — runtime resolution handles credentials
|
"", # current_api_key — scoped runtime resolution handles credentials
|
||||||
False, # is_global → session-scoped when conversation given
|
False, # is_global → session-scoped when conversation given
|
||||||
provider or "",
|
provider or "",
|
||||||
cfg.get("providers") if isinstance(cfg, dict) else None,
|
cfg.get("providers") if isinstance(cfg, dict) else None,
|
||||||
@@ -772,15 +795,23 @@ async def set_model(model: str, provider: Optional[str],
|
|||||||
logger.error("[pheby] switch_model failed", exc_info=True)
|
logger.error("[pheby] switch_model failed", exc_info=True)
|
||||||
return {"ok": False, "code": proto.ERR_BAD_REQUEST,
|
return {"ok": False, "code": proto.ERR_BAD_REQUEST,
|
||||||
"message": proto.safe_str(exc, 200)}
|
"message": proto.safe_str(exc, 200)}
|
||||||
|
finally:
|
||||||
|
if secret_token is not None:
|
||||||
|
reset_secret_scope(secret_token)
|
||||||
|
if home_token is not None:
|
||||||
|
reset_hermes_home_override(home_token)
|
||||||
|
|
||||||
ok = bool(getattr(result, "success", False))
|
ok = bool(getattr(result, "success", False))
|
||||||
if not ok:
|
if not ok:
|
||||||
|
error = (getattr(result, "error_message", "") or
|
||||||
|
getattr(result, "error", ""))
|
||||||
return {"ok": False, "code": proto.ERR_BAD_REQUEST,
|
return {"ok": False, "code": proto.ERR_BAD_REQUEST,
|
||||||
"message": proto.safe_str(getattr(result, "error", ""),
|
"message": proto.safe_str(error, 300)}
|
||||||
300)}
|
|
||||||
|
|
||||||
resolved_model = getattr(result, "model", model)
|
resolved_model = (getattr(result, "new_model", "") or
|
||||||
resolved_provider = getattr(result, "provider", provider or "")
|
getattr(result, "model", "") or model)
|
||||||
|
resolved_provider = (getattr(result, "target_provider", "") or
|
||||||
|
getattr(result, "provider", "") or provider or "")
|
||||||
override = {"model": resolved_model}
|
override = {"model": resolved_model}
|
||||||
if resolved_provider:
|
if resolved_provider:
|
||||||
override["provider"] = resolved_provider
|
override["provider"] = resolved_provider
|
||||||
@@ -788,14 +819,36 @@ async def set_model(model: str, provider: Optional[str],
|
|||||||
store = _session_store()
|
store = _session_store()
|
||||||
if conversation_id and store is not None:
|
if conversation_id and store is not None:
|
||||||
try:
|
try:
|
||||||
|
session_key = _session_key_for(conversation_id)
|
||||||
if await asyncio.to_thread(
|
if await asyncio.to_thread(
|
||||||
store.peek_session_id,
|
store.peek_session_id, session_key) is None:
|
||||||
_session_key_for(conversation_id)) is None:
|
|
||||||
return {"ok": False, "code": proto.ERR_CONVERSATION_NOT_FOUND,
|
return {"ok": False, "code": proto.ERR_CONVERSATION_NOT_FOUND,
|
||||||
"message": "Conversation not found"}
|
"message": "Conversation not found"}
|
||||||
await asyncio.to_thread(store.set_model_override,
|
await asyncio.to_thread(store.set_model_override,
|
||||||
_session_key_for(conversation_id),
|
session_key, override)
|
||||||
override)
|
|
||||||
|
# Match Hermes's native /model commit path: the persisted override
|
||||||
|
# survives restarts, while the richer in-memory override gives the
|
||||||
|
# very next turn its resolved endpoint/key/capabilities. Evict any
|
||||||
|
# cached agent so it cannot answer once more with the old model.
|
||||||
|
runner = _runner()
|
||||||
|
runtime_overrides = getattr(
|
||||||
|
runner, "_session_model_overrides", None)
|
||||||
|
if isinstance(runtime_overrides, dict):
|
||||||
|
runtime_overrides[session_key] = {
|
||||||
|
"model": resolved_model,
|
||||||
|
"provider": resolved_provider,
|
||||||
|
"api_key": getattr(result, "api_key", "") or "",
|
||||||
|
"base_url": getattr(result, "base_url", "") or "",
|
||||||
|
"api_mode": getattr(result, "api_mode", "") or "",
|
||||||
|
"request_overrides": dict(
|
||||||
|
getattr(result, "request_overrides", None) or {}),
|
||||||
|
"capabilities": dict(
|
||||||
|
getattr(result, "runtime_capabilities", None) or {}),
|
||||||
|
}
|
||||||
|
evict = getattr(runner, "_evict_cached_agent", None)
|
||||||
|
if callable(evict):
|
||||||
|
evict(session_key)
|
||||||
return {"ok": True, "model": resolved_model,
|
return {"ok": True, "model": resolved_model,
|
||||||
"provider": resolved_provider, "scope": "conversation"}
|
"provider": resolved_provider, "scope": "conversation"}
|
||||||
except Exception:
|
except Exception:
|
||||||
|
|||||||
+115
-2
@@ -16,6 +16,7 @@ import json
|
|||||||
import threading
|
import threading
|
||||||
import time
|
import time
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
from types import SimpleNamespace
|
||||||
from typing import Any, Dict, List, Optional
|
from typing import Any, Dict, List, Optional
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
@@ -703,7 +704,7 @@ class TestBridge:
|
|||||||
|
|
||||||
from pheby import hermes_bridge as hb
|
from pheby import hermes_bridge as hb
|
||||||
import hermes_cli.config as hermes_config
|
import hermes_cli.config as hermes_config
|
||||||
import hermes_cli.model_switch as model_switch
|
import hermes_cli.model_switch_providers as model_switch_providers
|
||||||
monkeypatch.setattr(hb, "_load_cfg", lambda: {
|
monkeypatch.setattr(hb, "_load_cfg", lambda: {
|
||||||
"model": {"default": "m1", "provider": "test"},
|
"model": {"default": "m1", "provider": "test"},
|
||||||
"providers": {"test": {"base_url": "http://example"}},
|
"providers": {"test": {"base_url": "http://example"}},
|
||||||
@@ -713,7 +714,7 @@ class TestBridge:
|
|||||||
hermes_config, "get_compatible_custom_providers",
|
hermes_config, "get_compatible_custom_providers",
|
||||||
lambda _cfg: [{"name": "test", "base_url": "http://example"}])
|
lambda _cfg: [{"name": "test", "base_url": "http://example"}])
|
||||||
monkeypatch.setattr(
|
monkeypatch.setattr(
|
||||||
model_switch, "list_picker_providers", fake_list_picker_providers)
|
model_switch_providers, "list_picker_providers", fake_list_picker_providers)
|
||||||
|
|
||||||
snapshot = await hb.models_snapshot()
|
snapshot = await hb.models_snapshot()
|
||||||
assert snapshot["providers"][0]["slug"] == "test"
|
assert snapshot["providers"][0]["slug"] == "test"
|
||||||
@@ -721,6 +722,118 @@ class TestBridge:
|
|||||||
assert captured["excluded_providers"] == ["hidden"]
|
assert captured["excluded_providers"] == ["hidden"]
|
||||||
assert captured["custom_providers"][0]["name"] == "test"
|
assert captured["custom_providers"][0]["name"] == "test"
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_model_set_uses_profile_secret_scope_and_current_result_fields(
|
||||||
|
self, tmp_path, monkeypatch):
|
||||||
|
from agent.secret_scope import (
|
||||||
|
get_secret, reset_secret_scope, set_multiplex_active,
|
||||||
|
set_secret_scope,
|
||||||
|
)
|
||||||
|
from pheby import hermes_bridge as hb
|
||||||
|
import hermes_cli.config as hermes_config
|
||||||
|
import hermes_cli.model_switch as model_switch
|
||||||
|
|
||||||
|
(tmp_path / ".env").write_text(
|
||||||
|
"OPENROUTER_API_KEY=scoped-openrouter-key\n", encoding="utf-8")
|
||||||
|
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||||
|
monkeypatch.setattr(hb, "_load_cfg", lambda: {
|
||||||
|
"model": {"default": "gpt-old", "provider": "openai-codex"},
|
||||||
|
})
|
||||||
|
monkeypatch.setattr(
|
||||||
|
hermes_config, "get_compatible_custom_providers", lambda _cfg: [])
|
||||||
|
|
||||||
|
def fake_switch_model(*_args, **_kwargs):
|
||||||
|
assert get_secret("OPENROUTER_API_KEY") == "scoped-openrouter-key"
|
||||||
|
return SimpleNamespace(
|
||||||
|
success=True,
|
||||||
|
new_model="z-ai/glm-5.3-flash",
|
||||||
|
target_provider="openrouter",
|
||||||
|
api_key="scoped-openrouter-key",
|
||||||
|
base_url="https://openrouter.ai/api/v1",
|
||||||
|
api_mode="chat_completions",
|
||||||
|
request_overrides={},
|
||||||
|
runtime_capabilities={"native_compaction": False},
|
||||||
|
)
|
||||||
|
|
||||||
|
monkeypatch.setattr(model_switch, "switch_model", fake_switch_model)
|
||||||
|
|
||||||
|
class Store:
|
||||||
|
saved = None
|
||||||
|
|
||||||
|
def peek_session_id(self, _session_key):
|
||||||
|
return "session-1"
|
||||||
|
|
||||||
|
def set_model_override(self, _session_key, override):
|
||||||
|
self.saved = override
|
||||||
|
|
||||||
|
store = Store()
|
||||||
|
|
||||||
|
class Runner:
|
||||||
|
def __init__(self):
|
||||||
|
self._session_model_overrides = {}
|
||||||
|
self.evicted = []
|
||||||
|
|
||||||
|
def _evict_cached_agent(self, session_key):
|
||||||
|
self.evicted.append(session_key)
|
||||||
|
|
||||||
|
runner = Runner()
|
||||||
|
monkeypatch.setattr(hb, "_session_store", lambda: store)
|
||||||
|
monkeypatch.setattr(hb, "_runner", lambda: runner)
|
||||||
|
outer_token = set_secret_scope(None)
|
||||||
|
set_multiplex_active(True)
|
||||||
|
try:
|
||||||
|
result = await hb.set_model("glm-flash", "openrouter", "conv")
|
||||||
|
finally:
|
||||||
|
set_multiplex_active(False)
|
||||||
|
reset_secret_scope(outer_token)
|
||||||
|
|
||||||
|
assert result == {
|
||||||
|
"ok": True,
|
||||||
|
"model": "z-ai/glm-5.3-flash",
|
||||||
|
"provider": "openrouter",
|
||||||
|
"scope": "conversation",
|
||||||
|
}
|
||||||
|
assert store.saved == {
|
||||||
|
"model": "z-ai/glm-5.3-flash",
|
||||||
|
"provider": "openrouter",
|
||||||
|
}
|
||||||
|
assert runner._session_model_overrides[
|
||||||
|
"agent:main:pheby:dm:conv"] == {
|
||||||
|
"model": "z-ai/glm-5.3-flash",
|
||||||
|
"provider": "openrouter",
|
||||||
|
"api_key": "scoped-openrouter-key",
|
||||||
|
"base_url": "https://openrouter.ai/api/v1",
|
||||||
|
"api_mode": "chat_completions",
|
||||||
|
"request_overrides": {},
|
||||||
|
"capabilities": {"native_compaction": False},
|
||||||
|
}
|
||||||
|
assert runner.evicted == ["agent:main:pheby:dm:conv"]
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_model_set_surfaces_current_hermes_error_message(
|
||||||
|
self, monkeypatch):
|
||||||
|
from pheby import hermes_bridge as hb
|
||||||
|
import hermes_cli.config as hermes_config
|
||||||
|
import hermes_cli.model_switch as model_switch
|
||||||
|
|
||||||
|
monkeypatch.setattr(hb, "_load_cfg", lambda: {"model": {}})
|
||||||
|
monkeypatch.setattr(
|
||||||
|
hermes_config, "get_compatible_custom_providers", lambda _cfg: [])
|
||||||
|
monkeypatch.setattr(
|
||||||
|
model_switch,
|
||||||
|
"switch_model",
|
||||||
|
lambda *_args, **_kwargs: SimpleNamespace(
|
||||||
|
success=False,
|
||||||
|
error_message="OpenRouter credentials unavailable",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
result = await hb.set_model("z-ai/glm-5.3-flash", "openrouter", None)
|
||||||
|
|
||||||
|
assert result["ok"] is False
|
||||||
|
assert result["code"] == proto.ERR_BAD_REQUEST
|
||||||
|
assert result["message"] == "OpenRouter credentials unavailable"
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_cancel_run_interrupts_agent(self, tmp_path):
|
async def test_cancel_run_interrupts_agent(self, tmp_path):
|
||||||
server = make_server(tmp_path)
|
server = make_server(tmp_path)
|
||||||
|
|||||||
Reference in New Issue
Block a user