mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
fix(gateway): keep only the custom-identity fixes main does not cover
Main's session_provider_identity (#143660, 0bc8789d/69d5e0e4/f3b5b980) already makes the turn sync and the TUI runtime record the entry the agent resolved while it serves the session's base_url (MP-23/MP-25). Drop the duplicate writers: the run_turn_prepare heal block (including its fallback guard, redundant since fallbacks overwrite requested_provider) and the update_session_model heal, plus the guess=False/routed_custom_identity_in machinery behind them and the tests that pass on main. Keep: a bare custom launch pick freezes its entry (MP-19), disabled entries never name a route, /status and the model pickers name the live agent's entry via main's session_provider_identity (MP-28, picker current-entry), the MP-20 tuple fix and the CLI-43 changes. Refs #143547
This commit is contained in:
1 parent
f6280475f8
commit
f65c227878
7 files changed
+45
-137
No files matched your search
@@ -276,13 +276,6 @@ class GatewayTurnPrepareMixin:
|
||||
runtime = {k: v for k, v in runtime.items() if v not in (None, "")}
|
||||
try:
|
||||
db = self._session_db._db
|
||||
if runtime.get("provider") == "custom":
|
||||
# The adapter's billing class is shared by every named endpoint: resume needs the entry,
|
||||
# the one the runtime resolved unless a fallback moved the turn off it.
|
||||
from hermes_cli.runtime_provider_custom import routed_custom_identity_in
|
||||
requested = None if runtime["fallback_active"] else getattr(agent, "requested_provider", None)
|
||||
runtime["provider"] = routed_custom_identity_in(
|
||||
Path(db.db_path).parent, runtime.get("base_url"), model, requested) or "custom"
|
||||
row = db.get_session(session_id)
|
||||
if not row:
|
||||
return
|
||||
|
||||
@@ -225,10 +225,8 @@ async def model_options(connection, ref, params):
|
||||
selected = provider
|
||||
if selected == 'custom':
|
||||
from hermes_cli.runtime_provider import canonical_custom_identity
|
||||
requested = None if getattr(agent, '_fallback_activated', False) else getattr(
|
||||
agent, 'requested_provider', None)
|
||||
selected = canonical_custom_identity(base_url=base_url, config_provider=ctx.current_provider,
|
||||
model=model, requested=requested) or selected
|
||||
selected = canonical_custom_identity(base_url=base_url, config_provider=ctx.current_provider, model=model,
|
||||
requested=getattr(agent, 'requested_provider', None)) or selected
|
||||
return build_model_options_payload(ctx.with_overrides(current_provider=selected,
|
||||
current_model=model, current_base_url=base_url), **{k: params[k] for k in flags & params.keys()})
|
||||
return await asyncio.to_thread(discover)
|
||||
@@ -12,6 +12,7 @@ from __future__ import annotations
|
||||
import asyncio
|
||||
import contextlib
|
||||
from collections import Counter
|
||||
from pathlib import Path
|
||||
|
||||
# The TUI surfaces whose /status and /context read as main's TUI sidecar did.
|
||||
_TUI_SOURCES = frozenset({'tui'})
|
||||
@@ -50,10 +51,11 @@ def _status(authority, ref, live, policy, agent, arg) -> str:
|
||||
tokens=getattr(agent, 'session_total_tokens', 0) if agent is not None else None,
|
||||
agent_running=running, home=authority.profile_id)
|
||||
if fields['provider'] == 'custom': # the adapter's billing class: show the entry the session runs on
|
||||
from hermes_cli.runtime_provider_custom import routed_custom_identity_in
|
||||
fields['provider'] = routed_custom_identity_in(
|
||||
authority.profile_id, getattr(agent, 'base_url', None) or policy.base_url, fields['model'],
|
||||
getattr(agent, 'requested_provider', None)) or 'custom'
|
||||
from gateway.run import _profile_runtime_scope
|
||||
from hermes_cli.runtime_provider_custom import session_provider_identity
|
||||
with _profile_runtime_scope(Path(authority.profile_id)):
|
||||
fields['provider'] = session_provider_identity('custom', getattr(agent, 'requested_provider', None),
|
||||
getattr(agent, 'base_url', None) or policy.base_url, fields['model'])
|
||||
project = server._project_info_for_cwd(policy.cwd)
|
||||
return '\n'.join([
|
||||
'Hermes TUI Status', '', *status_lines(fields, 'session_id', 'path'),
|
||||
|
||||
@@ -9,7 +9,6 @@ from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import os
|
||||
from pathlib import Path
|
||||
from typing import Any, Callable, Dict, Optional, Tuple
|
||||
|
||||
from hermes_cli.providers import custom_provider_aliases, custom_provider_slug
|
||||
@@ -299,35 +298,30 @@ def codex_model_provider_id(requested_provider: str) -> Optional[str]:
|
||||
# ── identity recovery (bare "custom" -> durable ``custom:<name>``) ─────────────────────────
|
||||
|
||||
|
||||
def _custom_identities(matches: Callable[[dict[str, Any]], bool], config: Optional[dict[str, Any]] = None):
|
||||
"""Every entry in ``providers:`` then legacy ``custom_providers:`` where ``matches(entry)``
|
||||
def _find_custom_identity(matches: Callable[[dict[str, Any]], bool],
|
||||
config: Optional[dict[str, Any]] = None) -> Optional[str]:
|
||||
"""First enabled entry in ``providers:`` then legacy ``custom_providers:`` where ``matches(entry)``
|
||||
holds, as its canonical ``custom:<name>`` slug. *config* defaults to the active profile's."""
|
||||
from hermes_cli.config import is_provider_enabled
|
||||
rp = _rp()
|
||||
try:
|
||||
config = rp.load_config() if config is None else config
|
||||
except Exception:
|
||||
logger.debug("custom provider identity: config unreadable", exc_info=True)
|
||||
return
|
||||
return None
|
||||
providers = config.get("providers")
|
||||
if isinstance(providers, dict):
|
||||
for ep_name, entry in providers.items():
|
||||
if isinstance(entry, dict) and is_provider_enabled(entry) and matches(entry):
|
||||
yield custom_provider_slug(str(ep_name), str(ep_name))
|
||||
return custom_provider_slug(str(ep_name), str(ep_name))
|
||||
try:
|
||||
custom_providers = rp.get_compatible_custom_providers(config)
|
||||
except Exception:
|
||||
logger.debug("custom provider identity: custom_providers unreadable", exc_info=True)
|
||||
custom_providers = None
|
||||
for entry in custom_providers or []:
|
||||
name = entry.get("name") if isinstance(entry, dict) else None
|
||||
if isinstance(name, str) and name.strip() and matches(entry):
|
||||
yield custom_provider_slug(name, str(entry.get("provider_key", "") or ""))
|
||||
|
||||
|
||||
def _find_custom_identity(matches: Callable[[dict[str, Any]], bool],
|
||||
config: Optional[dict[str, Any]] = None) -> Optional[str]:
|
||||
return next(_custom_identities(matches, config), None)
|
||||
return custom_provider_slug(name, str(entry.get("provider_key", "") or ""))
|
||||
return None
|
||||
|
||||
|
||||
def find_custom_provider_identity(base_url: str) -> Optional[str]:
|
||||
@@ -364,46 +358,23 @@ def _entry_serves_model(entry: dict[str, Any], target: str) -> bool:
|
||||
return False
|
||||
|
||||
|
||||
def routed_custom_identity(base_url: Optional[str] = None, model: Optional[str] = None,
|
||||
config: Optional[dict[str, Any]] = None, requested: Optional[str] = None,
|
||||
guess: bool = True) -> Optional[str]:
|
||||
"""The configured ``custom:<name>`` entry a bare ``custom`` route (the billing class every named
|
||||
endpoint reports) ran on. The entry the runtime resolved (*requested*) is that route when it
|
||||
sits on *base_url*: entries sharing one relay may list each other's models, so (URL, model) is
|
||||
only a guess for when no entry is known. Then the entry also serving *model* beats the first one
|
||||
on that URL; without a URL the model alone names it. ``guess=False`` (a record of the route that
|
||||
ran) names an unknown entry only when it is the one enabled on *base_url*."""
|
||||
def routed_custom_identity(base_url: Optional[str], model: Optional[str],
|
||||
config: Optional[dict[str, Any]] = None) -> Optional[str]:
|
||||
"""The configured ``custom:<name>`` entry a bare ``custom`` pick (the billing class every named
|
||||
endpoint reports) names: on *base_url*, the entry also serving *model* beats the first one there;
|
||||
without a URL the model alone names it."""
|
||||
url = _route_key(base_url)
|
||||
target = str(model or "").strip().lower()
|
||||
if not url:
|
||||
return _find_custom_identity(lambda entry: _entry_serves_model(entry, target), config) if target and guess else None
|
||||
|
||||
def on_url(entry: dict[str, Any]) -> bool:
|
||||
return _route_key(_entry_url(entry)) == url
|
||||
named = (_get_named_custom_provider(requested, config=config, resolve_credentials=False)
|
||||
if requested and _normalize_custom_provider_name(requested) != "custom" else None)
|
||||
if named and on_url(named):
|
||||
return custom_provider_slug(str(named.get("name") or ""), str(named.get("provider_key") or ""))
|
||||
if not guess:
|
||||
# A bare-custom /model switch keeps the session's key whichever sibling lists the model.
|
||||
on = set(_custom_identities(on_url, config))
|
||||
return on.pop() if len(on) == 1 else None
|
||||
return not url or _route_key(_entry_url(entry)) == url
|
||||
return ((target and _find_custom_identity(lambda e: on_url(e) and _entry_serves_model(e, target), config))
|
||||
or _find_custom_identity(on_url, config))
|
||||
or (url and _find_custom_identity(on_url, config)) or None)
|
||||
|
||||
|
||||
def routed_custom_identity_in(home: Any, base_url: Optional[str] = None,
|
||||
model: Optional[str] = None, requested: Optional[str] = None) -> Optional[str]:
|
||||
"""The entry a session at *home* ran on (:func:`routed_custom_identity` with ``guess=False``), named
|
||||
from its own profile's config, never the ambient scope's (one gateway serves many profiles)."""
|
||||
from hermes_cli.config_effective import load_user_config_effective
|
||||
return routed_custom_identity(base_url, model, load_user_config_effective(Path(home) / "config.yaml"),
|
||||
requested, guess=False)
|
||||
|
||||
|
||||
def _endpoint_identity(base_url: str, requested: Optional[str] = None) -> Optional[str]:
|
||||
def _endpoint_identity(base_url: str) -> Optional[str]:
|
||||
"""The menu identity that owns ``base_url``: a configured entry, else the managed server."""
|
||||
identity = routed_custom_identity(base_url, requested=requested)
|
||||
identity = find_custom_provider_identity(base_url)
|
||||
if identity:
|
||||
return identity
|
||||
# The managed server has no custom-provider config entry. Recover its menu key
|
||||
@@ -421,9 +392,9 @@ def canonical_custom_identity(*, base_url: Optional[str] = None, config_provider
|
||||
endpoint first, then the ownership-checked managed server, then a configured model or
|
||||
provider. Every session persistence/restore path shares this lookup."""
|
||||
rp = _rp()
|
||||
# No model: a sibling listing it is not the entry a bare-custom switch kept (URL-first, as ever).
|
||||
identity = _endpoint_identity(base_url, requested) if base_url else None
|
||||
if identity:
|
||||
# The entry the live agent resolved (*requested*) while it still serves base_url, else its owner.
|
||||
identity = session_provider_identity("custom", requested, base_url) if base_url else "custom"
|
||||
if identity != "custom":
|
||||
return identity
|
||||
identity = find_custom_provider_identity_by_model(model) if model else None
|
||||
if identity:
|
||||
|
||||
@@ -725,8 +725,9 @@ class SessionSessionsMixin:
|
||||
keys for the TUI/Desktop, ``gateway_runtime`` for the CLI), so a later resume recombines the
|
||||
model with the provider that serves it (#79536). ``base_url``/``api_mode`` are always
|
||||
replaced then (``None`` deletes): the previous provider's endpoint must not survive a switch,
|
||||
or resume sends the new provider's model to the old host; a bare ``custom`` is stored as its
|
||||
entry when unambiguous. Callers without provider knowledge leave the stored route untouched."""
|
||||
or resume sends the new provider's model to the old host. Callers without provider knowledge
|
||||
leave the stored route untouched.
|
||||
"""
|
||||
# Flush first: a still-queued pre-switch delta applied after this UPDATE would trip the
|
||||
# first_accounted_route overwrite and resurrect the old route.
|
||||
self.flush_token_counts()
|
||||
@@ -734,9 +735,6 @@ class SessionSessionsMixin:
|
||||
if model:
|
||||
patch["model"] = model
|
||||
if provider:
|
||||
if provider == "custom": # the billing class every named endpoint reports, not a route
|
||||
from hermes_cli.runtime_provider_custom import routed_custom_identity_in
|
||||
provider = routed_custom_identity_in(Path(self.db_path).parent, base_url, model) or provider
|
||||
route = {"provider": provider, "base_url": base_url or None, "api_mode": api_mode or None}
|
||||
patch.update(route, gateway_runtime=route)
|
||||
self._write_model_config_patch(
|
||||
|
||||
@@ -1,9 +1,8 @@
|
||||
"""A bare ``custom`` provider is the billing class every named endpoint reports, never a route.
|
||||
|
||||
The gateway freezes, persists and syncs the configured ``custom:<name>`` entry it ran on: a launch
|
||||
pick echoing ``custom`` back, a ``/model`` switch persisting it, and a turn syncing the adapter's
|
||||
label. A turn records the entry its runtime resolved; only an unknown entry is told apart from
|
||||
siblings on one relay base_url by the model they serve.
|
||||
A launch pick echoing ``custom`` back freezes the configured ``custom:<name>`` entry, a disabled entry
|
||||
never names a route, and the model picker marks the entry the live agent resolved, not the first
|
||||
sibling on a shared relay base_url.
|
||||
|
||||
Also: a profile-following session's model pick with an inline secret outside the followed
|
||||
sections, and ``--source desktop``. No endpoint is contacted.
|
||||
@@ -45,78 +44,25 @@ def test_a_bare_custom_launch_pick_freezes_the_configured_entry(tmp_path, pick,
|
||||
assert (model['provider'], model.get('base_url')) == (provider, RELAY)
|
||||
|
||||
|
||||
def test_a_disabled_sibling_serving_the_model_never_names_the_route():
|
||||
from hermes_cli.runtime_provider_custom import routed_custom_identity
|
||||
config = {'providers': {**SIBLINGS['providers'],
|
||||
'deepseek': {**SIBLINGS['providers']['deepseek'], 'enabled': False}}}
|
||||
assert routed_custom_identity(RELAY, 'deepseek-v4.1-flash', config) == 'custom:glm'
|
||||
|
||||
|
||||
SPLIT_KEYS = {'providers': {name: {**entry, 'key_env': f'{name.upper()}_KEY'}
|
||||
for name, entry in SIBLINGS['providers'].items()}}
|
||||
|
||||
|
||||
def test_a_bare_custom_switch_resumes_on_the_key_that_served_it(home, monkeypatch):
|
||||
from gateway.run_turn_prepare import GatewayTurnPrepareMixin
|
||||
from hermes_cli.model_switch import switch_model
|
||||
from hermes_state import SessionDB
|
||||
from tui_gateway import server
|
||||
monkeypatch.setenv('GLM_KEY', 'glm-key')
|
||||
monkeypatch.setenv('DEEPSEEK_KEY', 'deepseek-key')
|
||||
(home / 'config.yaml').write_text(json.dumps(SPLIT_KEYS))
|
||||
# A glm session (the adapter reports bare ``custom``) runs /model deepseek-v4.1-flash --provider custom.
|
||||
switched = switch_model('deepseek-v4.1-flash', 'custom', 'glm-5.3', RELAY, 'glm-key',
|
||||
explicit_provider='custom', user_providers=SPLIT_KEYS['providers'])
|
||||
assert switched.success and switched.api_key == 'glm-key', switched
|
||||
db = SessionDB(db_path=home / 'state.db')
|
||||
try:
|
||||
db.create_session('s', source='cli', model='glm-5.3')
|
||||
db.update_session_model('s', switched.new_model, switched.target_provider, base_url=switched.base_url)
|
||||
# The next turn syncs the switched agent, whose requested provider is bare ``custom`` too.
|
||||
agent = SimpleNamespace(model=switched.new_model, provider='custom', requested_provider='custom',
|
||||
base_url=RELAY, api_mode='chat_completions')
|
||||
GatewayTurnPrepareMixin._sync_session_model_from_agent(
|
||||
SimpleNamespace(_session_db=SimpleNamespace(_db=db)), 's', agent)
|
||||
# Resume reads the stored route back and resolves its credentials.
|
||||
overrides = server._stored_session_runtime_overrides(db.get_session('s'))
|
||||
_model, runtime = server._resolve_agent_model_runtime(
|
||||
overrides['model_override'], overrides.get('provider_override'))
|
||||
assert runtime['api_key'] == 'glm-key'
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
@pytest.mark.parametrize('requested, model', [('custom:glm', 'deepseek-v4.1-flash'), ('custom:deepseek', 'glm-5.3')],
|
||||
ids=['glm-running-the-siblings-model', 'deepseek-running-the-siblings-model'])
|
||||
def test_a_turn_on_a_named_entry_persists_that_entry_not_the_sibling_listing_its_model(home, requested, model):
|
||||
from gateway.run_turn_prepare import GatewayTurnPrepareMixin
|
||||
from hermes_state import SessionDB
|
||||
(home / 'config.yaml').write_text(json.dumps(SIBLINGS))
|
||||
db = SessionDB(db_path=home / 'state.db')
|
||||
try:
|
||||
db.create_session('s', source='cli', model=model)
|
||||
agent = SimpleNamespace(model=model, provider='custom', requested_provider=requested, base_url=RELAY,
|
||||
api_mode='chat_completions')
|
||||
GatewayTurnPrepareMixin._sync_session_model_from_agent(
|
||||
SimpleNamespace(_session_db=SimpleNamespace(_db=db)), 's', agent)
|
||||
assert json.loads(db.get_session('s')['model_config'])['gateway_runtime']['provider'] == requested
|
||||
finally:
|
||||
db.close()
|
||||
def test_a_disabled_entry_on_the_url_never_names_the_route(home):
|
||||
from hermes_cli.runtime_provider_custom import session_provider_identity
|
||||
(home / 'config.yaml').write_text(json.dumps({'providers': {
|
||||
'glm': {**SIBLINGS['providers']['glm'], 'enabled': False}, 'deepseek': SIBLINGS['providers']['deepseek']}}))
|
||||
assert session_provider_identity('custom', 'custom', RELAY, 'deepseek-v4.1-flash') == 'custom:deepseek'
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize('requested, model', [('custom:glm', 'deepseek-v4.1-flash'), ('custom:deepseek', 'glm-5.3')],
|
||||
ids=['glm-running-the-siblings-model', 'deepseek-running-the-siblings-model'])
|
||||
async def test_the_model_picker_marks_the_entry_a_session_runs_on_current(home, monkeypatch, requested, model):
|
||||
async def test_the_model_picker_marks_the_entry_a_session_runs_on_current(home, monkeypatch):
|
||||
from gateway import session_config
|
||||
(home / 'config.yaml').write_text(json.dumps(SIBLINGS))
|
||||
agent = SimpleNamespace(model=model, provider='custom', requested_provider=requested, base_url=RELAY)
|
||||
# deepseek runs glm's model: the first entry on the shared URL (and the one listing it) is not the route.
|
||||
agent = SimpleNamespace(model='glm-5.3', provider='custom', requested_provider='custom:deepseek', base_url=RELAY)
|
||||
connection = SimpleNamespace(authority=SimpleNamespace(profile_id=str(home), agent=lambda ref: agent))
|
||||
monkeypatch.setattr(session_config, '_authorize', lambda connection, ref, params: None)
|
||||
monkeypatch.setattr('hermes_cli.inventory.build_model_options_payload',
|
||||
lambda ctx, **kwargs: {'provider': ctx.current_provider})
|
||||
result = await session_config.model_options(connection, SimpleNamespace(session_id='s'), {})
|
||||
assert result == {'provider': requested}
|
||||
assert result == {'provider': 'custom:deepseek'}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
@@ -136,9 +136,9 @@ def _model_picker_context(agent):
|
||||
if str(provider or "").strip().lower() == "custom":
|
||||
try:
|
||||
from hermes_cli.runtime_provider import canonical_custom_identity
|
||||
requested = None if getattr(agent, "_fallback_activated", False) else getattr(agent, "requested_provider", None)
|
||||
provider = canonical_custom_identity(base_url=base_url or None, config_provider=ctx.current_provider,
|
||||
model=model or None, requested=requested) or provider
|
||||
provider = canonical_custom_identity(
|
||||
base_url=base_url or None, config_provider=ctx.current_provider, model=model or None,
|
||||
requested=getattr(agent, "requested_provider", None)) or provider
|
||||
except Exception:
|
||||
logger.debug("custom provider identity recovery failed (model picker)", exc_info=True)
|
||||
return ctx.with_overrides(
|
||||
|
||||
Reference in new issue
Block a user