Merge pull request #735 from NousResearch/hermes/hermes-f8d56335
fix: allow non-codex-suffixed models (e.g. gpt-5.4) with OpenAI Codex provider
This commit is contained in:
81
cli.py
81
cli.py
@@ -1159,14 +1159,18 @@ class HermesCLI:
|
||||
self._app.invalidate()
|
||||
|
||||
def _normalize_model_for_provider(self, resolved_provider: str) -> bool:
|
||||
"""Normalize obviously incompatible model/provider pairings.
|
||||
"""Strip provider prefixes and swap the default model for Codex.
|
||||
|
||||
When the resolved provider is ``openai-codex``, the Codex Responses API
|
||||
only accepts Codex-compatible model slugs (e.g. ``gpt-5.3-codex``).
|
||||
If the active model is incompatible (e.g. the OpenRouter default
|
||||
``anthropic/claude-opus-4.6``), swap it for the best available Codex
|
||||
model. Also strips provider prefixes the API does not accept
|
||||
(``openai/gpt-5.3-codex`` → ``gpt-5.3-codex``).
|
||||
When the resolved provider is ``openai-codex``:
|
||||
|
||||
1. Strip any ``provider/`` prefix (the Codex Responses API only
|
||||
accepts bare model slugs like ``gpt-5.4``, not ``openai/gpt-5.4``).
|
||||
2. If the active model is still the *untouched default* (user never
|
||||
explicitly chose a model), replace it with a Codex-compatible
|
||||
default so the first session doesn't immediately error.
|
||||
|
||||
If the user explicitly chose a model — *any* model — we trust them
|
||||
and let the API be the judge. No allowlists, no slug checks.
|
||||
|
||||
Returns True when the active model was changed.
|
||||
"""
|
||||
@@ -1174,46 +1178,39 @@ class HermesCLI:
|
||||
return False
|
||||
|
||||
current_model = (self.model or "").strip()
|
||||
current_slug = current_model.split("/")[-1] if current_model else ""
|
||||
changed = False
|
||||
|
||||
# Keep explicit Codex models, but strip any provider prefix that the
|
||||
# Codex Responses API does not accept.
|
||||
if current_slug and "codex" in current_slug.lower():
|
||||
if current_slug != current_model:
|
||||
self.model = current_slug
|
||||
if not self._model_is_default:
|
||||
self.console.print(
|
||||
f"[yellow]⚠️ Stripped provider prefix from '{current_model}'; "
|
||||
f"using '{current_slug}' for OpenAI Codex.[/]"
|
||||
)
|
||||
return True
|
||||
return False
|
||||
|
||||
# Model is not Codex-compatible — replace with the best available
|
||||
fallback_model = "gpt-5.3-codex"
|
||||
try:
|
||||
from hermes_cli.codex_models import get_codex_model_ids
|
||||
|
||||
codex_models = get_codex_model_ids(
|
||||
access_token=self.api_key if self.api_key else None,
|
||||
)
|
||||
fallback_model = next(
|
||||
(mid for mid in codex_models if "codex" in mid.lower()),
|
||||
fallback_model,
|
||||
)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
if current_model != fallback_model:
|
||||
# 1. Strip provider prefix ("openai/gpt-5.4" → "gpt-5.4")
|
||||
if "/" in current_model:
|
||||
slug = current_model.split("/", 1)[1]
|
||||
if not self._model_is_default:
|
||||
self.console.print(
|
||||
f"[yellow]⚠️ Model '{current_model}' is not supported with "
|
||||
f"OpenAI Codex; switching to '{fallback_model}'.[/]"
|
||||
f"[yellow]⚠️ Stripped provider prefix from '{current_model}'; "
|
||||
f"using '{slug}' for OpenAI Codex.[/]"
|
||||
)
|
||||
self.model = fallback_model
|
||||
return True
|
||||
self.model = slug
|
||||
current_model = slug
|
||||
changed = True
|
||||
|
||||
return False
|
||||
# 2. Replace untouched default with a Codex model
|
||||
if self._model_is_default:
|
||||
fallback_model = "gpt-5.3-codex"
|
||||
try:
|
||||
from hermes_cli.codex_models import get_codex_model_ids
|
||||
|
||||
available = get_codex_model_ids(
|
||||
access_token=self.api_key if self.api_key else None,
|
||||
)
|
||||
if available:
|
||||
fallback_model = available[0]
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
if current_model != fallback_model:
|
||||
self.model = fallback_model
|
||||
changed = True
|
||||
|
||||
return changed
|
||||
|
||||
def _ensure_runtime_credentials(self) -> bool:
|
||||
"""
|
||||
|
||||
@@ -94,8 +94,6 @@ def _read_cache_models(codex_home: Path) -> List[str]:
|
||||
if not isinstance(slug, str) or not slug.strip():
|
||||
continue
|
||||
slug = slug.strip()
|
||||
if "codex" not in slug.lower():
|
||||
continue
|
||||
if item.get("supported_in_api") is False:
|
||||
continue
|
||||
visibility = item.get("visibility")
|
||||
|
||||
@@ -197,10 +197,10 @@ def test_codex_provider_replaces_incompatible_default_model(monkeypatch):
|
||||
assert shell.model == "gpt-5.2-codex"
|
||||
|
||||
|
||||
def test_codex_provider_replaces_incompatible_envvar_model(monkeypatch):
|
||||
"""Exact scenario from #651: LLM_MODEL is set to a non-Codex model and
|
||||
provider resolves to openai-codex. The model must be replaced and a
|
||||
warning printed since the user explicitly chose it."""
|
||||
def test_codex_provider_trusts_explicit_envvar_model(monkeypatch):
|
||||
"""When the user explicitly sets LLM_MODEL, we trust their choice and
|
||||
let the API be the judge — even if it's a non-OpenAI model. Only
|
||||
provider prefixes are stripped; the bare model passes through."""
|
||||
cli = _import_cli()
|
||||
|
||||
monkeypatch.setenv("LLM_MODEL", "claude-opus-4-6")
|
||||
@@ -217,18 +217,14 @@ def test_codex_provider_replaces_incompatible_envvar_model(monkeypatch):
|
||||
|
||||
monkeypatch.setattr("hermes_cli.runtime_provider.resolve_runtime_provider", _runtime_resolve)
|
||||
monkeypatch.setattr("hermes_cli.runtime_provider.format_runtime_provider_error", lambda exc: str(exc))
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.codex_models.get_codex_model_ids",
|
||||
lambda access_token=None: ["gpt-5.2-codex", "gpt-5.1-codex-mini"],
|
||||
)
|
||||
|
||||
shell = cli.HermesCLI(compact=True, max_turns=1)
|
||||
|
||||
assert shell._model_is_default is False
|
||||
assert shell._ensure_runtime_credentials() is True
|
||||
assert shell.provider == "openai-codex"
|
||||
assert "claude" not in shell.model
|
||||
assert shell.model == "gpt-5.2-codex"
|
||||
# User explicitly chose this model — it passes through untouched
|
||||
assert shell.model == "claude-opus-4-6"
|
||||
|
||||
|
||||
def test_codex_provider_preserves_explicit_codex_model(monkeypatch):
|
||||
|
||||
@@ -1,4 +1,9 @@
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
from unittest.mock import patch
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||
|
||||
from hermes_cli.codex_models import DEFAULT_CODEX_MODELS, get_codex_model_ids
|
||||
|
||||
@@ -13,7 +18,7 @@ def test_get_codex_model_ids_prioritizes_default_and_cache(tmp_path, monkeypatch
|
||||
"models": [
|
||||
{"slug": "gpt-5.3-codex", "priority": 20, "supported_in_api": True},
|
||||
{"slug": "gpt-5.1-codex", "priority": 5, "supported_in_api": True},
|
||||
{"slug": "gpt-4o", "priority": 1, "supported_in_api": True},
|
||||
{"slug": "gpt-5.4", "priority": 1, "supported_in_api": True},
|
||||
{"slug": "gpt-5-hidden-codex", "priority": 2, "visibility": "hidden"},
|
||||
]
|
||||
}
|
||||
@@ -26,7 +31,8 @@ def test_get_codex_model_ids_prioritizes_default_and_cache(tmp_path, monkeypatch
|
||||
assert models[0] == "gpt-5.2-codex"
|
||||
assert "gpt-5.1-codex" in models
|
||||
assert "gpt-5.3-codex" in models
|
||||
assert "gpt-4o" not in models
|
||||
# Non-codex-suffixed models are included when the cache says they're available
|
||||
assert "gpt-5.4" in models
|
||||
assert "gpt-5-hidden-codex" not in models
|
||||
|
||||
|
||||
@@ -46,3 +52,144 @@ def test_get_codex_model_ids_falls_back_to_curated_defaults(tmp_path, monkeypatc
|
||||
models = get_codex_model_ids()
|
||||
|
||||
assert models[: len(DEFAULT_CODEX_MODELS)] == DEFAULT_CODEX_MODELS
|
||||
|
||||
|
||||
# ── Tests for _normalize_model_for_provider ──────────────────────────
|
||||
|
||||
|
||||
def _make_cli(model="anthropic/claude-opus-4.6", **kwargs):
|
||||
"""Create a HermesCLI with minimal mocking."""
|
||||
import cli as _cli_mod
|
||||
from cli import HermesCLI
|
||||
|
||||
_clean_config = {
|
||||
"model": {
|
||||
"default": "anthropic/claude-opus-4.6",
|
||||
"base_url": "https://openrouter.ai/api/v1",
|
||||
"provider": "auto",
|
||||
},
|
||||
"display": {"compact": False, "tool_progress": "all", "resume_display": "full"},
|
||||
"agent": {},
|
||||
"terminal": {"env_type": "local"},
|
||||
}
|
||||
clean_env = {"LLM_MODEL": "", "HERMES_MAX_ITERATIONS": ""}
|
||||
with (
|
||||
patch("cli.get_tool_definitions", return_value=[]),
|
||||
patch.dict("os.environ", clean_env, clear=False),
|
||||
patch.dict(_cli_mod.__dict__, {"CLI_CONFIG": _clean_config}),
|
||||
):
|
||||
cli = HermesCLI(model=model, **kwargs)
|
||||
return cli
|
||||
|
||||
|
||||
class TestNormalizeModelForProvider:
|
||||
"""_normalize_model_for_provider() trusts user-selected models.
|
||||
|
||||
Only two things happen:
|
||||
1. Provider prefixes are stripped (API needs bare slugs)
|
||||
2. The *untouched default* model is swapped for a Codex model
|
||||
Everything else passes through — the API is the judge.
|
||||
"""
|
||||
|
||||
def test_non_codex_provider_is_noop(self):
|
||||
cli = _make_cli(model="gpt-5.4")
|
||||
changed = cli._normalize_model_for_provider("openrouter")
|
||||
assert changed is False
|
||||
assert cli.model == "gpt-5.4"
|
||||
|
||||
def test_bare_codex_model_passes_through(self):
|
||||
cli = _make_cli(model="gpt-5.3-codex")
|
||||
changed = cli._normalize_model_for_provider("openai-codex")
|
||||
assert changed is False
|
||||
assert cli.model == "gpt-5.3-codex"
|
||||
|
||||
def test_bare_non_codex_model_passes_through(self):
|
||||
"""gpt-5.4 (no 'codex' suffix) passes through — user chose it."""
|
||||
cli = _make_cli(model="gpt-5.4")
|
||||
changed = cli._normalize_model_for_provider("openai-codex")
|
||||
assert changed is False
|
||||
assert cli.model == "gpt-5.4"
|
||||
|
||||
def test_any_bare_model_trusted(self):
|
||||
"""Even a non-OpenAI bare model passes through — user explicitly set it."""
|
||||
cli = _make_cli(model="claude-opus-4-6")
|
||||
changed = cli._normalize_model_for_provider("openai-codex")
|
||||
# User explicitly chose this model — we trust them, API will error if wrong
|
||||
assert changed is False
|
||||
assert cli.model == "claude-opus-4-6"
|
||||
|
||||
def test_provider_prefix_stripped(self):
|
||||
"""openai/gpt-5.4 → gpt-5.4 (strip prefix, keep model)."""
|
||||
cli = _make_cli(model="openai/gpt-5.4")
|
||||
changed = cli._normalize_model_for_provider("openai-codex")
|
||||
assert changed is True
|
||||
assert cli.model == "gpt-5.4"
|
||||
|
||||
def test_any_provider_prefix_stripped(self):
|
||||
"""anthropic/claude-opus-4.6 → claude-opus-4.6 (strip prefix only).
|
||||
User explicitly chose this — let the API decide if it works."""
|
||||
cli = _make_cli(model="anthropic/claude-opus-4.6")
|
||||
changed = cli._normalize_model_for_provider("openai-codex")
|
||||
assert changed is True
|
||||
assert cli.model == "claude-opus-4.6"
|
||||
|
||||
def test_default_model_replaced(self):
|
||||
"""The untouched default (anthropic/claude-opus-4.6) gets swapped."""
|
||||
import cli as _cli_mod
|
||||
_clean_config = {
|
||||
"model": {
|
||||
"default": "anthropic/claude-opus-4.6",
|
||||
"base_url": "https://openrouter.ai/api/v1",
|
||||
"provider": "auto",
|
||||
},
|
||||
"display": {"compact": False, "tool_progress": "all", "resume_display": "full"},
|
||||
"agent": {},
|
||||
"terminal": {"env_type": "local"},
|
||||
}
|
||||
# Don't pass model= so _model_is_default is True
|
||||
with (
|
||||
patch("cli.get_tool_definitions", return_value=[]),
|
||||
patch.dict("os.environ", {"LLM_MODEL": "", "HERMES_MAX_ITERATIONS": ""}, clear=False),
|
||||
patch.dict(_cli_mod.__dict__, {"CLI_CONFIG": _clean_config}),
|
||||
):
|
||||
from cli import HermesCLI
|
||||
cli = HermesCLI()
|
||||
|
||||
assert cli._model_is_default is True
|
||||
with patch(
|
||||
"hermes_cli.codex_models.get_codex_model_ids",
|
||||
return_value=["gpt-5.3-codex", "gpt-5.4"],
|
||||
):
|
||||
changed = cli._normalize_model_for_provider("openai-codex")
|
||||
assert changed is True
|
||||
# Uses first from available list
|
||||
assert cli.model == "gpt-5.3-codex"
|
||||
|
||||
def test_default_fallback_when_api_fails(self):
|
||||
"""Default model falls back to gpt-5.3-codex when API unreachable."""
|
||||
import cli as _cli_mod
|
||||
_clean_config = {
|
||||
"model": {
|
||||
"default": "anthropic/claude-opus-4.6",
|
||||
"base_url": "https://openrouter.ai/api/v1",
|
||||
"provider": "auto",
|
||||
},
|
||||
"display": {"compact": False, "tool_progress": "all", "resume_display": "full"},
|
||||
"agent": {},
|
||||
"terminal": {"env_type": "local"},
|
||||
}
|
||||
with (
|
||||
patch("cli.get_tool_definitions", return_value=[]),
|
||||
patch.dict("os.environ", {"LLM_MODEL": "", "HERMES_MAX_ITERATIONS": ""}, clear=False),
|
||||
patch.dict(_cli_mod.__dict__, {"CLI_CONFIG": _clean_config}),
|
||||
):
|
||||
from cli import HermesCLI
|
||||
cli = HermesCLI()
|
||||
|
||||
with patch(
|
||||
"hermes_cli.codex_models.get_codex_model_ids",
|
||||
side_effect=Exception("offline"),
|
||||
):
|
||||
changed = cli._normalize_model_for_provider("openai-codex")
|
||||
assert changed is True
|
||||
assert cli.model == "gpt-5.3-codex"
|
||||
|
||||
Reference in New Issue
Block a user