mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-08-31 00:03:01 +03:00
fix(providers): harden OAuth model discovery
This commit is contained in:
@@ -814,8 +814,8 @@ a nanobot update.
|
||||
GitHub Copilot uses OAuth instead of API keys. Requires a [GitHub account with a plan](https://github.com/features/copilot/plans) configured. No `providers.github_copilot` block is needed in `config.json`; `nanobot provider login` stores the OAuth session outside config.
|
||||
|
||||
After login, the WebUI loads the account-specific Copilot model catalog online.
|
||||
Only models compatible with nanobot's current Copilot chat-completions transport
|
||||
are shown; Responses-only entries are intentionally omitted.
|
||||
Only models compatible with nanobot's current chat-completions or Responses
|
||||
transport are shown.
|
||||
|
||||
For GitHub Enterprise / Copilot for Business, set the endpoint overrides you need before login:
|
||||
```bash
|
||||
|
||||
+2
-3
@@ -608,9 +608,8 @@ nanobot provider login github-copilot --set-main
|
||||
```
|
||||
|
||||
The WebUI reads the models enabled for the signed-in Copilot account. nanobot
|
||||
currently lists only entries that support Copilot's chat-completions endpoint;
|
||||
models exposed solely through the Responses endpoint stay hidden until that
|
||||
wire protocol is supported by the Copilot provider.
|
||||
lists entries that support its current Copilot chat-completions or Responses
|
||||
transport and hides models that it cannot route safely.
|
||||
|
||||
Each command authenticates the selected provider and makes its current default model active. OpenAI Codex and eligible GitHub Copilot models participate in [Responses state retention](./configuration.md#responses-state-and-compaction), while native compaction remains provider-capability-specific. OAuth providers are not valid automatic fallbacks. See [`troubleshooting.md`](./troubleshooting.md#provider-and-model-problems) for proxy, headless-login, model-name, and config-key errors.
|
||||
|
||||
|
||||
@@ -314,7 +314,7 @@ def get_github_copilot_model_catalog(
|
||||
account_key = _catalog_account_key(getattr(token, "account_id", None))
|
||||
cache_key = (
|
||||
f"{storage.get_token_path()}\0{account_key}\0"
|
||||
f"{_resolve('NANOBOT_COPILOT_BASE_URL', DEFAULT_COPILOT_BASE_URL)}"
|
||||
f"{_resolve('NANOBOT_COPILOT_BASE_URL', DEFAULT_COPILOT_BASE_URL)}\0{proxy or ''}"
|
||||
)
|
||||
return _GITHUB_COPILOT_MODEL_CATALOG.get(cache_key=cache_key, proxy=proxy)
|
||||
|
||||
@@ -389,10 +389,7 @@ def _parse_github_copilot_models(payload: Any) -> tuple[ProviderModelSpec, ...]:
|
||||
or wire_id in seen
|
||||
or row.get("model_picker_enabled") is not True
|
||||
or policy.get("state") == "disabled"
|
||||
or (
|
||||
isinstance(endpoints, list)
|
||||
and "/chat/completions" not in cast(list[object], endpoints)
|
||||
)
|
||||
or not _copilot_transport_supported(wire_id, endpoints)
|
||||
):
|
||||
continue
|
||||
seen.add(wire_id)
|
||||
@@ -419,6 +416,18 @@ def _parse_github_copilot_models(payload: Any) -> tuple[ProviderModelSpec, ...]:
|
||||
return tuple(models)
|
||||
|
||||
|
||||
def _copilot_transport_supported(wire_id: str, endpoints: object) -> bool:
|
||||
if not isinstance(endpoints, list):
|
||||
return True
|
||||
supported = cast(list[object], endpoints)
|
||||
if "/chat/completions" in supported:
|
||||
return True
|
||||
model = wire_id.lower()
|
||||
return "/responses" in supported and any(
|
||||
token in model for token in ("gpt-5", "o1", "o3", "o4")
|
||||
)
|
||||
|
||||
|
||||
def _oauth_fallback_models(provider_name: str) -> tuple[ProviderModelSpec, ...]:
|
||||
spec = find_by_name(provider_name)
|
||||
assert spec is not None
|
||||
|
||||
@@ -73,49 +73,51 @@ class OAuthModelCatalog:
|
||||
|
||||
def get(self, *, cache_key: str, proxy: str | None = None) -> OAuthModelCatalogSnapshot:
|
||||
"""Return a fresh catalog, sharing concurrent work and retaining a fallback."""
|
||||
while True:
|
||||
with self._condition:
|
||||
with self._condition:
|
||||
generation = self._generation
|
||||
cached = self._cached_result(cache_key)
|
||||
if cached is not None:
|
||||
return cached
|
||||
while cache_key in self._inflight:
|
||||
self._condition.wait()
|
||||
if generation != self._generation:
|
||||
return self._stale_or_fallback(None, self._monotonic())
|
||||
cached = self._cached_result(cache_key)
|
||||
if cached is not None:
|
||||
return cached
|
||||
while cache_key in self._inflight:
|
||||
self._condition.wait()
|
||||
cached = self._cached_result(cache_key)
|
||||
if cached is not None:
|
||||
return cached
|
||||
generation = self._generation
|
||||
self._inflight.add(cache_key)
|
||||
self._inflight.add(cache_key)
|
||||
|
||||
try:
|
||||
models = tuple(self._fetch(proxy))
|
||||
if not models:
|
||||
raise ValueError("provider returned an empty model catalog")
|
||||
except Exception as exc:
|
||||
logger.warning("OAuth model catalog refresh failed: type={}", type(exc).__name__)
|
||||
with self._condition:
|
||||
invalidated = generation != self._generation
|
||||
result = self._failure_result(cache_key) if not invalidated else None
|
||||
else:
|
||||
now = self._monotonic()
|
||||
result = OAuthModelCatalogSnapshot(
|
||||
models=models,
|
||||
source="remote",
|
||||
fetched_at=self._wall_clock(),
|
||||
try:
|
||||
models = tuple(self._fetch(proxy))
|
||||
if not models:
|
||||
raise ValueError("provider returned an empty model catalog")
|
||||
except Exception as exc:
|
||||
logger.warning("OAuth model catalog refresh failed: type={}", type(exc).__name__)
|
||||
with self._condition:
|
||||
result = (
|
||||
self._stale_or_fallback(None, self._monotonic())
|
||||
if generation != self._generation
|
||||
else self._failure_result(cache_key)
|
||||
)
|
||||
with self._condition:
|
||||
invalidated = generation != self._generation
|
||||
if not invalidated:
|
||||
self._store(cache_key, _CacheEntry(snapshot=result, stored_at=now))
|
||||
self._failures.pop(cache_key, None)
|
||||
finally:
|
||||
with self._condition:
|
||||
self._inflight.discard(cache_key)
|
||||
self._condition.notify_all()
|
||||
else:
|
||||
now = self._monotonic()
|
||||
result = OAuthModelCatalogSnapshot(
|
||||
models=models,
|
||||
source="remote",
|
||||
fetched_at=self._wall_clock(),
|
||||
)
|
||||
with self._condition:
|
||||
if generation != self._generation:
|
||||
result = self._stale_or_fallback(None, now)
|
||||
else:
|
||||
self._store(cache_key, _CacheEntry(snapshot=result, stored_at=now))
|
||||
self._failures.pop(cache_key, None)
|
||||
finally:
|
||||
with self._condition:
|
||||
self._inflight.discard(cache_key)
|
||||
self._condition.notify_all()
|
||||
|
||||
if invalidated:
|
||||
continue
|
||||
assert result is not None
|
||||
return result
|
||||
return result
|
||||
|
||||
def invalidate(self) -> None:
|
||||
"""Drop cached work and prevent an older identity refresh from being stored."""
|
||||
@@ -123,18 +125,23 @@ class OAuthModelCatalog:
|
||||
self._generation += 1
|
||||
self._entries.clear()
|
||||
self._failures.clear()
|
||||
self._condition.notify_all()
|
||||
|
||||
def _cached_result(self, cache_key: str) -> OAuthModelCatalogSnapshot | None:
|
||||
now = self._monotonic()
|
||||
entry = self._entries.get(cache_key)
|
||||
if entry is not None and now - entry.stored_at < self._fresh_ttl_s:
|
||||
return replace(entry.snapshot, source="cache")
|
||||
if self._failures.get(cache_key, 0) > now:
|
||||
failure_until = self._failures.get(cache_key)
|
||||
if failure_until is not None and failure_until <= now:
|
||||
self._failures.pop(cache_key, None)
|
||||
elif failure_until is not None:
|
||||
return self._stale_or_fallback(entry, now)
|
||||
return None
|
||||
|
||||
def _failure_result(self, cache_key: str) -> OAuthModelCatalogSnapshot:
|
||||
now = self._monotonic()
|
||||
self._reserve(cache_key)
|
||||
self._failures[cache_key] = now + self._failure_ttl_s
|
||||
return self._stale_or_fallback(self._entries.get(cache_key), now)
|
||||
|
||||
@@ -157,12 +164,24 @@ class OAuthModelCatalog:
|
||||
)
|
||||
|
||||
def _store(self, cache_key: str, entry: _CacheEntry) -> None:
|
||||
if cache_key not in self._entries and len(self._entries) >= self._max_entries:
|
||||
oldest = min(self._entries, key=lambda key: self._entries[key].stored_at)
|
||||
self._entries.pop(oldest, None)
|
||||
self._failures.pop(oldest, None)
|
||||
self._reserve(cache_key)
|
||||
self._entries[cache_key] = entry
|
||||
|
||||
def _reserve(self, cache_key: str) -> None:
|
||||
known = set(self._entries) | set(self._failures)
|
||||
if cache_key in known or len(known) < self._max_entries:
|
||||
return
|
||||
oldest = min(
|
||||
known,
|
||||
key=lambda key: (
|
||||
self._entries[key].stored_at
|
||||
if key in self._entries
|
||||
else self._failures[key] - self._failure_ttl_s
|
||||
),
|
||||
)
|
||||
self._entries.pop(oldest, None)
|
||||
self._failures.pop(oldest, None)
|
||||
|
||||
|
||||
def get_oauth_model_catalog(
|
||||
provider_name: str,
|
||||
|
||||
@@ -153,6 +153,7 @@ PROVIDERS: tuple[ProviderSpec, ...] = (
|
||||
backend="openai_compat",
|
||||
is_direct=True,
|
||||
),
|
||||
|
||||
# === Azure OpenAI (direct API calls with API version 2024-10-21) =====
|
||||
ProviderSpec(
|
||||
name="azure_openai",
|
||||
@@ -315,6 +316,7 @@ PROVIDERS: tuple[ProviderSpec, ...] = (
|
||||
detect_by_base_keyword="siliconflow",
|
||||
default_api_base="https://api.siliconflow.cn/v1",
|
||||
),
|
||||
|
||||
# Novita AI: OpenAI-compatible gateway for hosted model APIs.
|
||||
ProviderSpec(
|
||||
name="novita",
|
||||
@@ -326,6 +328,7 @@ PROVIDERS: tuple[ProviderSpec, ...] = (
|
||||
detect_by_base_keyword="novita",
|
||||
default_api_base="https://api.novita.ai/openai",
|
||||
),
|
||||
|
||||
# VolcEngine (火山引擎): OpenAI-compatible gateway, pay-per-use models
|
||||
ProviderSpec(
|
||||
name="volcengine",
|
||||
@@ -339,6 +342,7 @@ PROVIDERS: tuple[ProviderSpec, ...] = (
|
||||
thinking_style="thinking_type",
|
||||
supports_max_completion_tokens=True,
|
||||
),
|
||||
|
||||
# VolcEngine Coding Plan (火山引擎 Coding Plan): same key as volcengine
|
||||
ProviderSpec(
|
||||
name="volcengine_coding_plan",
|
||||
@@ -352,6 +356,7 @@ PROVIDERS: tuple[ProviderSpec, ...] = (
|
||||
thinking_style="thinking_type",
|
||||
supports_max_completion_tokens=True,
|
||||
),
|
||||
|
||||
# BytePlus: VolcEngine international, pay-per-use models
|
||||
ProviderSpec(
|
||||
name="byteplus",
|
||||
@@ -365,6 +370,7 @@ PROVIDERS: tuple[ProviderSpec, ...] = (
|
||||
strip_model_prefix=True,
|
||||
thinking_style="thinking_type",
|
||||
),
|
||||
|
||||
# BytePlus Coding Plan: same key as byteplus
|
||||
ProviderSpec(
|
||||
name="byteplus_coding_plan",
|
||||
@@ -377,6 +383,8 @@ PROVIDERS: tuple[ProviderSpec, ...] = (
|
||||
strip_model_prefix=True,
|
||||
thinking_style="thinking_type",
|
||||
),
|
||||
|
||||
|
||||
# === Standard providers (matched by model-name keywords) ===============
|
||||
# Anthropic: native Anthropic SDK
|
||||
ProviderSpec(
|
||||
@@ -492,6 +500,11 @@ PROVIDERS: tuple[ProviderSpec, ...] = (
|
||||
display_name="Github Copilot",
|
||||
model_catalog="hybrid",
|
||||
builtin_models=(
|
||||
ProviderModelSpec(
|
||||
id="github-copilot/gpt-5.4-mini",
|
||||
label="GPT-5.4 Mini",
|
||||
description="GitHub Copilot Responses model.",
|
||||
),
|
||||
ProviderModelSpec(
|
||||
id="github-copilot/gpt-4.1",
|
||||
label="GPT-4.1",
|
||||
@@ -768,7 +781,7 @@ PROVIDERS: tuple[ProviderSpec, ...] = (
|
||||
env_key="QIANFAN_API_KEY",
|
||||
display_name="Qianfan",
|
||||
backend="openai_compat",
|
||||
default_api_base="https://qianfan.baidubce.com/v2",
|
||||
default_api_base="https://qianfan.baidubce.com/v2"
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
@@ -207,6 +207,7 @@ class XAIGrokProvider(LLMProvider):
|
||||
)
|
||||
if on_stream_recover is not None:
|
||||
await on_stream_recover()
|
||||
headers = _build_headers(token.access, wire_model)
|
||||
|
||||
content, tool_calls, finish_reason, usage, reasoning_content = result
|
||||
usage = _combine_usage(retry_usage, usage)
|
||||
@@ -614,10 +615,11 @@ def _xai_error_response(exc: Exception) -> LLMResponse:
|
||||
)
|
||||
message = str(exc).strip() or "unexpected error"
|
||||
retry_after = getattr(exc, "retry_after", None)
|
||||
usage = getattr(exc, "usage", None)
|
||||
return LLMResponse(
|
||||
content=f"Error calling xAI ({type(exc).__name__}): {message}",
|
||||
finish_reason="error",
|
||||
usage=getattr(exc, "usage", None),
|
||||
usage=usage if isinstance(usage, LLMUsage) else None,
|
||||
retry_after=retry_after,
|
||||
error_status_code=int(status_code) if status_code is not None else None,
|
||||
error_kind=error_kind,
|
||||
|
||||
@@ -259,8 +259,14 @@ def test_github_copilot_catalog_only_lists_compatible_chat_models(
|
||||
},
|
||||
},
|
||||
{
|
||||
"id": "responses-only",
|
||||
"name": "Responses only",
|
||||
"id": "gpt-5.4-mini",
|
||||
"name": "GPT-5.4 Mini",
|
||||
"model_picker_enabled": True,
|
||||
"supported_endpoints": ["/responses"],
|
||||
},
|
||||
{
|
||||
"id": "unknown-responses-only",
|
||||
"name": "Unknown Responses only",
|
||||
"model_picker_enabled": True,
|
||||
"supported_endpoints": ["/responses"],
|
||||
},
|
||||
@@ -298,13 +304,22 @@ def test_github_copilot_catalog_only_lists_compatible_chat_models(
|
||||
catalog = get_oauth_model_catalog("github_copilot")
|
||||
|
||||
assert catalog.source == "remote"
|
||||
assert [model.id for model in catalog.models] == ["github-copilot/claude-sonnet"]
|
||||
assert [model.id for model in catalog.models] == [
|
||||
"github-copilot/claude-sonnet",
|
||||
"github-copilot/gpt-5.4-mini",
|
||||
]
|
||||
assert catalog.models[0].context_window == 200_000
|
||||
assert catalog.models[0].reasoning_efforts == ("low", "high")
|
||||
assert len(captured) == 2
|
||||
assert captured[0].headers["Authorization"] == "token github-secret"
|
||||
assert captured[1].headers["Authorization"] == "Bearer copilot-secret"
|
||||
assert str(captured[1].url) == "https://api.individual.githubcopilot.com/models"
|
||||
assert get_oauth_model_catalog("github_copilot").source == "cache"
|
||||
assert get_oauth_model_catalog(
|
||||
"github_copilot",
|
||||
proxy="http://proxy.example:8080",
|
||||
).source == "remote"
|
||||
assert len(captured) == 4
|
||||
|
||||
|
||||
def test_catalog_single_flights_concurrent_refreshes() -> None:
|
||||
@@ -337,28 +352,53 @@ def test_catalog_single_flights_concurrent_refreshes() -> None:
|
||||
def test_catalog_invalidation_discards_an_inflight_account_refresh() -> None:
|
||||
started = threading.Event()
|
||||
release = threading.Event()
|
||||
identity = ["old-account"]
|
||||
|
||||
def fetch(_proxy: str | None) -> tuple[ProviderModelSpec, ...]:
|
||||
current = identity[0]
|
||||
if current == "old-account":
|
||||
started.set()
|
||||
assert release.wait(timeout=2)
|
||||
return (ProviderModelSpec(id=f"provider/{current}", label=current),)
|
||||
|
||||
catalog = OAuthModelCatalog(fallback_models=(_fallback_model(),), fetch=fetch)
|
||||
with ThreadPoolExecutor(max_workers=2) as pool:
|
||||
old_future = pool.submit(catalog.get, cache_key="old-key")
|
||||
assert started.wait(timeout=2)
|
||||
identity[0] = "new-account"
|
||||
catalog.invalidate()
|
||||
new_future = pool.submit(catalog.get, cache_key="new-key")
|
||||
new_result = new_future.result(timeout=2)
|
||||
release.set()
|
||||
old_result = old_future.result(timeout=2)
|
||||
|
||||
assert old_result.source == "fallback"
|
||||
assert new_result.models[0].id == "provider/new-account"
|
||||
|
||||
identity[0] = "old-account"
|
||||
assert catalog.get(cache_key="old-key").models[0].id == "provider/old-account"
|
||||
|
||||
|
||||
def test_catalog_bounds_failure_only_keys() -> None:
|
||||
calls = 0
|
||||
|
||||
def fetch(_proxy: str | None) -> tuple[ProviderModelSpec, ...]:
|
||||
nonlocal calls
|
||||
calls += 1
|
||||
if calls == 1:
|
||||
started.set()
|
||||
assert release.wait(timeout=2)
|
||||
return (ProviderModelSpec(id="provider/old-account", label="Old"),)
|
||||
return (ProviderModelSpec(id="provider/new-account", label="New"),)
|
||||
raise httpx.ConnectError("offline")
|
||||
|
||||
catalog = OAuthModelCatalog(fallback_models=(_fallback_model(),), fetch=fetch)
|
||||
with ThreadPoolExecutor(max_workers=1) as pool:
|
||||
future = pool.submit(catalog.get, cache_key="shared")
|
||||
assert started.wait(timeout=2)
|
||||
catalog.invalidate()
|
||||
release.set()
|
||||
result = future.result(timeout=2)
|
||||
catalog = OAuthModelCatalog(
|
||||
fallback_models=(_fallback_model(),),
|
||||
fetch=fetch,
|
||||
max_entries=2,
|
||||
)
|
||||
|
||||
assert calls == 2
|
||||
assert result.models[0].id == "provider/new-account"
|
||||
assert catalog.get(cache_key="shared").models[0].id == "provider/new-account"
|
||||
for key in ("one", "two", "three"):
|
||||
assert catalog.get(cache_key=key).source == "fallback"
|
||||
|
||||
assert calls == 3
|
||||
assert catalog.get(cache_key="one").source == "fallback"
|
||||
assert calls == 4
|
||||
|
||||
|
||||
def test_catalog_returns_stale_then_negative_caches_refresh_failure() -> None:
|
||||
|
||||
@@ -637,14 +637,16 @@ async def test_provider_recovers_unfinished_hosted_tool_once_and_preserves_usage
|
||||
_mock_token(monkeypatch)
|
||||
_mock_model_capabilities(monkeypatch, supports_backend_search=True)
|
||||
attempts = 0
|
||||
request_ids: list[str] = []
|
||||
streamed: list[str] = []
|
||||
recovered: list[bool] = []
|
||||
first_usage = LLMUsage.reported(input_tokens=10, output_tokens=2)
|
||||
second_usage = LLMUsage.reported(input_tokens=11, output_tokens=4)
|
||||
|
||||
async def fake_request(_url, _headers, body, **kwargs):
|
||||
async def fake_request(_url, headers, body, **kwargs):
|
||||
nonlocal attempts
|
||||
attempts += 1
|
||||
request_ids.append(headers["x-grok-req-id"])
|
||||
assert body["max_turns"] == 5
|
||||
if attempts == 1:
|
||||
await kwargs["on_content_delta"]("I will keep searching.")
|
||||
@@ -668,12 +670,43 @@ async def test_provider_recovers_unfinished_hosted_tool_once_and_preserves_usage
|
||||
)
|
||||
|
||||
assert attempts == 2
|
||||
assert len(set(request_ids)) == 2
|
||||
assert recovered == [True]
|
||||
assert streamed == ["I will keep searching.", "Final researched answer."]
|
||||
assert response.content == "Final researched answer."
|
||||
assert response.usage == first_usage + second_usage
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_provider_preserves_usage_when_hosted_tool_recovery_also_fails(
|
||||
monkeypatch,
|
||||
) -> None:
|
||||
_mock_token(monkeypatch)
|
||||
_mock_model_capabilities(monkeypatch, supports_backend_search=True)
|
||||
attempts = 0
|
||||
usage = LLMUsage.reported(input_tokens=10, output_tokens=2)
|
||||
|
||||
async def fake_request(*_args, **_kwargs):
|
||||
nonlocal attempts
|
||||
attempts += 1
|
||||
raise _XAIIncompleteHostedToolError(
|
||||
[{"name": "x_search", "call_id": f"search-{attempts}"}],
|
||||
usage=usage,
|
||||
)
|
||||
|
||||
monkeypatch.setattr("nanobot.providers.xai_grok_provider._request_xai", fake_request)
|
||||
provider = XAIGrokProvider()
|
||||
|
||||
response = await provider.chat_stream_with_retry(
|
||||
[{"role": "user", "content": "Search X"}],
|
||||
on_stream_recover=lambda: _append([], True),
|
||||
)
|
||||
|
||||
assert attempts == 2
|
||||
assert response.finish_reason == "error"
|
||||
assert response.usage == usage + usage
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_raw_response_error_preserves_bounded_redacted_body(monkeypatch) -> None:
|
||||
original_client = httpx.AsyncClient
|
||||
|
||||
Reference in New Issue
Block a user