diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c6f75e8..218ea61b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## [Unreleased] +- Fixed custom provider names such as `Local (127.0.0.1:15721)` producing colon-bearing name slugs that corrupted `@custom:...:model` parsing; name-derived slugs now strip slug-hostile punctuation consistently across model routing, provider cards, key detection, and key removal while preserving endpoint-derived `custom::` slugs (#2047). + ## [v0.51.44] — 2026-05-11 — Release T (5-PR contributor batch — security + worktree sessions + LM Studio + onboarding docs + transcript dedup, plus comprehensive test-suite network isolation) ### Added diff --git a/api/config.py b/api/config.py index 0c241ce5..d44c1039 100644 --- a/api/config.py +++ b/api/config.py @@ -15,6 +15,7 @@ import json import logging import os import queue +import re import sys import threading import time @@ -747,7 +748,14 @@ def _custom_provider_slug_from_name(name: object) -> str: return "" if raw.startswith("custom:"): return raw - return "custom:" + raw.replace(" ", "-") + # Keep name-derived custom provider slugs out of the @provider:model colon + # grammar. Endpoint-derived slugs may still be custom::, but a + # friendly name like "Local (127.0.0.1:15721)" should not preserve ':'. + slug = re.sub(r"[^a-z0-9._-]+", "-", raw).strip("-") + slug = re.sub(r"-{2,}", "-", slug) + if not slug: + return "" + return "custom:" + slug def _custom_provider_entries(config_obj: dict | None = None) -> list[dict]: @@ -1592,7 +1600,7 @@ def resolve_model_provider(model_id: str) -> tuple: if isinstance(key, str) and key.strip() ) if entry_name and model_id in entry_model_ids: - provider_hint = 'custom:' + entry_name.lower().replace(' ', '-') + provider_hint = _custom_provider_slug_from_name(entry_name) return model_id, provider_hint, entry_base_url or None # @provider:model format — explicit provider hint from the dropdown. @@ -2895,7 +2903,7 @@ def get_available_models() -> dict: continue entry_name = str(entry.get("name") or "").strip() if entry_name: - return "custom:" + entry_name.lower().replace(" ", "-") + return _custom_provider_slug_from_name(entry_name) return "custom" return "" diff --git a/api/providers.py b/api/providers.py index a1291c20..7ea23919 100644 --- a/api/providers.py +++ b/api/providers.py @@ -24,6 +24,7 @@ from typing import Any from api.config import ( _PROVIDER_DISPLAY, _PROVIDER_MODELS, + _custom_provider_slug_from_name, _get_label_for_model, _models_from_live_provider_ids, _read_live_provider_model_ids, @@ -36,6 +37,19 @@ from api.config import ( logger = logging.getLogger(__name__) + +def _custom_provider_name_matches(provider_id: str, name: object) -> bool: + """Return True when *provider_id* refers to a named custom provider.""" + pid = str(provider_id or "").strip().lower() + raw_name = str(name or "").strip().lower() + if not pid or not raw_name: + return False + slug = _custom_provider_slug_from_name(raw_name) + candidates = {raw_name, f"custom:{raw_name}"} + if slug: + candidates.add(slug) + return pid in candidates + _OPENROUTER_KEY_URL = "https://openrouter.ai/api/v1/key" _PROVIDER_QUOTA_TIMEOUT_SECONDS = 3.0 _ACCOUNT_USAGE_SUBPROCESS_TIMEOUT_SECONDS = 35.0 @@ -395,8 +409,7 @@ def _provider_has_key(provider_id: str) -> bool: if isinstance(custom_providers, list): for cp in custom_providers: if isinstance(cp, dict): - cp_name = (cp.get("name") or "").strip().lower().replace(" ", "-") - if f"custom:{cp_name}" == provider_id or cp.get("name", "").strip().lower() == provider_id: + if _custom_provider_name_matches(provider_id, cp.get("name")): if str(cp.get("api_key") or "").strip(): return True return False @@ -440,8 +453,7 @@ def _get_provider_api_key(provider_id: str) -> str | None: for cp in custom_providers: if not isinstance(cp, dict): continue - cp_name = str(cp.get("name") or "").strip().lower().replace(" ", "-") - if f"custom:{cp_name}" == provider_id or str(cp.get("name", "")).strip().lower() == provider_id: + if _custom_provider_name_matches(provider_id, cp.get("name")): cp_key = str(cp.get("api_key") or "").strip() if cp_key.startswith("${") and cp_key.endswith("}"): return os.getenv(cp_key[2:-1], "").strip() or None @@ -1033,7 +1045,9 @@ def get_providers() -> dict[str, Any]: if not isinstance(cp, dict) or not cp.get("name"): continue cp_name = str(cp["name"]).strip() - cp_id = f"custom:{cp_name}" + cp_id = _custom_provider_slug_from_name(cp_name) + if not cp_id: + continue # Collect models from `models` list or `model` single cp_models = [] if isinstance(cp.get("models"), list): @@ -1206,8 +1220,7 @@ def _clean_provider_key_from_config(provider_id: str) -> None: if isinstance(custom_providers, list): for cp in custom_providers: if isinstance(cp, dict): - cp_name = (cp.get("name") or "").strip().lower().replace(" ", "-") - if f"custom:{cp_name}" == provider_id or cp.get("name", "").strip().lower() == provider_id: + if _custom_provider_name_matches(provider_id, cp.get("name")): if cp.get("api_key"): del cp["api_key"] changed = True diff --git a/tests/test_custom_providers_in_panel.py b/tests/test_custom_providers_in_panel.py index 27e010fd..1253a068 100644 --- a/tests/test_custom_providers_in_panel.py +++ b/tests/test_custom_providers_in_panel.py @@ -202,6 +202,38 @@ class TestCustomProvidersInGetProviders: finally: self._restore_cfg(old_cfg, old_mtime) + def test_custom_provider_parenthesized_port_uses_safe_provider_id(self, monkeypatch, tmp_path): + """Local setup names with ports must expose the same safe id used by routing.""" + _install_fake_hermes_cli(monkeypatch) + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path) + monkeypatch.setenv("LOCAL_PORT_API_KEY", "sk-local-port-test-12345678") + + old_cfg, old_mtime = self._setup_cfg([ + { + "name": "Local (127.0.0.1:15721)", + "base_url": "http://127.0.0.1:15721/v1", + "api_key": "${LOCAL_PORT_API_KEY}", + "model": "deepseek-v4-flash", + }, + ]) + + from api.providers import _get_provider_api_key, _provider_has_key, get_providers + try: + provider_id = "custom:local-127.0.0.1-15721" + result = get_providers() + provider_ids = {p["id"] for p in result["providers"]} + assert provider_id in provider_ids + assert "custom:Local (127.0.0.1:15721)" not in provider_ids + assert "custom:local-(127.0.0.1:15721)" not in provider_ids + + local = [p for p in result["providers"] if p["id"] == provider_id][0] + assert local["display_name"] == "Local (127.0.0.1:15721)" + assert local["has_key"] is True + assert _provider_has_key(provider_id) is True + assert _get_provider_api_key(provider_id) == "sk-local-port-test-12345678" + finally: + self._restore_cfg(old_cfg, old_mtime) + def test_custom_provider_no_name_skipped(self, monkeypatch, tmp_path): """Malformed custom provider without name should be silently skipped.""" _install_fake_hermes_cli(monkeypatch) diff --git a/tests/test_model_resolver.py b/tests/test_model_resolver.py index 1bc45831..0b616dfe 100644 --- a/tests/test_model_resolver.py +++ b/tests/test_model_resolver.py @@ -179,6 +179,35 @@ def test_custom_provider_models_dict_routes_to_named_custom_provider(): assert base_url == 'http://127.0.0.1:8080/v1' +# ── Issue #2047: parenthesized local provider names with ports ──────────── + +def test_custom_provider_name_with_parenthesized_port_uses_safe_slug(): + """Setup-generated names like 'Local (host:port)' must not leak ':' into slugs.""" + model, provider, base_url = _resolve_with_config( + 'deepseek-v4-flash', + provider='custom', + custom_providers=[{ + 'name': 'Local (127.0.0.1:15721)', + 'base_url': 'http://127.0.0.1:15721/v1', + 'model': 'deepseek-v4-flash', + }], + ) + assert model == 'deepseek-v4-flash' + assert provider == 'custom:local-127.0.0.1-15721' + assert base_url == 'http://127.0.0.1:15721/v1' + + +def test_safe_custom_provider_hint_keeps_model_after_port_slug(): + """The safe slug emitted by the picker must parse back without corrupting the model.""" + model, provider, base_url = _resolve_with_config( + '@custom:local-127.0.0.1-15721:deepseek-v4-flash', + provider='custom', + ) + assert model == 'deepseek-v4-flash' + assert provider == 'custom:local-127.0.0.1-15721' + assert base_url is None + + # ── Issue #1922: default model shadowed by overlapping custom_providers[] ── def test_default_model_not_shadowed_by_overlapping_custom_provider(): diff --git a/tests/test_provider_management.py b/tests/test_provider_management.py index 93464e96..4c1b5491 100644 --- a/tests/test_provider_management.py +++ b/tests/test_provider_management.py @@ -383,6 +383,36 @@ class TestRemoveProviderKey: assert "api_key" not in active["providers"]["openai"] assert active["model"] == {"provider": "openai"} + def test_clean_custom_provider_key_matches_safe_name_slug(self, monkeypatch, tmp_path): + """Custom-provider key removal must match the canonical safe name slug.""" + import yaml + + import api.config as cfg_mod + import api.providers as providers + + config_path = tmp_path / "config.yaml" + config_path.write_text( + yaml.safe_dump({ + "custom_providers": [{ + "name": "Local (127.0.0.1:15721)", + "base_url": "http://127.0.0.1:15721/v1", + "api_key": "${LOCAL_PORT_API_KEY}", + "model": "deepseek-v4-flash", + }], + }), + encoding="utf-8", + ) + + monkeypatch.setattr(cfg_mod, "_get_config_path", lambda: config_path) + monkeypatch.setattr(providers, "reload_config", lambda: None) + + providers._clean_provider_key_from_config("custom:local-127.0.0.1-15721") + + reloaded = yaml.safe_load(config_path.read_text(encoding="utf-8")) + custom_provider = reloaded["custom_providers"][0] + assert custom_provider["name"] == "Local (127.0.0.1:15721)" + assert "api_key" not in custom_provider + def test_remove_provider_key_calls_set_with_none(self, monkeypatch, tmp_path): """remove_provider_key should delegate to set_provider_key(id, None).""" _install_fake_hermes_cli(monkeypatch)