fix(providers): honor a custom base_url over models_url in fetch_models
Follow-up to the salvaged CommandCode signature fix: accepting base_url but ignoring it left custom endpoints (user-configured model.base_url / COMMANDCODE_BASE_URL proxies) fetching the public catalog instead of the configured one. Reviewer dansigma flagged this on PR #88851. Class-wide fix, not a CommandCode patch: - providers/base.py: a caller base_url that DIFFERS from the profile's default now wins over models_url. Equality with the default means "not customised" (callers pass base_url unconditionally, defaulting to the profile's own URL) and keeps models_url as the endpoint, preserving the OpenRouter-style split-catalog behavior. - commandcode: _fetch_commandcode_models() takes the endpoint override; both profile overrides forward base_url. - Tests: base-class precedence (custom beats models_url, default does not), CommandCode redirect via live local HTTP server incl. claude-* filter, and default-echo hitting the canonical endpoint. All verified to fail against the pre-fix implementation (sabotage run).
This commit is contained in:
1
contributors/emails/sahabatheri@gmail.com
Normal file
1
contributors/emails/sahabatheri@gmail.com
Normal file
@@ -0,0 +1 @@
|
||||
greyvito
|
||||
@@ -47,14 +47,26 @@ _COMMANDCODE_ANTHROPIC_ENV = ("COMMANDCODE_API_KEY", "COMMANDCODE_ANTHROPIC_BASE
|
||||
|
||||
def _fetch_commandcode_models(
|
||||
timeout: float = 10.0,
|
||||
base_url: str | None = None,
|
||||
) -> list[str] | None:
|
||||
"""Fetch the live model list from the CommandCode /models endpoint.
|
||||
|
||||
Returns a flat list of model IDs or None on failure.
|
||||
No auth required — the public models endpoint is open.
|
||||
|
||||
``base_url`` overrides the endpoint only when the caller passed a URL
|
||||
that differs from the default ``_COMMANDCODE_BASE`` (a user-configured
|
||||
``model.base_url`` / ``COMMANDCODE_BASE_URL`` pointing at a proxy or
|
||||
custom deployment). The picker passes base_url unconditionally, falling
|
||||
back to the profile default — equality means "not customised".
|
||||
"""
|
||||
caller_base = (base_url or "").strip()
|
||||
if caller_base and caller_base.rstrip("/") != _COMMANDCODE_BASE.rstrip("/"):
|
||||
models_url = caller_base.rstrip("/") + "/models"
|
||||
else:
|
||||
models_url = _COMMANDCODE_MODELS_URL
|
||||
try:
|
||||
req = urllib.request.Request(_COMMANDCODE_MODELS_URL)
|
||||
req = urllib.request.Request(models_url)
|
||||
req.add_header("Accept", "application/json")
|
||||
req.add_header("User-Agent", _profile_user_agent())
|
||||
with urllib.request.urlopen(req, timeout=timeout) as resp:
|
||||
@@ -83,7 +95,7 @@ class CommandCodeProfile(ProviderProfile):
|
||||
timeout: float = 8.0,
|
||||
) -> list[str] | None:
|
||||
"""Fetch from the public CommandCode /models endpoint."""
|
||||
return _fetch_commandcode_models(timeout=timeout)
|
||||
return _fetch_commandcode_models(timeout=timeout, base_url=base_url)
|
||||
|
||||
|
||||
commandcode = CommandCodeProfile(
|
||||
@@ -134,7 +146,7 @@ class CommandCodeAnthropicProfile(ProviderProfile):
|
||||
|
||||
Filter to Anthropic-family models only (claude-*).
|
||||
"""
|
||||
all_models = _fetch_commandcode_models(timeout=timeout)
|
||||
all_models = _fetch_commandcode_models(timeout=timeout, base_url=base_url)
|
||||
if all_models is None:
|
||||
return None
|
||||
return [m for m in all_models if m.startswith("claude-")]
|
||||
|
||||
@@ -207,11 +207,17 @@ class ProviderProfile:
|
||||
the provider does not support live model listing.
|
||||
|
||||
Resolution order for the endpoint URL:
|
||||
1. self.models_url (explicit override — use when the models
|
||||
1. base_url + "/models", but ONLY when the caller passed a base_url
|
||||
that differs from this profile's default (a user-configured
|
||||
model.base_url pointing at a proxy/custom endpoint). Callers
|
||||
pass base_url unconditionally — falling back to the profile
|
||||
default when the user configured nothing — so equality with
|
||||
self.base_url means "not customised" and must not shadow
|
||||
models_url.
|
||||
2. self.models_url (explicit override — use when the models
|
||||
endpoint differs from the inference base URL, e.g. OpenRouter
|
||||
exposes a public catalog at /api/v1/models while inference is
|
||||
at /api/v1)
|
||||
2. base_url (caller override — user-configured model.base_url)
|
||||
3. self.base_url + "/models" (standard OpenAI-compat fallback)
|
||||
|
||||
The default implementation sends Bearer auth when api_key is given
|
||||
@@ -221,12 +227,19 @@ class ProviderProfile:
|
||||
Callers must always fall back to the static _PROVIDER_MODELS list
|
||||
when this returns None.
|
||||
"""
|
||||
effective_base = base_url or self.base_url
|
||||
url = (self.models_url or "").strip()
|
||||
if not url:
|
||||
if not effective_base:
|
||||
return None
|
||||
url = effective_base.rstrip("/") + "/models"
|
||||
caller_base = (base_url or "").strip()
|
||||
effective_base = caller_base or self.base_url
|
||||
custom_base = bool(caller_base) and (
|
||||
caller_base.rstrip("/") != (self.base_url or "").rstrip("/")
|
||||
)
|
||||
if custom_base:
|
||||
url = caller_base.rstrip("/") + "/models"
|
||||
else:
|
||||
url = (self.models_url or "").strip()
|
||||
if not url:
|
||||
if not effective_base:
|
||||
return None
|
||||
url = effective_base.rstrip("/") + "/models"
|
||||
|
||||
import json
|
||||
import urllib.request
|
||||
|
||||
@@ -275,3 +275,93 @@ class TestCommandCodeFetchModelsPickerContract:
|
||||
anth = resolve_provider_full("commandcode-anthropic", {}, [])
|
||||
assert anth is not None and anth.id == "commandcode-anthropic"
|
||||
assert anth.transport == "anthropic_messages"
|
||||
|
||||
|
||||
# ── base_url endpoint override ───────────────────────────────────────────────
|
||||
|
||||
class TestCommandCodeBaseUrlOverride:
|
||||
"""A custom base_url must redirect the catalog fetch; the default must not.
|
||||
|
||||
The picker passes ``base_url`` unconditionally (profile default when the
|
||||
user configured nothing), so only a value differing from the default
|
||||
``_COMMANDCODE_BASE`` counts as a customised endpoint.
|
||||
"""
|
||||
|
||||
def _serve(self, models):
|
||||
import json
|
||||
from http.server import BaseHTTPRequestHandler, HTTPServer
|
||||
from threading import Thread
|
||||
|
||||
class H(BaseHTTPRequestHandler):
|
||||
def do_GET(self):
|
||||
body = json.dumps({"data": models}).encode()
|
||||
self.send_response(200)
|
||||
self.send_header("Content-Type", "application/json")
|
||||
self.end_headers()
|
||||
self.wfile.write(body)
|
||||
|
||||
def log_message(self, fmt, *args):
|
||||
pass
|
||||
|
||||
server = HTTPServer(("127.0.0.1", 0), H)
|
||||
Thread(target=server.serve_forever, daemon=True).start()
|
||||
return server, server.server_address[1]
|
||||
|
||||
def test_custom_base_url_redirects_fetch(self, commandcode_profile):
|
||||
server, port = self._serve([{"id": "proxied/model-x"}])
|
||||
try:
|
||||
result = commandcode_profile.fetch_models(
|
||||
api_key="k", base_url=f"http://127.0.0.1:{port}"
|
||||
)
|
||||
assert result == ["proxied/model-x"]
|
||||
finally:
|
||||
server.shutdown()
|
||||
|
||||
def test_custom_base_url_redirects_anthropic_fetch(
|
||||
self, commandcode_anthropic_profile
|
||||
):
|
||||
server, port = self._serve(
|
||||
[{"id": "claude-sonnet-4-6"}, {"id": "deepseek/deepseek-v4-pro"}]
|
||||
)
|
||||
try:
|
||||
result = commandcode_anthropic_profile.fetch_models(
|
||||
api_key="k", base_url=f"http://127.0.0.1:{port}"
|
||||
)
|
||||
assert result == ["claude-sonnet-4-6"] # claude-* filter still applies
|
||||
finally:
|
||||
server.shutdown()
|
||||
|
||||
def test_default_base_url_hits_default_endpoint(self, commandcode_profile):
|
||||
"""Echoing the profile default back must NOT count as an override."""
|
||||
import sys
|
||||
from unittest.mock import patch as mock_patch
|
||||
|
||||
# The bundled plugin module is registered at discovery time under
|
||||
# ``plugins.model_providers.commandcode`` — resolve via the profile's
|
||||
# own __module__ so the test doesn't depend on discovery mechanics.
|
||||
cc_mod = sys.modules[type(commandcode_profile).__module__]
|
||||
|
||||
captured = {}
|
||||
|
||||
class _FakeResp:
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, *a):
|
||||
return False
|
||||
|
||||
def read(self):
|
||||
return b'{"data": [{"id": "m1"}]}'
|
||||
|
||||
def fake_urlopen(req, timeout=0):
|
||||
captured["url"] = req.full_url
|
||||
return _FakeResp()
|
||||
|
||||
with mock_patch.object(
|
||||
cc_mod.urllib.request, "urlopen", side_effect=fake_urlopen
|
||||
):
|
||||
result = commandcode_profile.fetch_models(
|
||||
api_key="k", base_url=cc_mod._COMMANDCODE_BASE + "/"
|
||||
)
|
||||
assert result == ["m1"]
|
||||
assert captured["url"] == cc_mod._COMMANDCODE_MODELS_URL
|
||||
|
||||
@@ -58,6 +58,45 @@ class TestFetchModelsBaseUrlOverride:
|
||||
finally:
|
||||
server.shutdown()
|
||||
|
||||
def test_custom_base_url_beats_models_url(self):
|
||||
"""A caller base_url differing from the profile default overrides
|
||||
models_url — a user-configured proxy must win over the profile's
|
||||
hardcoded catalog endpoint (Discord report: CommandCode picker)."""
|
||||
server, port = _start_server([{"id": "proxy-model-b"}])
|
||||
try:
|
||||
profile = ProviderProfile(
|
||||
name="test",
|
||||
base_url="http://127.0.0.1:1",
|
||||
models_url="http://127.0.0.1:1/models", # unreachable
|
||||
)
|
||||
result = profile.fetch_models(
|
||||
api_key="test-key",
|
||||
base_url=f"http://127.0.0.1:{port}",
|
||||
)
|
||||
assert result == ["proxy-model-b"]
|
||||
finally:
|
||||
server.shutdown()
|
||||
|
||||
def test_default_base_url_does_not_shadow_models_url(self):
|
||||
"""Callers pass base_url unconditionally (profile default when the
|
||||
user configured nothing). Equality with self.base_url means "not
|
||||
customised" and must keep models_url as the endpoint."""
|
||||
server, port = _start_server([{"id": "catalog-model"}])
|
||||
try:
|
||||
profile = ProviderProfile(
|
||||
name="test",
|
||||
base_url="http://127.0.0.1:1", # inference URL, unreachable
|
||||
models_url=f"http://127.0.0.1:{port}/models",
|
||||
)
|
||||
# Caller echoes the profile default back — models_url must win.
|
||||
result = profile.fetch_models(
|
||||
api_key="test-key",
|
||||
base_url="http://127.0.0.1:1/", # same default, trailing slash
|
||||
)
|
||||
assert result == ["catalog-model"]
|
||||
finally:
|
||||
server.shutdown()
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user