refactor: single-owner model switch parsing + effective-model resolution (kills the api_server/run.py divergence class)

cli.py

@@ -9057,7 +9057,7 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin):
         """
         from hermes_cli.model_switch import (
             switch_model,
-            parse_model_flags_detailed,
+            parse_model_switch_args,
             resolve_persist_behavior,
         )
         from hermes_cli.providers import get_label
@@ -9067,18 +9067,17 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin):
         raw_args = parts[1].strip() if len(parts) > 1 else ""
 
         # Parse --provider, --global, --session, --once, and --refresh flags
-        parsed_flags = parse_model_flags_detailed(raw_args)
-        model_input = parsed_flags.model_input
-        explicit_provider = parsed_flags.explicit_provider
-        is_global_flag = parsed_flags.is_global
-        force_refresh = parsed_flags.force_refresh
-        is_session = parsed_flags.is_session
-        one_turn = parsed_flags.is_once
-        if is_global_flag and one_turn:
-            _cprint("  ✗ /model --once cannot be combined with --global")
-            return
-        if one_turn and not model_input and not explicit_provider:
-            _cprint("  ✗ /model --once requires a model or provider.")
+        # via the shared single-owner parser (hermes_cli.model_switch).
+        request = parse_model_switch_args(raw_args)
+        model_input = request.target
+        explicit_provider = request.explicit_provider
+        is_global_flag = request.is_global
+        force_refresh = request.force_refresh
+        is_session = request.is_session
+        one_turn = request.is_once
+        if request.errors:
+            # CLI decoration: "  ✗ " prefix over the canonical error copy.
+            _cprint(f"  ✗ {request.error_messages()[0]}")
             return
         # Resolve the effective persistence once: --global forces persist,
         # --session/--once force session-scope, otherwise defer to

gateway/platforms/api_server.py

@@ -1410,17 +1410,22 @@ class APIServerAdapter(BasePlatformAdapter):
         1. Explicit override (config extra or API_SERVER_MODEL_NAME env var)
         2. Active profile name (so each profile advertises a distinct model)
         3. Fallback: "hermes-agent"
+
+        Delegates the tiered fallthrough to
+        :func:`hermes_cli.model_switch.resolve_effective_model` (the shared
+        override > mid-tier > default precedence owner).
         """
-        if explicit and explicit.strip():
-            return explicit.strip()
+        from hermes_cli.model_switch import resolve_effective_model
+
+        profile_name = ""
         try:
             from hermes_cli.profiles import get_active_profile_name
             profile = get_active_profile_name()
             if profile and profile not in {"default", "custom"}:
-                return profile
+                profile_name = profile
         except Exception:
             pass
-        return "hermes-agent"
+        return resolve_effective_model(explicit, profile_name, "hermes-agent")
 
     def _cors_headers_for_origin(self, origin: str) -> Optional[Dict[str, str]]:
         """Return CORS headers for an allowed browser origin."""
@@ -2487,8 +2492,13 @@ class APIServerAdapter(BasePlatformAdapter):
         session_override = None
         if not confirmed_runtime_lock:
             session_override = self._session_model_override_for(session_key)
+        # Model-string precedence delegates to the shared owner
+        # hermes_cli.model_switch.resolve_effective_model (session /model
+        # override > session-persisted model > global) — the rule 7dd00bb47d
+        # had to re-fix here after it diverged from gateway/run.py.
+        from hermes_cli.model_switch import resolve_effective_model
         if session_override:
-            override_model = _clean_request_string(session_override.get("model")) or model
+            override_model = resolve_effective_model(session_override, None, model)
             session_provider = _clean_request_string(session_override.get("provider"))
             current_provider = _clean_request_string(runtime_kwargs.get("provider"))
             provider_runtime = _resolve_provider_runtime(
@@ -2518,7 +2528,7 @@ class APIServerAdapter(BasePlatformAdapter):
             )
             if provider_runtime:
                 _apply_runtime_agent_overrides(runtime_kwargs, provider_runtime)
-            model = session_row_model
+            model = resolve_effective_model(None, session_row_model, model)
             if request_model or request_provider:
                 logger.debug(
                     "api_server request selection skipped: session-persisted model wins for %s",

gateway/run.py

@@ -5556,7 +5556,19 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
         thread_id: Optional[str] = None,
         parent_id: Optional[str] = None,
     ) -> str:
-        """Resolve model for this channel: channel_overrides else global default."""
+        """Resolve model for this channel: channel_overrides else global default.
+
+        Delegates the precedence rule to
+        :func:`hermes_cli.model_switch.resolve_effective_model` (session
+        override > channel override > global default) — the single owner
+        shared with the API server, so the two surfaces cannot diverge
+        again (see 7dd00bb47d).  This call site has no session tier: session
+        /model overrides are applied later by
+        ``_apply_session_model_override`` on the resolved runtime.
+        """
+        from hermes_cli.model_switch import resolve_effective_model
+
+        override = None
         config = getattr(self, "config", None)
         if config:
             override = _get_channel_override(
@@ -5566,9 +5578,11 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
                 thread_id=thread_id,
                 parent_id=parent_id,
             )
-            if override and override.model:
-                return override.model
-        return _resolve_gateway_model(user_config)
+        return resolve_effective_model(
+            None,  # session tier applied downstream (_apply_session_model_override)
+            override,
+            _resolve_gateway_model(user_config),
+        )
 
     def _get_system_prompt_for_channel(
         self,

gateway/slash_commands.py

@@ -1724,7 +1724,7 @@ class GatewaySlashCommandsMixin:
         """
         from gateway.run import _hermes_home, _load_gateway_config
         from hermes_cli.model_switch import (
-            switch_model as _switch_model, parse_model_flags_detailed,
+            switch_model as _switch_model, parse_model_switch_args,
             resolve_persist_behavior,
             list_authenticated_providers,
             list_picker_providers,
@@ -1740,17 +1740,17 @@ class GatewaySlashCommandsMixin:
             )(source)
 
         # Parse --provider, --global, --session, --once, and --refresh flags
-        parsed_flags = parse_model_flags_detailed(raw_args)
-        model_input = parsed_flags.model_input
-        explicit_provider = parsed_flags.explicit_provider
-        is_global_flag = parsed_flags.is_global
-        force_refresh = parsed_flags.force_refresh
-        is_session = parsed_flags.is_session
-        one_turn = parsed_flags.is_once
-        if is_global_flag and one_turn:
-            return "❌ /model --once cannot be combined with --global"
-        if one_turn and not model_input and not explicit_provider:
-            return "❌ /model --once requires a model or provider."
+        # via the shared single-owner parser (hermes_cli.model_switch).
+        request = parse_model_switch_args(raw_args)
+        model_input = request.target
+        explicit_provider = request.explicit_provider
+        is_global_flag = request.is_global
+        force_refresh = request.force_refresh
+        is_session = request.is_session
+        one_turn = request.is_once
+        if request.errors:
+            # Gateway decoration: "❌ " prefix over the canonical error copy.
+            return f"❌ {request.error_messages()[0]}"
         persist_global = resolve_persist_behavior(
             is_global_flag,
             is_session,

hermes_cli/model_switch.py

@@ -598,6 +598,167 @@ def resolve_persist_behavior(
     return False
 
 
+# ---------------------------------------------------------------------------
+# Single-owner /model request parsing + effective-model resolution
+# ---------------------------------------------------------------------------
+#
+# Historically each surface (cli.py, gateway/slash_commands.py,
+# tui_gateway/server.py) re-implemented flag parsing + conflict checks, and
+# each resolution surface (gateway/run.py, gateway/platforms/api_server.py)
+# re-implemented the session-override > channel/session > global precedence.
+# Commit 7dd00bb47d had to re-fix the api_server discarding session-persisted
+# models precisely because the precedence rule lived in two places.  The
+# helpers below are the ONE owner; surfaces map error codes to their own
+# user-facing copy but never re-derive the semantics.
+
+# Error codes emitted by parse_model_switch_args().
+MODEL_SWITCH_ERR_ONCE_WITH_GLOBAL = "once_with_global"
+MODEL_SWITCH_ERR_ONCE_REQUIRES_TARGET = "once_requires_target"
+
+# Canonical (surface-neutral) error copy.  Surfaces prepend their own
+# decoration ("  ✗ " in the CLI, "❌ " in the gateway) but MUST NOT change
+# the core sentence — it is shared user-visible copy.
+MODEL_SWITCH_ERROR_TEXT = {
+    MODEL_SWITCH_ERR_ONCE_WITH_GLOBAL: "/model --once cannot be combined with --global",
+    MODEL_SWITCH_ERR_ONCE_REQUIRES_TARGET: "/model --once requires a model or provider.",
+}
+
+
+@dataclass(frozen=True)
+class ModelSwitchRequest:
+    """A fully parsed /model command request.
+
+    ``scope`` is the *requested* persistence scope derived purely from the
+    flags: ``"once"`` | ``"session"`` | ``"global"`` | ``"default"`` (no
+    explicit scope flag; the effective decision then belongs to
+    :func:`resolve_persist_behavior`, which also reads config).
+
+    ``errors`` carries error *codes* (see ``MODEL_SWITCH_ERR_*``); surfaces
+    render them via :data:`MODEL_SWITCH_ERROR_TEXT` plus their own prefix.
+    """
+
+    raw: str
+    target: str
+    explicit_provider: str = ""
+    is_global: bool = False
+    is_session: bool = False
+    is_once: bool = False
+    force_refresh: bool = False
+    scope: str = "default"
+    errors: tuple = ()
+
+    # Compat properties so a ModelSwitchRequest can be passed anywhere a
+    # ModelFlagParseResult was accepted (e.g. tui_gateway._apply_model_switch).
+    @property
+    def model_input(self) -> str:
+        return self.target
+
+    @property
+    def flags(self) -> "ModelFlagParseResult":
+        return ModelFlagParseResult(
+            model_input=self.target,
+            explicit_provider=self.explicit_provider,
+            is_global=self.is_global,
+            force_refresh=self.force_refresh,
+            is_session=self.is_session,
+            is_once=self.is_once,
+        )
+
+    def error_messages(self) -> list:
+        """Canonical (undercorated) error strings for this request."""
+        return [MODEL_SWITCH_ERROR_TEXT[code] for code in self.errors]
+
+
+def parse_model_switch_args(raw: str) -> ModelSwitchRequest:
+    """Parse a raw /model argument string into a :class:`ModelSwitchRequest`.
+
+    The ONE parser for every /model surface.  Wraps
+    :func:`parse_model_flags_detailed` (tokenization + Unicode-dash
+    normalization) and layers on the flag-conflict validation that cli.py,
+    gateway/slash_commands.py, and tui_gateway/server.py each used to
+    re-implement:
+
+    * ``--once`` + ``--global``  → ``MODEL_SWITCH_ERR_ONCE_WITH_GLOBAL``
+    * ``--once`` with no model and no ``--provider``
+      → ``MODEL_SWITCH_ERR_ONCE_REQUIRES_TARGET``
+
+    Model targets pass through untouched: bare names (``sonnet``),
+    aggregator slugs (``vendor/model``), and colon forms (``vendor:model``)
+    are all resolved later by :func:`switch_model` (aggregator-aware — bare
+    names resolve WITHIN the current aggregator first).
+    """
+    raw = str(raw or "")
+    parsed = parse_model_flags_detailed(raw)
+
+    errors: list = []
+    if parsed.is_once and parsed.is_global:
+        errors.append(MODEL_SWITCH_ERR_ONCE_WITH_GLOBAL)
+    if parsed.is_once and not parsed.model_input and not parsed.explicit_provider:
+        errors.append(MODEL_SWITCH_ERR_ONCE_REQUIRES_TARGET)
+
+    if parsed.is_once:
+        scope = "once"
+    elif parsed.is_session:
+        scope = "session"
+    elif parsed.is_global:
+        scope = "global"
+    else:
+        scope = "default"
+
+    return ModelSwitchRequest(
+        raw=raw,
+        target=parsed.model_input,
+        explicit_provider=parsed.explicit_provider,
+        is_global=parsed.is_global,
+        is_session=parsed.is_session,
+        is_once=parsed.is_once,
+        force_refresh=parsed.force_refresh,
+        scope=scope,
+        errors=tuple(errors),
+    )
+
+
+def _effective_model_candidate(value: Any) -> str:
+    """Extract a model-name candidate from a str / dict / attr-object."""
+    if value is None:
+        return ""
+    if isinstance(value, str):
+        return value.strip()
+    if isinstance(value, dict):
+        return str(value.get("model") or "").strip()
+    model_attr = getattr(value, "model", None)
+    if model_attr is not None:
+        return str(model_attr or "").strip()
+    return ""
+
+
+def resolve_effective_model(
+    session_overrides: Any = None,
+    channel_config: Any = None,
+    global_config: Any = "",
+) -> str:
+    """Resolve the effective model: session override > channel > global.
+
+    The single owner of the precedence rule that gateway/run.py
+    (``_resolve_model_for_channel`` / ``_apply_session_model_override``) and
+    gateway/platforms/api_server.py (``_create_agent``'s session-override /
+    session-persisted-model branches) each encoded independently — the
+    divergence commit 7dd00bb47d had to close.  A user-issued ``/model``
+    (session override) always wins over per-channel/session-persisted
+    configuration, which wins over the global default.
+
+    Each argument may be a plain model string, a dict with a ``"model"``
+    key (a gateway ``_session_model_overrides`` entry), or an object with a
+    ``.model`` attribute (a ``ChannelOverride``).  Empty/None entries fall
+    through to the next tier.
+    """
+    for tier in (session_overrides, channel_config, global_config):
+        candidate = _effective_model_candidate(tier)
+        if candidate:
+            return candidate
+    return ""
+
+
 # ---------------------------------------------------------------------------
 # Alias resolution
 # ---------------------------------------------------------------------------

tests/hermes_cli/test_model_switch_parsing.py

@@ -0,0 +1,232 @@
+"""Single-owner /model parsing + effective-model resolution tests.
+
+Covers the consolidation of the 7 historical parsing/resolution variants
+into hermes_cli.model_switch (parse_model_switch_args +
+resolve_effective_model), including the 7dd00bb47d regression class
+(api_server discarding session-persisted models) as a permanent parity
+test against the pre-consolidation logic captured from origin/main.
+
+Real imports throughout (AGENTS.md: no mocks for resolution chains).
+"""
+
+import pytest
+
+from hermes_cli.model_switch import (
+    MODEL_SWITCH_ERR_ONCE_REQUIRES_TARGET,
+    MODEL_SWITCH_ERR_ONCE_WITH_GLOBAL,
+    MODEL_SWITCH_ERROR_TEXT,
+    ModelSwitchRequest,
+    parse_model_flags_detailed,
+    parse_model_switch_args,
+    resolve_effective_model,
+)
+
+
+# ---------------------------------------------------------------------------
+# parse_model_switch_args — the ONE parser
+# ---------------------------------------------------------------------------
+
+def test_bare_name_on_aggregator_passes_through():
+    # Bare names are NOT provider-qualified by the parser: aggregator-aware
+    # resolution (bare names resolve WITHIN the aggregator first, via
+    # switch_model's catalog search) happens downstream. The parser must not
+    # hardcode a provider.
+    req = parse_model_switch_args("sonnet")
+    assert req.target == "sonnet"
+    assert req.explicit_provider == ""
+    assert req.scope == "default"
+    assert req.errors == ()
+
+
+def test_provider_colon_model_target_preserved():
+    # vendor:model colon forms are preserved verbatim — switch_model converts
+    # them to aggregator slugs only when the current provider is an aggregator.
+    req = parse_model_switch_args("anthropic:claude-sonnet-4-5")
+    assert req.target == "anthropic:claude-sonnet-4-5"
+    assert req.explicit_provider == ""
+
+
+def test_provider_flag_and_scopes():
+    req = parse_model_switch_args("sonnet --provider anthropic --global")
+    assert req.target == "sonnet"
+    assert req.explicit_provider == "anthropic"
+    assert req.is_global is True
+    assert req.scope == "global"
+    assert req.errors == ()
+
+    assert parse_model_switch_args("sonnet --session").scope == "session"
+    assert parse_model_switch_args("sonnet --once").scope == "once"
+    assert parse_model_switch_args("--refresh").force_refresh is True
+
+
+def test_once_with_global_conflict():
+    req = parse_model_switch_args("sonnet --once --global")
+    assert MODEL_SWITCH_ERR_ONCE_WITH_GLOBAL in req.errors
+    assert (
+        MODEL_SWITCH_ERROR_TEXT[MODEL_SWITCH_ERR_ONCE_WITH_GLOBAL]
+        == "/model --once cannot be combined with --global"
+    )
+    assert "/model --once cannot be combined with --global" in req.error_messages()
+
+
+def test_once_without_target_error():
+    req = parse_model_switch_args("--once")
+    assert MODEL_SWITCH_ERR_ONCE_REQUIRES_TARGET in req.errors
+    assert (
+        MODEL_SWITCH_ERROR_TEXT[MODEL_SWITCH_ERR_ONCE_REQUIRES_TARGET]
+        == "/model --once requires a model or provider."
+    )
+    # --once with just a provider is valid.
+    assert parse_model_switch_args("--once --provider anthropic").errors == ()
+
+
+def test_unicode_dash_normalization_matches_legacy_parser():
+    # Telegram/iOS auto-converts "--" to em dashes; the shared parser must
+    # keep the legacy normalization.
+    req = parse_model_switch_args("sonnet \u2014global")
+    assert req.is_global is True
+    assert req.target == "sonnet"
+
+
+def test_request_is_compatible_with_flag_result_consumers():
+    # tui_gateway._apply_model_switch duck-types on .model_input; the request
+    # object must satisfy the same consumer surface.
+    req = parse_model_switch_args("sonnet --provider anthropic --once")
+    assert req.model_input == "sonnet"
+    legacy = parse_model_flags_detailed("sonnet --provider anthropic --once")
+    assert req.flags == legacy
+
+
+# ---------------------------------------------------------------------------
+# resolve_effective_model — session > channel/session-persisted > global
+# ---------------------------------------------------------------------------
+
+class _ChannelOverride:
+    def __init__(self, model):
+        self.model = model
+
+
+def test_session_override_beats_channel_and_global():
+    assert (
+        resolve_effective_model({"model": "session-model"}, _ChannelOverride("chan-model"), "global-model")
+        == "session-model"
+    )
+
+
+def test_channel_beats_global_when_no_session():
+    assert (
+        resolve_effective_model(None, _ChannelOverride("chan-model"), "global-model")
+        == "chan-model"
+    )
+    assert resolve_effective_model({}, _ChannelOverride(""), "global-model") == "global-model"
+
+
+def test_global_fallback_and_empty():
+    assert resolve_effective_model(None, None, "global-model") == "global-model"
+    assert resolve_effective_model(None, None, "") == ""
+
+
+# ---------------------------------------------------------------------------
+# Parity: run.py-style channel resolution (old logic from origin/main)
+# ---------------------------------------------------------------------------
+
+def _old_run_py_resolve(override, global_model):
+    # Captured from origin/main gateway/run.py:_resolve_model_for_channel:
+    #     if override and override.model:
+    #         return override.model
+    #     return _resolve_gateway_model(user_config)
+    if override and override.model:
+        return override.model
+    return global_model
+
+
+@pytest.mark.parametrize(
+    "override,global_model",
+    [
+        (None, "global-model"),
+        (_ChannelOverride("chan-model"), "global-model"),
+        (_ChannelOverride(""), "global-model"),
+        (_ChannelOverride("chan-model"), ""),
+        (None, ""),
+    ],
+)
+def test_run_py_channel_resolution_parity(override, global_model):
+    assert resolve_effective_model(None, override, global_model) == _old_run_py_resolve(
+        override, global_model
+    )
+
+
+# ---------------------------------------------------------------------------
+# Parity: api_server-style resolution (old logic from origin/main)
+# ---------------------------------------------------------------------------
+
+def _clean(value):
+    # api_server._clean_request_string equivalent for the parity harness.
+    if value is None:
+        return None
+    text = str(value).strip()
+    return text or None
+
+
+def _old_api_server_resolve(session_override, session_row_model, global_model):
+    # Captured from origin/main gateway/platforms/api_server.py:_create_agent
+    # (post-7dd00bb47d — session /model override > session-persisted model >
+    # global default):
+    model = global_model
+    if session_override:
+        model = (_clean(session_override.get("model")) or model)
+    elif _clean(session_row_model):
+        model = _clean(session_row_model)
+    return model
+
+
+@pytest.mark.parametrize(
+    "session_override,session_row_model,global_model",
+    [
+        (None, None, "global-model"),
+        (None, "session-persisted", "global-model"),  # the 7dd00bb47d regression
+        ({"model": "override-model"}, "session-persisted", "global-model"),
+        ({"model": ""}, "session-persisted", "global-model"),
+        ({"model": "override-model"}, None, "global-model"),
+        (None, "  ", "global-model"),
+    ],
+)
+def test_api_server_resolution_parity(session_override, session_row_model, global_model):
+    # New logic mirrors the migrated api_server code path exactly:
+    if session_override:
+        new = resolve_effective_model(session_override, None, global_model)
+    elif _clean(session_row_model):
+        new = resolve_effective_model(None, session_row_model, global_model)
+    else:
+        new = global_model
+    assert new == _old_api_server_resolve(session_override, session_row_model, global_model)
+
+
+def test_session_persisted_model_honored_by_both_surfaces():
+    """Permanent 7dd00bb47d regression test.
+
+    A session-persisted model (POST /api/sessions {"model": ...} on the API
+    server; per-channel/session config on the native gateway) must be honored
+    over the global default by BOTH resolution styles — the divergence class
+    this consolidation kills.
+    """
+    session_persisted = "vendor/session-pinned-model"
+    global_model = "vendor/global-default"
+
+    # run.py-style: channel/session tier vs global.
+    assert (
+        resolve_effective_model(None, session_persisted, global_model)
+        == session_persisted
+    )
+    # api_server-style: same shared owner, same answer.
+    assert (
+        resolve_effective_model(None, {"model": session_persisted}, global_model)
+        == session_persisted
+    )
+    # And an explicit session /model override still beats both.
+    assert (
+        resolve_effective_model(
+            {"model": "vendor/live-override"}, session_persisted, global_model
+        )
+        == "vendor/live-override"
+    )

tui_gateway/server.py

@@ -3991,14 +3991,16 @@ def _apply_model_switch(
     persist_override: bool | None = None,
 ) -> dict:
     from hermes_cli.model_switch import (
-        parse_model_flags_detailed,
+        parse_model_switch_args,
         resolve_persist_behavior,
         switch_model,
+        MODEL_SWITCH_ERR_ONCE_WITH_GLOBAL,
+        MODEL_SWITCH_ERROR_TEXT,
     )
     from hermes_cli.runtime_provider import resolve_runtime_provider
 
     if parsed_flags is None:
-        parsed_flags = parse_model_flags_detailed(raw_input)
+        parsed_flags = parse_model_switch_args(raw_input)
     if hasattr(parsed_flags, "model_input"):
         model_input = parsed_flags.model_input
         explicit_provider = parsed_flags.explicit_provider
@@ -4008,8 +4010,11 @@ def _apply_model_switch(
     else:
         model_input, explicit_provider, is_global_flag, _force_refresh, is_session = parsed_flags
         one_turn = False
+    # Conflict validation delegates to the shared single-owner parser; the
+    # TUI surfaces it as a raised ValueError (its historical behavior)
+    # using the canonical error copy.
     if is_global_flag and one_turn:
-        raise ValueError("/model --once cannot be combined with --global")
+        raise ValueError(MODEL_SWITCH_ERROR_TEXT[MODEL_SWITCH_ERR_ONCE_WITH_GLOBAL])
     persist_global = (
         persist_override
         if persist_override is not None
@@ -13656,9 +13661,9 @@ def _(rid, params: dict) -> dict:
                         4009,
                         "session busy — /interrupt the current turn before switching models",
                     )
-                from hermes_cli.model_switch import parse_model_flags_detailed
+                from hermes_cli.model_switch import parse_model_switch_args
 
-                parsed_flags = parse_model_flags_detailed(value)
+                parsed_flags = parse_model_switch_args(value)
                 explicit_provider = parsed_flags.explicit_provider
                 if session.get("agent") is None and not explicit_provider.strip():
                     session_id = params.get("session_id", "")