From 044ce8d71eb39196ca101748abeaacd69fb4c38e Mon Sep 17 00:00:00 2001 From: James Yang Date: Wed, 5 Aug 2026 02:53:04 -0400 Subject: [PATCH 1/2] fix(providers): make Test probe chat completions, not just /models OpenAI-compatible verify can pass on a base missing /v1 if GET /models works; probing /chat/completions fails Test instead of hanging later turns. --- coworker/providers/registry.py | 108 +++++++++++++++++++++++++++------ tests/test_provider_verify.py | 68 ++++++++++++++++++--- 2 files changed, 149 insertions(+), 27 deletions(-) diff --git a/coworker/providers/registry.py b/coworker/providers/registry.py index 6c147705..c591f3f0 100644 --- a/coworker/providers/registry.py +++ b/coworker/providers/registry.py @@ -795,6 +795,88 @@ def _verify_vertex(fields: dict[str, Any], timeout: float) -> dict[str, Any]: return {"ok": False, "error": f"Vertex AI returned HTTP {resp.status_code}."} +def _first_listed_model_id(resp: Any) -> Optional[str]: + """Best-effort `id` from an OpenAI-style `GET /models` body, or None.""" + try: + data = resp.json().get("data") or [] + except Exception: + return None + if data and isinstance(data[0], dict) and data[0].get("id"): + return str(data[0]["id"]) + return None + + +def _verify_openai_compat( + d: ProviderDescriptor, + key: str, + base_url: Optional[str], + timeout: float, +) -> dict[str, Any]: + """Validate an OpenAI-compatible endpoint: list models, then probe chat completions. + + Listing `/models` alone is not enough — a base missing the `/v1` segment can still + answer GET /models while POST /chat/completions 404s, and Test would green-light a + setup that hangs on every real turn (#431). + """ + import httpx + + default_base = next( + (f.default for f in d.fields if f.key == "base_url" and f.default), "" + ) + base = ( + (base_url or "").strip().rstrip("/") + or default_base.rstrip("/") + or "https://api.openai.com/v1" + ) + headers = {"Authorization": f"Bearer {key}"} + try: + resp = httpx.get(base + "/models", headers=headers, timeout=timeout) + except Exception as exc: + return { + "ok": False, + "error": f"Couldn't reach {d.title} ({exc.__class__.__name__}).", + } + if resp.status_code in (401, 403): + return {"ok": False, "error": "Invalid API key."} + if resp.status_code >= 300: + return {"ok": False, "error": f"{d.title} returned HTTP {resp.status_code}."} + + model = _first_listed_model_id(resp) or "openworker-verify" + try: + creq = httpx.post( + base + "/chat/completions", + headers=headers, + json={ + "model": model, + "messages": [{"role": "user", "content": "ping"}], + "max_tokens": 1, + }, + timeout=timeout, + ) + except Exception as exc: + return { + "ok": False, + "error": f"Couldn't reach {d.title} ({exc.__class__.__name__}).", + } + if creq.status_code < 300: + return {"ok": True} + if creq.status_code in (401, 403): + return {"ok": False, "error": "Invalid API key."} + if creq.status_code == 404: + return { + "ok": False, + "error": ( + "Reached the server, but chat completions aren't at this endpoint. " + "For OpenAI-compatible servers the URL usually needs a `/v1` suffix " + "(e.g. http://127.0.0.1:1234/v1)." + ), + } + # Endpoint exists; a 400/422 is often "unknown model" when the list was empty. + if creq.status_code in (400, 422): + return {"ok": True} + return {"ok": False, "error": f"{d.title} returned HTTP {creq.status_code}."} + + def verify_provider_key( name: str, *, @@ -803,9 +885,10 @@ def verify_provider_key( fields: Optional[dict[str, Any]] = None, timeout: float = 10.0, ) -> dict[str, Any]: - """Validate a provider's credentials with one cheap, read-only call (list models) — the same - pattern connectors use to validate tokens. Transient: callers pass the key directly so a user - can Test before saving. Never raises; returns {ok, error?}. Multi-field cloud providers + """Validate a provider's credentials with a cheap live call. OpenAI-compatible + endpoints also probe `/chat/completions` so a missing `/v1` fails Test instead of + hanging later (#431). Transient: callers pass the key directly so a user can Test + before saving. Never raises; returns {ok, error?}. Multi-field cloud providers (Bedrock, Vertex) take their whole form via `fields`; everyone else uses api_key/base_url. """ import httpx @@ -816,6 +899,9 @@ def verify_provider_key( return _verify_bedrock(fields or {}, timeout) if name == "vertex": return _verify_vertex(fields or {}, timeout) + if name not in ("anthropic", "gemini", "ollama"): + # openai + any OpenAI-compatible endpoint (Azure, OpenRouter, vendors, vLLM…) + return _verify_openai_compat(d, key, base_url, timeout) try: if name == "anthropic": resp = httpx.get( @@ -829,23 +915,9 @@ def verify_provider_key( params={"key": key}, timeout=timeout, ) - elif name == "ollama": + else: # ollama base = _normalize_ollama_url(base_url) resp = httpx.get(base.rstrip("/") + "/models", timeout=timeout) - else: # openai + any OpenAI-compatible endpoint (Azure, OpenRouter, vendors, vLLM…) - default_base = next( - (f.default for f in d.fields if f.key == "base_url" and f.default), "" - ) - base = ( - (base_url or "").strip().rstrip("/") - or default_base.rstrip("/") - or "https://api.openai.com/v1" - ) - resp = httpx.get( - base + "/models", - headers={"Authorization": f"Bearer {key}"}, - timeout=timeout, - ) except Exception as exc: # DNS/connection/timeout — never let it bubble to a 500 return { "ok": False, diff --git a/tests/test_provider_verify.py b/tests/test_provider_verify.py index 8da7fe76..e865d104 100644 --- a/tests/test_provider_verify.py +++ b/tests/test_provider_verify.py @@ -1,5 +1,5 @@ -"""Tests for provider key detection + the live (read-only) Test/verify path. SDK-free: the -single httpx.get is monkeypatched so no network is touched.""" +"""Tests for provider key detection + the live Test/verify path. SDK-free: httpx is +monkeypatched so no network is touched.""" from __future__ import annotations @@ -41,32 +41,82 @@ def fake_get(url, **kwargs): monkeypatch.setattr("httpx.get", fake_get) +def _patch_openai_compat( + monkeypatch, + *, + models_status=200, + completions_status=200, + model_id="demo", + capture=None, + models_exc=None, + completions_exc=None, +): + """Stub GET /models + POST /chat/completions for the OpenAI-compat verify path.""" + + def fake_get(url, **kwargs): + if capture is not None: + capture["models_url"] = url + capture["models_headers"] = kwargs.get("headers") + if models_exc is not None: + raise models_exc + body = {"data": [{"id": model_id}]} if model_id else {"data": []} + return SimpleNamespace( + status_code=models_status, json=lambda: body + ) + + def fake_post(url, **kwargs): + if capture is not None: + capture["completions_url"] = url + capture["completions_json"] = kwargs.get("json") + if completions_exc is not None: + raise completions_exc + return SimpleNamespace(status_code=completions_status) + + monkeypatch.setattr("httpx.get", fake_get) + monkeypatch.setattr("httpx.post", fake_post) + + def test_verify_openai_ok(monkeypatch): cap: dict = {} - _patch_get(monkeypatch, status=200, capture=cap) + _patch_openai_compat(monkeypatch, capture=cap) assert verify_provider_key("openai", api_key="sk-x") == {"ok": True} - assert cap["url"] == "https://api.openai.com/v1/models" - assert cap["headers"]["Authorization"] == "Bearer sk-x" + assert cap["models_url"] == "https://api.openai.com/v1/models" + assert cap["models_headers"]["Authorization"] == "Bearer sk-x" + assert cap["completions_url"] == "https://api.openai.com/v1/chat/completions" + assert cap["completions_json"]["model"] == "demo" + assert cap["completions_json"]["max_tokens"] == 1 def test_verify_openai_custom_endpoint(monkeypatch): cap: dict = {} - _patch_get(monkeypatch, status=200, capture=cap) + _patch_openai_compat(monkeypatch, capture=cap) verify_provider_key( "openai", api_key="sk-x", base_url="https://gw.example/openai/v1/" ) - # trailing slash trimmed, /models appended to the custom endpoint - assert cap["url"] == "https://gw.example/openai/v1/models" + # trailing slash trimmed; /models then /chat/completions on the custom endpoint + assert cap["models_url"] == "https://gw.example/openai/v1/models" + assert cap["completions_url"] == "https://gw.example/openai/v1/chat/completions" def test_verify_bad_key_is_invalid(monkeypatch): - _patch_get(monkeypatch, status=401) + _patch_openai_compat(monkeypatch, models_status=401) assert verify_provider_key("openai", api_key="sk-bad") == { "ok": False, "error": "Invalid API key.", } +def test_verify_rejects_endpoint_missing_v1_when_completions_404(monkeypatch): + """#431: GET /models can succeed at a root that has no /chat/completions.""" + _patch_openai_compat(monkeypatch, completions_status=404) + res = verify_provider_key( + "qwen", api_key="placeholder", base_url="http://127.0.0.1:1234" + ) + assert res["ok"] is False + assert "/v1" in res["error"] + assert "chat completions" in res["error"].lower() + + def test_verify_anthropic_headers(monkeypatch): cap: dict = {} _patch_get(monkeypatch, status=200, capture=cap) From 1977f652c7a5c54afb0a17760a9af7edd19d5013 Mon Sep 17 00:00:00 2001 From: James Yang Date: Wed, 5 Aug 2026 02:56:09 -0400 Subject: [PATCH 2/2] refactor(providers): tighten OpenAI-compat Test probe Drop the extra helper and trim verify/test noise around the #431 completions check. --- coworker/providers/registry.py | 73 ++++++++++++++-------------------- tests/test_provider_verify.py | 27 +++---------- 2 files changed, 35 insertions(+), 65 deletions(-) diff --git a/coworker/providers/registry.py b/coworker/providers/registry.py index c591f3f0..c6e3775d 100644 --- a/coworker/providers/registry.py +++ b/coworker/providers/registry.py @@ -795,29 +795,13 @@ def _verify_vertex(fields: dict[str, Any], timeout: float) -> dict[str, Any]: return {"ok": False, "error": f"Vertex AI returned HTTP {resp.status_code}."} -def _first_listed_model_id(resp: Any) -> Optional[str]: - """Best-effort `id` from an OpenAI-style `GET /models` body, or None.""" - try: - data = resp.json().get("data") or [] - except Exception: - return None - if data and isinstance(data[0], dict) and data[0].get("id"): - return str(data[0]["id"]) - return None - - def _verify_openai_compat( d: ProviderDescriptor, key: str, base_url: Optional[str], timeout: float, ) -> dict[str, Any]: - """Validate an OpenAI-compatible endpoint: list models, then probe chat completions. - - Listing `/models` alone is not enough — a base missing the `/v1` segment can still - answer GET /models while POST /chat/completions 404s, and Test would green-light a - setup that hangs on every real turn (#431). - """ + """List models, then probe `/chat/completions` so a missing `/v1` fails Test (#431).""" import httpx default_base = next( @@ -829,19 +813,31 @@ def _verify_openai_compat( or "https://api.openai.com/v1" ) headers = {"Authorization": f"Bearer {key}"} - try: - resp = httpx.get(base + "/models", headers=headers, timeout=timeout) - except Exception as exc: + + def _unreachable(exc: Exception) -> dict[str, Any]: return { "ok": False, "error": f"Couldn't reach {d.title} ({exc.__class__.__name__}).", } - if resp.status_code in (401, 403): - return {"ok": False, "error": "Invalid API key."} + + def _http_error(code: int) -> dict[str, Any]: + if code in (401, 403): + return {"ok": False, "error": "Invalid API key."} + return {"ok": False, "error": f"{d.title} returned HTTP {code}."} + + try: + resp = httpx.get(base + "/models", headers=headers, timeout=timeout) + except Exception as exc: + return _unreachable(exc) if resp.status_code >= 300: - return {"ok": False, "error": f"{d.title} returned HTTP {resp.status_code}."} + return _http_error(resp.status_code) + + try: + data = resp.json().get("data") or [] + model = str(data[0]["id"]) if data and data[0].get("id") else "openworker-verify" + except Exception: + model = "openworker-verify" - model = _first_listed_model_id(resp) or "openworker-verify" try: creq = httpx.post( base + "/chat/completions", @@ -854,27 +850,19 @@ def _verify_openai_compat( timeout=timeout, ) except Exception as exc: - return { - "ok": False, - "error": f"Couldn't reach {d.title} ({exc.__class__.__name__}).", - } - if creq.status_code < 300: - return {"ok": True} - if creq.status_code in (401, 403): - return {"ok": False, "error": "Invalid API key."} + return _unreachable(exc) + if creq.status_code < 300 or creq.status_code in (400, 422): + return {"ok": True} # 400/422: path exists, model rejected if creq.status_code == 404: return { "ok": False, "error": ( "Reached the server, but chat completions aren't at this endpoint. " - "For OpenAI-compatible servers the URL usually needs a `/v1` suffix " + "OpenAI-compatible URLs usually need a `/v1` suffix " "(e.g. http://127.0.0.1:1234/v1)." ), } - # Endpoint exists; a 400/422 is often "unknown model" when the list was empty. - if creq.status_code in (400, 422): - return {"ok": True} - return {"ok": False, "error": f"{d.title} returned HTTP {creq.status_code}."} + return _http_error(creq.status_code) def verify_provider_key( @@ -885,11 +873,10 @@ def verify_provider_key( fields: Optional[dict[str, Any]] = None, timeout: float = 10.0, ) -> dict[str, Any]: - """Validate a provider's credentials with a cheap live call. OpenAI-compatible - endpoints also probe `/chat/completions` so a missing `/v1` fails Test instead of - hanging later (#431). Transient: callers pass the key directly so a user can Test - before saving. Never raises; returns {ok, error?}. Multi-field cloud providers - (Bedrock, Vertex) take their whole form via `fields`; everyone else uses api_key/base_url. + """Validate a provider's credentials with one cheap live call. Transient: callers + pass the key directly so a user can Test before saving. Never raises; returns + {ok, error?}. Multi-field cloud providers (Bedrock, Vertex) take their whole form + via `fields`; everyone else uses api_key/base_url. """ import httpx @@ -900,7 +887,7 @@ def verify_provider_key( if name == "vertex": return _verify_vertex(fields or {}, timeout) if name not in ("anthropic", "gemini", "ollama"): - # openai + any OpenAI-compatible endpoint (Azure, OpenRouter, vendors, vLLM…) + # openai + OpenAI-compatible endpoints (Azure, OpenRouter, vendors, vLLM…) return _verify_openai_compat(d, key, base_url, timeout) try: if name == "anthropic": diff --git a/tests/test_provider_verify.py b/tests/test_provider_verify.py index e865d104..5845df9a 100644 --- a/tests/test_provider_verify.py +++ b/tests/test_provider_verify.py @@ -48,28 +48,17 @@ def _patch_openai_compat( completions_status=200, model_id="demo", capture=None, - models_exc=None, - completions_exc=None, ): - """Stub GET /models + POST /chat/completions for the OpenAI-compat verify path.""" - def fake_get(url, **kwargs): if capture is not None: capture["models_url"] = url - capture["models_headers"] = kwargs.get("headers") - if models_exc is not None: - raise models_exc + capture["headers"] = kwargs.get("headers") body = {"data": [{"id": model_id}]} if model_id else {"data": []} - return SimpleNamespace( - status_code=models_status, json=lambda: body - ) + return SimpleNamespace(status_code=models_status, json=lambda: body) def fake_post(url, **kwargs): if capture is not None: capture["completions_url"] = url - capture["completions_json"] = kwargs.get("json") - if completions_exc is not None: - raise completions_exc return SimpleNamespace(status_code=completions_status) monkeypatch.setattr("httpx.get", fake_get) @@ -81,10 +70,8 @@ def test_verify_openai_ok(monkeypatch): _patch_openai_compat(monkeypatch, capture=cap) assert verify_provider_key("openai", api_key="sk-x") == {"ok": True} assert cap["models_url"] == "https://api.openai.com/v1/models" - assert cap["models_headers"]["Authorization"] == "Bearer sk-x" + assert cap["headers"]["Authorization"] == "Bearer sk-x" assert cap["completions_url"] == "https://api.openai.com/v1/chat/completions" - assert cap["completions_json"]["model"] == "demo" - assert cap["completions_json"]["max_tokens"] == 1 def test_verify_openai_custom_endpoint(monkeypatch): @@ -93,7 +80,6 @@ def test_verify_openai_custom_endpoint(monkeypatch): verify_provider_key( "openai", api_key="sk-x", base_url="https://gw.example/openai/v1/" ) - # trailing slash trimmed; /models then /chat/completions on the custom endpoint assert cap["models_url"] == "https://gw.example/openai/v1/models" assert cap["completions_url"] == "https://gw.example/openai/v1/chat/completions" @@ -106,15 +92,12 @@ def test_verify_bad_key_is_invalid(monkeypatch): } -def test_verify_rejects_endpoint_missing_v1_when_completions_404(monkeypatch): - """#431: GET /models can succeed at a root that has no /chat/completions.""" +def test_verify_rejects_endpoint_when_completions_404(monkeypatch): _patch_openai_compat(monkeypatch, completions_status=404) res = verify_provider_key( "qwen", api_key="placeholder", base_url="http://127.0.0.1:1234" ) - assert res["ok"] is False - assert "/v1" in res["error"] - assert "chat completions" in res["error"].lower() + assert not res["ok"] and "/v1" in res["error"] def test_verify_anthropic_headers(monkeypatch):