fix(provider): connect ChatGPT OAuth and fail its errors loudly - #79
Merged
Conversation
…ery 400 A successful ChatGPT sign in landed on "Limited connection" with a raw Dio 400 message. The OAuth exchange was fine: `_discoverAndSave` ran model discovery unconditionally, and the ChatGPT runtime base URL points at the Codex backend, which serves only the Responses API and answers 400 for `/models`. That failure classified as `unavailable`, which maps to `degraded`. Make discovery support an explicit property of the runtime config so the ChatGPT special case stays beside the base-URL override it belongs to, and settle those connections on the bundled catalog. The 400 classification is unchanged for endpoints that really do serve `/models`.
…ng Dio Six defects around the ChatGPT authorization flow, none of which could report themselves usefully: - A denied authorization redirects back with `error` and no `code`. The callback loop answered "Authorization code missing" and kept waiting for a code the identity provider would never send. - The callback binder silently fell back from port 1455 to 1457, producing a redirect_uri the public Codex client has not registered and an opaque 400 at the token endpoint. It now fails naming the busy port. - A rejected token exchange surfaced the raw Dio message, which quotes a status code the user cannot act on, instead of the OAuth error the token endpoint returned. - That rejection also closed the callback server without answering, so the browser tab showed a connection reset rather than the reason. - The device polling loop caught the exchange's DioException and mistook a rejection for `authorization_pending`, polling on until expiry. - The callback page interpolated provider-supplied text into HTML unescaped. Add `OAuthAuthorizationFailure`, whose `toString` is the bare sentence the coordinator publishes straight to the attempt tile.
`_callbackBody` returned the decode future from inside `try`, so the `finally` force-closed the HttpClient while the body was still streaming. Linux delivered the small page in a single packet and won the race; Windows tore the socket down mid-read and failed with "Connection closed while receiving data".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Signing in with Sign in with ChatGPT completed the browser flow and then showed:
OAuth itself was fine.
Limited connection(degraded) is only reachable after a credential is stored, so the token exchange had succeeded — the 400 came from the step after it.Root cause
ProviderService._discoverAndSaveran model discovery unconditionally. ChatGPT OAuth connections route tohttps://chatgpt.com/backend-api/codex, which serves only the Responses API and has no/modelslisting, so it answers 400. That classified asunavailable→degraded→ the raw Dio message on the tile.Changes
1. Skip discovery where it does not apply.
ProviderRuntimeConfig.supportsModelDiscovery(defaulttrue) makes this an explicit typed property; only the ChatGPT OAuth branch sets itfalse, keeping the special case beside the base-URL override that already lives in_runtimeConfig. Those connections settle on the bundled catalog. The 400 classification is unchanged for endpoints that really do serve/models.2. Fix six defects in the authorization flow, none of which could report themselves usefully:
?error=access_denied) was never parsed — the callback loop answered "Authorization code missing" and waited forever for a code that would never arrive.redirect_urithe public Codex client has not registered and an opaque 400. It now fails naming the busy port.DioExceptionand mistook a rejection forauthorization_pending, polling on until expiry.New
OAuthAuthorizationFailurereturns the bare sentence fromtoString, since the coordinator publishes'$error'straight to the attempt tile.Tests
Gateway: denied authorization, HTML escaping of
<script>inerror_description, rejected exchange (asserts nostatus code/DioExceptionin the message), device loop halting on rejection (delays == 0), binder refusing to move off its port. Coordinator: the message reaches the attempt verbatim, without a class-name prefix. Service: discovery never called for ChatGPT OAuth, plus an API-key regression proving discovery still runs and still degrades. Also adapter defaults, a real-daemon vertical slice, widget, and e2e.The e2e previously asserted
contains('oauth-e2e-model')— a discovered model. That encoded the broken behavior and now asserts the bundled catalog instead.The binder test uses port 0 and reuses the assigned port rather than closing and rebinding, avoiding a TOCTOU race; verified over repeated runs.
Verification
dart run melos verify— all PASSdart run melos verify:debug— exit 0 (neededxvfb-run; this shell has noDISPLAY)Not verified: a real ChatGPT sign-in end to end, and sending a message over the OAuth connection to confirm
.../codex/responsesworks with the stored token.