From 9c16ca8790ede000914e7d617358a3320b1b7799 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 11 Jun 2026 05:07:58 -0700 Subject: [PATCH] fix(dashboard): normalize model assignments + confirm-modal for backup import (#44237) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two beta-reported dashboard bugs: 1. Models page: 'Use as -> Main model' on an analytics card sends entry.provider, which falls back to the model's VENDOR prefix (modelVendor('anthropic/claude-opus-4.6') == 'anthropic') when the session row has no billing_provider. That persisted provider: anthropic + default: anthropic/claude-opus-4.6 — a vendor-prefixed OpenRouter slug on the NATIVE Anthropic provider. New sessions then 400 against api.anthropic.com and the user reads it as 'changing models does nothing'. Unknown vendors (moonshotai, poolside, ...) were worse: a provider that can never resolve credentials. Fix: _normalize_main_model_assignment() at the single write chokepoint — maps non-provider vendor names back to the user's current aggregator (else openrouter), and runs the model through normalize_model_for_provider() so the persisted name matches the target provider's API format. Wired into both /api/model/set and the profile-scoped _write_profile_model. 2. System page: 'Restore from backup' spawns hermes import with stdin=DEVNULL, so the CLI's interactive 'Continue? [y/N]' overwrite prompt hits EOF and auto-aborts whenever a config already exists (always, when the dashboard is running). Fix: ConfirmDialog in the dashboard owns the consent, then the endpoint passes --force so the restore runs non-interactively. Validated live: dashboard on a temp HERMES_HOME, repro'd both failure modes pre-fix (vendor-slug write verified via config.yaml + tui session.create; import 'Aborted.' in action-import.log), then verified post-fix (normalized writes, modal -> --force -> restored marker file). --- hermes_cli/web_server.py | 82 ++++++++++++++++++++- tests/hermes_cli/test_web_server.py | 107 ++++++++++++++++++++++++++++ web/src/lib/api.ts | 4 +- web/src/pages/SystemPage.tsx | 20 +++++- 4 files changed, 209 insertions(+), 4 deletions(-) diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 2f3e3b201..0a35d93e6 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -723,6 +723,73 @@ class ModelAssignment(BaseModel): profile: Optional[str] = None +def _normalize_main_model_assignment(provider: str, model: str) -> tuple[str, str]: + """Normalize a main-slot (provider, model) pair before persisting. + + The Models page has two assignment paths and only one of them was safe: + + - The "Change" picker sends a real Hermes provider slug — fine. + - The per-card "Use as → Main model" menu sends ``entry.provider`` + from the analytics rows, falling back to the model's VENDOR prefix + (``modelVendor("anthropic/claude-opus-4.6") == "anthropic"``) when + the session row has no ``billing_provider`` (older sessions, NULL + rows). That wrote ``provider: anthropic`` + + ``default: anthropic/claude-opus-4.6`` to config — a vendor-prefixed + OpenRouter slug on the NATIVE Anthropic provider. New sessions then + 400 against api.anthropic.com ("model: anthropic/claude-opus-4.6 not + found") and the user reads it as "changing models does nothing". + + Two repairs, both at this single chokepoint so every caller inherits: + + 1. Vendor-name → Hermes-provider mapping: when the provider string is + not a known Hermes provider/alias (e.g. ``moonshotai``, ``x-ai`` is + known but ``poolside`` isn't) but the model is a vendor-prefixed + aggregator slug, keep the user's CURRENT aggregator if they're on + one, else fall back to openrouter. + 2. Model-format normalization for the resolved provider via + ``normalize_model_for_provider`` (e.g. ``anthropic/claude-opus-4.6`` + on native anthropic → ``claude-opus-4-6``). + """ + from hermes_cli.models import _KNOWN_PROVIDER_NAMES, normalize_provider + from hermes_cli.model_normalize import normalize_model_for_provider + + prov_in = (provider or "").strip() + model_in = (model or "").strip() + canonical = normalize_provider(prov_in) + + if canonical not in _KNOWN_PROVIDER_NAMES and "/" in model_in: + # Vendor prefix posing as a provider (analytics fallback). Resolve + # against the user's current provider when it's an aggregator that + # serves vendor-prefixed slugs; otherwise default to openrouter. + try: + cur_cfg = load_config().get("model", {}) + cur_provider = ( + str(cur_cfg.get("provider", "") or "").strip().lower() + if isinstance(cur_cfg, dict) else "" + ) + except Exception: + cur_provider = "" + from hermes_cli.models import _AGGREGATOR_PROVIDERS + if cur_provider and normalize_provider(cur_provider) in _AGGREGATOR_PROVIDERS: + canonical = normalize_provider(cur_provider) + prov_in = cur_provider + else: + canonical = "openrouter" + prov_in = "openrouter" + + # Custom/user-config providers keep the model verbatim — the registry + # normalizer doesn't know their namespaces. + if canonical in _KNOWN_PROVIDER_NAMES and not canonical.startswith("custom"): + try: + normalized_model = normalize_model_for_provider(model_in, canonical) + if normalized_model: + model_in = normalized_model + except Exception: + _log.debug("model normalization failed for %s/%s", prov_in, model_in, exc_info=True) + + return prov_in, model_in + + def _apply_main_model_assignment( model_cfg: "Any", provider: str, model: str, base_url: str = "" ) -> dict: @@ -2892,6 +2959,7 @@ def _apply_model_assignment_sync( if scope == "main": if not provider or not model: raise HTTPException(status_code=400, detail="provider and model required for main") + provider, model = _normalize_main_model_assignment(provider, model) model_cfg = _apply_main_model_assignment( cfg.get("model", {}), provider, model, base_url ) @@ -7302,6 +7370,14 @@ async def run_backup(body: BackupRequest): class ImportRequest(BaseModel): archive: str + # Pass --force to `hermes import`. The spawned action runs with + # stdin=DEVNULL, so the CLI's interactive "Continue? [y/N]" overwrite + # prompt hits EOF and auto-aborts ("Aborted.", exit 1) whenever the + # target already has a config — which it always does when the dashboard + # itself is running from it. The dashboard shows its own confirm modal + # before calling this endpoint, then sends force=True so the restore + # proceeds non-interactively. + force: bool = False @app.post("/api/ops/import") @@ -7311,8 +7387,11 @@ async def run_import(body: ImportRequest): raise HTTPException(status_code=400, detail="archive path is required") if not os.path.isfile(archive): raise HTTPException(status_code=404, detail=f"Archive not found: {archive}") + args = ["import", archive] + if body.force: + args.append("--force") try: - proc = _spawn_hermes_action(["import", archive], "import") + proc = _spawn_hermes_action(args, "import") except Exception as exc: _log.exception("Failed to spawn import") raise HTTPException(status_code=500, detail=f"Failed to run import: {exc}") @@ -8106,6 +8185,7 @@ def _write_profile_model(profile_dir: Path, provider: str, model: str) -> None: token = set_hermes_home_override(str(profile_dir)) try: + provider, model = _normalize_main_model_assignment(provider, model) cfg = load_config() cfg["model"] = _apply_main_model_assignment(cfg.get("model", {}), provider, model) save_config(cfg) diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index 3aeca71c7..dc92f7cef 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -1104,6 +1104,113 @@ class TestWebServerEndpoints: assert confirmed.status_code == 200 assert confirmed.json()["ok"] is True + def test_model_set_normalizes_vendor_slug_for_native_provider(self, monkeypatch): + """'Use as → Main' with an OpenRouter slug + native provider must not + persist the vendor-prefixed slug verbatim (it 400s against the native + API and reads as "changing models does nothing").""" + monkeypatch.setattr( + "hermes_cli.model_cost_guard.expensive_model_warning", + lambda *_args, **_kwargs: None, + ) + resp = self.client.post( + "/api/model/set", + json={ + "scope": "main", + "provider": "anthropic", + "model": "anthropic/claude-opus-4.6", + }, + ) + assert resp.status_code == 200 + data = resp.json() + assert data["ok"] is True + assert data["provider"] == "anthropic" + # Vendor prefix stripped + dots→hyphens for the native Anthropic API. + assert data["model"] == "claude-opus-4-6" + + from hermes_cli.config import load_config + cfg = load_config() + assert cfg["model"]["provider"] == "anthropic" + assert cfg["model"]["default"] == "claude-opus-4-6" + + def test_model_set_maps_unknown_vendor_to_aggregator(self, monkeypatch): + """A bare vendor name from analytics rows (no billing_provider) is not + a Hermes provider — keep the user's aggregator instead of writing a + provider that can never resolve credentials.""" + monkeypatch.setattr( + "hermes_cli.model_cost_guard.expensive_model_warning", + lambda *_args, **_kwargs: None, + ) + from hermes_cli.config import load_config, save_config + cfg = load_config() + cfg["model"] = {"provider": "openrouter", "default": "openai/gpt-5.5"} + save_config(cfg) + + resp = self.client.post( + "/api/model/set", + json={ + "scope": "main", + "provider": "moonshotai", # vendor prefix, not a provider + "model": "moonshotai/kimi-k2.6", + }, + ) + assert resp.status_code == 200 + data = resp.json() + assert data["ok"] is True + assert data["provider"] == "openrouter" + assert data["model"] == "moonshotai/kimi-k2.6" + + def test_model_set_keeps_aggregator_slug_unchanged(self, monkeypatch): + """The happy path (picker → openrouter + vendor/model) is untouched.""" + monkeypatch.setattr( + "hermes_cli.model_cost_guard.expensive_model_warning", + lambda *_args, **_kwargs: None, + ) + resp = self.client.post( + "/api/model/set", + json={ + "scope": "main", + "provider": "openrouter", + "model": "anthropic/claude-sonnet-4.6", + }, + ) + assert resp.status_code == 200 + data = resp.json() + assert data["ok"] is True + assert data["provider"] == "openrouter" + assert data["model"] == "anthropic/claude-sonnet-4.6" + + def test_ops_import_passes_force_flag(self, tmp_path, monkeypatch): + """force=True must append --force so the spawned non-interactive + `hermes import` doesn't auto-abort at the overwrite prompt.""" + import hermes_cli.web_server as ws + + archive = tmp_path / "backup.zip" + import zipfile + with zipfile.ZipFile(archive, "w") as zf: + zf.writestr("config.yaml", "model: {}\n") + + captured = {} + + def fake_spawn(subcommand, name): + captured["args"] = subcommand + captured["name"] = name + from types import SimpleNamespace as NS + return NS(pid=12345) + + monkeypatch.setattr(ws, "_spawn_hermes_action", fake_spawn) + + resp = self.client.post( + "/api/ops/import", json={"archive": str(archive), "force": True}, + ) + assert resp.status_code == 200 + assert captured["args"] == ["import", str(archive), "--force"] + + resp = self.client.post( + "/api/ops/import", json={"archive": str(archive)}, + ) + assert resp.status_code == 200 + assert captured["args"] == ["import", str(archive)] + def test_reveal_env_var(self, tmp_path): """POST /api/env/reveal should return the real unredacted value.""" diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index a587a0f0c..527a7cfab 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -985,11 +985,11 @@ export const api = { headers: { "Content-Type": "application/json" }, body: JSON.stringify({ output }), }), - runImport: (archive: string) => + runImport: (archive: string, force = false) => fetchJSON("/api/ops/import", { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ archive }), + body: JSON.stringify({ archive, force }), }), getHooks: () => fetchJSON("/api/ops/hooks"), createHook: (body: HookCreate) => diff --git a/web/src/pages/SystemPage.tsx b/web/src/pages/SystemPage.tsx index e78c177d4..6197b456e 100644 --- a/web/src/pages/SystemPage.tsx +++ b/web/src/pages/SystemPage.tsx @@ -170,6 +170,11 @@ export default function SystemPage() { const [addingCred, setAddingCred] = useState(false); const [importPath, setImportPath] = useState(""); + // Restore-from-backup is destructive (overwrites the live config) and the + // spawned `hermes import` runs non-interactively (stdin is /dev/null), so + // its CLI "Continue? [y/N]" prompt would auto-abort. The dashboard owns the + // consent: confirm here, then call the endpoint with force=true. + const [importConfirmOpen, setImportConfirmOpen] = useState(false); // Create-hook modal. const [hookModalOpen, setHookModalOpen] = useState(false); @@ -1181,11 +1186,24 @@ export default function SystemPage() { disabled={!importPath.trim()} onClick={() => { if (!importPath.trim()) return; - runOp(() => api.runImport(importPath.trim()), "Import"); + setImportConfirmOpen(true); }} > Import + setImportConfirmOpen(false)} + onConfirm={() => { + setImportConfirmOpen(false); + runOp(() => api.runImport(importPath.trim(), true), "Import"); + }} + />