Fix custom provider name slugs with ports
This commit is contained in:
1 parent
b766b7f759
commit
f6115b78c6
6 files changed
+124
-10
No files matched your search
@@ -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:<host>:<port>` 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
|
||||
|
||||
+11
-3
@@ -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:<host>:<port>, 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 ""
|
||||
|
||||
+20
-7
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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():
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in new issue
Block a user