fix(provider): route GitHub Copilot chat to the token's endpoints.api host (GHE data residency)#1555
Conversation
… host GitHub Enterprise Cloud data residency mints Copilot tokens that are only valid at a tenant-specific host (api.<subdomain>.ghe.com), reported in the token exchange's endpoints.api field. Netclaw ignored that field and sent chat/completions to the hardcoded public api.githubcopilot.com, so a GHE token was rejected with HTTP 400 while the token exchange itself succeeded (issue netclaw-dev#1550). - CopilotTokenExchanger parses endpoints.api and returns it alongside the bearer (now a SensitiveString) as CopilotToken; a non-HTTPS value is ignored so authenticated traffic can't be redirected off HTTPS. - CopilotRequestPolicy rebases the outgoing chat request onto that host (origin + any base path), unless the operator deliberately configured a custom endpoint (e.g. a corporate proxy), which is always respected. - GitHubCopilotDescriptor probes /models at the same host and no longer masks auth/connection failures behind the curated fallback, so a broken GHE provider surfaces at `provider add` instead of on the first chat. - ProbeHelpers/ProviderProbeResult gain a Transient classifier so only reachable-server retryable failures (timeout/5xx/429/408) are masked. Fixes netclaw-dev#1550
The optional apiBase=null default silently selected the no-endpoints.api (standard-account) token shape, so a new routing test could exercise the wrong shape without ever deciding to — the silent-fallback trap this fix is otherwise about. Make it mandatory so every call site states whether it models a standard or a GHE-data-residency token (apiBase: null vs a host).
1e1d804 to
c89cd0c
Compare
|
Found some issues that I've had Claude fix while doing an earlier review pass (silent fallbacks etc, the sort of design issue that led to this problem in the first place.) |
Live-tested against a real GitHub Copilot accountI built this branch, published the CLI and daemon, and swapped them into a running install to test against an actual GitHub Copilot account (a standard github.com one, not GHE). Two paths mattered here. First, model discovery, which runs the probe. Then the chat itself, which runs the routing. I pointed the main model at a Copilot model and cleared the fallback first, so nothing could quietly answer in Copilot's place. A one-word prompt came back with a clean response and real token usage, meaning the request routed to the host from The session log opens the same way as the bug report, with the token exchange returning 200. The difference is the next step: the chat completes instead of failing with 400. One thing this run doesn't cover is the GHE data-residency case, where I restored the original config afterward. |
Removes the last silent fallback in the endpoints.api routing path. When we follow the token's host (no operator override) but the exchange returns no usable endpoints.api, the code silently kept the configured/default host — the same guess that sent a GHE token to the public host in netclaw-dev#1550. Refuse to guess: the chat policy throws with a clear message and the setup probe fails loudly instead of probing a guessed host. A deliberate operator endpoint override is still honored — that's an explicit choice, not a guess. Test tokens now include a realistic endpoints.api (GitHub always reports it; litellm's Copilot provider likewise uses it with no fallback), and a null host is exercised only to prove the loud failure.
c89cd0c to
31ec86e
Compare
| /// list or must surface as a hard failure. Defaults to false so an | ||
| /// unclassified failure is never silently masked. | ||
| /// </summary> | ||
| public bool Transient { get; init; } |
| } | ||
|
|
||
| [Fact] | ||
| public async Task ProcessAsync_RoutesRequestToTokenApiHost() |
There was a problem hiding this comment.
Mostly just testing to make sure the request gets routed to the correct place.
| // Request-timeout, 5xx, and rate limiting are worth a retry; auth and | ||
| // other 4xx errors (incl. 404 "not found" at the token's own API host) | ||
| // signal a real misconfiguration the operator has to fix. | ||
| var transient = statusCode is HttpStatusCode.RequestTimeout |
There was a problem hiding this comment.
an improvement that should help with other cloud-hosted providers too
Fixes #1550.
Root cause
GitHub Enterprise Cloud data residency mints Copilot tokens that are scoped to the enterprise's region and are only valid at a tenant-specific host (
api.<subdomain>.ghe.com). GitHub reports that host in theendpoints.apifield of the/copilot_internal/v2/tokenresponse — and every real Copilot client (litellm, the VS Code extension) uses it as the chat base URL.Netclaw ignored
endpoints.apiand sentchat/completionsto the hardcoded publicapi.githubcopilot.com. A GHE-scoped token at the public host is rejected with HTTP 400 — exactly the split in the issue: the token exchange (against the correct tenant host) returned 200, then the chat call (against the wrong public host) returned 400.Secondary problem: the setup probe hit the same wrong host, failed, and was silently masked by the curated fallback model list — so
provider addreported healthy for a provider that couldn't chat.What changed
CopilotTokenExchangerparsesendpoints.apiand returns it alongside the bearer as a newCopilotToken. The bearer is now aSensitiveString(redactedToString), and a non-HTTPSendpoints.apiis ignored so authenticated traffic can never be redirected off HTTPS.CopilotRequestPolicyrebases the outgoing chat request onto that host (origin + any base path), preserving the SDK-built/chat/completionspath and query. This is the actual 400 fix.GitHubCopilotProviderPluginwires the routing decision through.GitHubCopilotDescriptorprobes/modelsat the token's host and no longer masks auth/connection failures behind the curated fallback — a broken GHE provider now surfaces atprovider addinstead of on the first chat.ProbeHelpers/ProviderProbeResultgain aTransientclassifier so only reachable-server retryable failures (timeout / 5xx / 429 / 408) are masked; connection failures and auth/4xx errors surface.Operator override is respected
--github-host/--github-api-basestill only configure the OAuth and token-exchange hosts. If an operator deliberately points the entry at a custom endpoint (e.g. a corporate proxy via--endpoint), that override always wins over the token's host on both the chat and probe paths — traffic is never silently rerouted.Testing
dotnet slopwatch analyze: 0 issues.Add-FileHeaders.ps1 -Verify: clean. Full solution builds warning-free.An xhigh multi-agent code review was run on the diff; findings on probe-failure classification (connection failures were masked), chat/probe URL-path consistency, and secret hygiene (
SensitiveString) are folded into this PR.Notes
No config-schema, system-skill, or eval changes are required: the
provider addCLI surface, config format, and operator guidance are unchanged, andProviderProbeResultis a runtime type (not a persisted*Config).