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"); + }} + />