feat: Add GitHub Copilot provider with OAuth support#2366
feat: Add GitHub Copilot provider with OAuth support#2366alex-spacemit wants to merge 3 commits intoagentscope-ai:mainfrom
Conversation
github-actions
Bot
added
the
first-time-contributor
Welcome to CoPaw! 🐾Hi @alex-spacemit, thank you for your first Pull Request! 🎉 🙌 Join Developer CommunityThanks so much for your contribution! We'd love to invite you to join the official CoPaw developer group! You can find the Discord and DingTalk group links under the "Developer Community" section on our docs page: We truly appreciate your enthusiasm—and look forward to your future contributions! 😊 We'll review your PR soon. Tip ⭐ If you find CoPaw useful, please give us a Star! Staying ahead Star CoPaw on GitHub and be instantly notified of new releases. Your star helps more developers discover this project! 🐾 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements support for GitHub Copilot by introducing a new provider that utilizes the GitHub device authorization flow. Key additions include a compatibility wrapper for the OpenAI Responses API, backend endpoints for session management, and frontend components to handle the authentication UI and polling. The review feedback identifies opportunities to improve the robustness of the React polling effect by optimizing its dependencies and suggests implementing a cleanup mechanism for expired authorization sessions in the backend to avoid potential memory leaks.
| cancelled = true; | ||
| window.clearTimeout(timer); | ||
| }; | ||
| }, [authSession, isOauthProvider, onSaved, open, provider.id, t]); |
There was a problem hiding this comment.
The onSaved function is included in the useEffect dependency array. If onSaved is not wrapped in useCallback in the parent component, it will have a new reference on each render, causing this effect to re-run unnecessarily. This would clear and reset the polling timer, potentially delaying or interrupting the authentication flow.
To make this effect more robust, consider using a ref to hold the latest version of onSaved and removing it from the dependency array. This avoids re-triggering the effect when the function reference changes.
Example of how to apply this pattern:
const onSavedRef = useRef(onSaved);
useEffect(() => {
onSavedRef.current = onSaved;
}, [onSaved]);
useEffect(() => {
// ... inside your polling logic
await onSavedRef.current();
// ...
}, [authSession, isOauthProvider, open, provider.id, t]); // onSaved is removed| self, | ||
| timeout: float = 10, | ||
| ) -> DeviceAuthorizationSession: | ||
| async with httpx.AsyncClient( |
There was a problem hiding this comment.
To prevent potential memory leaks from abandoned device authorization sessions, it's a good practice to clean up expired sessions from _device_sessions before creating a new one. A user might start the authorization process but never complete it, leaving an expired session in memory until the server restarts.
# Clean up expired sessions to prevent memory leaks from abandoned flows.
now = int(time.time())
expired_ids = [
sid for sid, s in self._device_sessions.items() if s.expires_at <= now
]
for sid in expired_ids:
self._device_sessions.pop(sid, None)
async with httpx.AsyncClient(There was a problem hiding this comment.
Pull request overview
This PR adds first-class GitHub Copilot support as a built-in provider, including GitHub device-flow OAuth login and a Responses-API-based chat model wrapper to work with Copilot’s newer model endpoints.
Changes:
- Introduces
GitHubCopilotProviderwith device authorization flow, token refresh, dynamic base URL derivation, and Responses API model usage. - Adds
OpenAIResponsesChatModelCompatto route chat requests through/responsesand normalize tool schemas/stream parsing. - Extends provider API/UI to support OAuth login state (start/poll/logout) and display authenticated status; adds unit tests for provider + compat wrapper.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/providers/test_provider_manager.py | Verifies ProviderManager dispatches github-copilot configs to GitHubCopilotProvider. |
| tests/unit/providers/test_openai_responses_chat_model_compat.py | Covers streaming/text/tool-call behavior and tool schema normalization for the Responses wrapper. |
| tests/unit/providers/test_github_copilot_provider.py | Tests device auth flow, token refresh/base-url derivation, model fetch, and Responses-based model checks. |
| src/copaw/providers/provider_manager.py | Registers GitHub Copilot as a built-in provider and enables deserialization dispatch. |
| src/copaw/providers/provider.py | Extends ProviderInfo with OAuth/auth-session state fields and returns them from get_info(). |
| src/copaw/providers/openai_responses_chat_model_compat.py | New Responses-API compatibility model for AgentScope chat usage. |
| src/copaw/providers/github_copilot_provider.py | New provider implementation with device login, token exchange, dynamic base URL, and Responses API model instance. |
| src/copaw/app/routers/providers.py | Adds REST endpoints for device auth start/poll and logout for GitHub Copilot providers. |
| console/src/pages/Settings/Models/components/sections/ModelsSection.tsx | Treats OAuth providers as “configured” only when authenticated. |
| console/src/pages/Settings/Models/components/modals/ProviderConfigModal.tsx | Adds OAuth login UI (device flow) and integrates start/poll/logout API calls. |
| console/src/pages/Settings/Models/components/cards/RemoteProviderCard.tsx | Displays OAuth authorization state instead of API key for OAuth providers. |
| console/src/locales/en.json | Adds GitHub device-flow UI strings. |
| console/src/locales/zh.json | Adds GitHub device-flow UI strings. |
| console/src/locales/ru.json | Adds GitHub device-flow UI strings. |
| console/src/locales/ja.json | Adds GitHub device-flow UI strings. |
| console/src/api/types/provider.ts | Adds OAuth/auth state fields to ProviderInfo and defines device-auth response types. |
| console/src/api/modules/provider.ts | Adds API methods for device auth start/poll and logout. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| provider = _get_github_copilot_provider(manager, provider_id) | ||
| status, message = await provider.poll_device_authorization(session_id) | ||
| manager.update_provider(provider_id, {}) | ||
| provider_info = None | ||
| if status == "authorized": | ||
| provider_info = await manager.get_provider_info(provider_id) | ||
| return DeviceAuthPollResponse( |
There was a problem hiding this comment.
poll_provider_device_auth persists provider state on every poll via manager.update_provider(provider_id, {}), which writes the provider JSON to disk even when the status is still pending. With the UI polling every few seconds this can create unnecessary disk I/O; consider only persisting when state changes (e.g., on authorized / terminal statuses) or when any provider fields were actually updated.
| const timer = window.setTimeout(async () => { | ||
| setAuthPolling(true); | ||
| try { | ||
| const result = await api.pollDeviceAuth(provider.id, authSession.session_id); | ||
| if (cancelled) { | ||
| return; | ||
| } | ||
| if (result.status === "authorized") { | ||
| setAuthSession(null); | ||
| setAuthPolling(false); | ||
| try { | ||
| await api.discoverModels(provider.id); | ||
| } catch { | ||
| // Ignore model discovery failure here; auth already succeeded. | ||
| } | ||
| await onSaved(); | ||
| message.success(result.message || t("models.githubAuthSuccess")); | ||
| return; | ||
| } | ||
| if (result.status === "pending") { | ||
| setAuthPolling(false); | ||
| return; | ||
| } | ||
| setAuthSession(null); | ||
| setAuthPolling(false); | ||
| message.warning(result.message || t("models.githubAuthFailed")); | ||
| } catch (error) { | ||
| if (!cancelled) { | ||
| setAuthPolling(false); | ||
| setAuthSession(null); | ||
| message.error( | ||
| error instanceof Error | ||
| ? error.message | ||
| : t("models.githubAuthFailed"), | ||
| ); | ||
| } | ||
| } | ||
| }, Math.max(authSession.interval, 2) * 1000); | ||
|
|
||
| return () => { | ||
| cancelled = true; | ||
| window.clearTimeout(timer); |
There was a problem hiding this comment.
The device-auth polling effect only schedules a single setTimeout. When the server returns pending, the code clears authPolling and returns without scheduling the next poll, and the effect won’t re-run because authSession hasn’t changed. This will stop polling after the first attempt; consider using a repeating timer (setInterval) or re-scheduling another timeout while status === "pending" (and respecting interval/slow_down).
| const timer = window.setTimeout(async () => { | |
| setAuthPolling(true); | |
| try { | |
| const result = await api.pollDeviceAuth(provider.id, authSession.session_id); | |
| if (cancelled) { | |
| return; | |
| } | |
| if (result.status === "authorized") { | |
| setAuthSession(null); | |
| setAuthPolling(false); | |
| try { | |
| await api.discoverModels(provider.id); | |
| } catch { | |
| // Ignore model discovery failure here; auth already succeeded. | |
| } | |
| await onSaved(); | |
| message.success(result.message || t("models.githubAuthSuccess")); | |
| return; | |
| } | |
| if (result.status === "pending") { | |
| setAuthPolling(false); | |
| return; | |
| } | |
| setAuthSession(null); | |
| setAuthPolling(false); | |
| message.warning(result.message || t("models.githubAuthFailed")); | |
| } catch (error) { | |
| if (!cancelled) { | |
| setAuthPolling(false); | |
| setAuthSession(null); | |
| message.error( | |
| error instanceof Error | |
| ? error.message | |
| : t("models.githubAuthFailed"), | |
| ); | |
| } | |
| } | |
| }, Math.max(authSession.interval, 2) * 1000); | |
| return () => { | |
| cancelled = true; | |
| window.clearTimeout(timer); | |
| let timer: number | undefined; | |
| const schedulePoll = (intervalSeconds: number) => { | |
| const delay = Math.max(intervalSeconds, 2) * 1000; | |
| timer = window.setTimeout(async () => { | |
| if (cancelled) { | |
| return; | |
| } | |
| setAuthPolling(true); | |
| try { | |
| const result = await api.pollDeviceAuth( | |
| provider.id, | |
| authSession.session_id, | |
| ); | |
| if (cancelled) { | |
| return; | |
| } | |
| if (result.status === "authorized") { | |
| setAuthSession(null); | |
| setAuthPolling(false); | |
| try { | |
| await api.discoverModels(provider.id); | |
| } catch { | |
| // Ignore model discovery failure here; auth already succeeded. | |
| } | |
| await onSaved(); | |
| message.success(result.message || t("models.githubAuthSuccess")); | |
| return; | |
| } | |
| if (result.status === "pending") { | |
| setAuthPolling(false); | |
| // Respect server-provided interval/slow_down if available. | |
| let nextInterval = | |
| typeof result.interval === "number" | |
| ? result.interval | |
| : intervalSeconds; | |
| if ((result as any).slow_down) { | |
| nextInterval += 5; | |
| } | |
| if (!cancelled) { | |
| schedulePoll(nextInterval); | |
| } | |
| return; | |
| } | |
| setAuthSession(null); | |
| setAuthPolling(false); | |
| message.warning(result.message || t("models.githubAuthFailed")); | |
| } catch (error) { | |
| if (!cancelled) { | |
| setAuthPolling(false); | |
| setAuthSession(null); | |
| message.error( | |
| error instanceof Error | |
| ? error.message | |
| : t("models.githubAuthFailed"), | |
| ); | |
| } | |
| } | |
| }, delay); | |
| }; | |
| schedulePoll(authSession.interval); | |
| return () => { | |
| cancelled = true; | |
| if (typeof timer === "number") { | |
| window.clearTimeout(timer); | |
| } |
| ) -> DeviceAuthStartResponse: | ||
| provider = _get_github_copilot_provider(manager, provider_id) | ||
| session = await provider.start_device_authorization() | ||
| manager.update_provider(provider_id, {}) |
There was a problem hiding this comment.
start_provider_device_auth calls manager.update_provider(provider_id, {}) immediately after starting the device flow, but start_device_authorization() only updates the in-memory _device_sessions (a PrivateAttr that won’t be persisted anyway). This write appears redundant and causes an extra disk write; consider removing it or persisting only meaningful state.
| manager.update_provider(provider_id, {}) |
| provider = _get_github_copilot_provider(manager, provider_id) | ||
| status, message = await provider.poll_device_authorization(session_id) | ||
| manager.update_provider(provider_id, {}) | ||
| provider_info = None | ||
| if status == "authorized": | ||
| provider_info = await manager.get_provider_info(provider_id) | ||
| return DeviceAuthPollResponse( |
There was a problem hiding this comment.
These endpoints call manager.update_provider(provider_id, {}) after mutating the in-memory GitHubCopilotProvider. This will persist all pydantic fields (including github_oauth_token / copilot_access_token) to disk via provider.model_dump(). Currently, builtin providers are only rehydrated with api_key, extra_models, and generate_kwargs in ProviderManager._init_from_storage, so the persisted OAuth/session fields won’t be restored after restart—while still writing long-lived secrets to disk. Consider either (a) explicitly excluding OAuth/session fields from persistence, or (b) updating builtin-provider rehydration to restore the needed auth fields (and then limiting writes to state changes).
- Implemented GitHubCopilotProvider to support device authorization and OAuth login. - Enhanced ProviderInfo model to include authentication state and OAuth support fields. - Added tests for GitHubCopilotProvider covering device authorization, polling, and model fetching. - Updated ProviderManager to register GitHub Copilot as a built-in provider. - Created compatibility tests for OpenAIResponsesChatModelCompat to ensure proper functionality with GitHub Copilot.
alex-spacemit
force-pushed
the
add-github-copilot-fixed
branch
from
1095f43 to
aaa44dd
Compare
…ng and state management
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| import json | ||
| from datetime import datetime | ||
| from types import SimpleNamespace |
There was a problem hiding this comment.
SimpleNamespace is imported but not used in this module. Please remove the unused import to keep linting clean.
| from types import SimpleNamespace |
|
|
||
| import httpx | ||
| from openai import APIError, AsyncOpenAI | ||
| from pydantic import BaseModel, Field, PrivateAttr |
There was a problem hiding this comment.
Field is imported from pydantic but not used in this file. Please remove the unused import.
| from pydantic import BaseModel, Field, PrivateAttr | |
| from pydantic import BaseModel, PrivateAttr |
| class GitHubCopilotProvider(OpenAIProvider): | ||
| """OpenAI-compatible provider backed by GitHub Copilot.""" | ||
|
|
||
| supports_oauth_login: bool = True | ||
| auth_method: str | None = "github-device" | ||
| is_authenticated: bool = False | ||
| auth_account_label: str | None = None | ||
| auth_expires_at: int | None = None |
There was a problem hiding this comment.
GitHubCopilotProvider inherits OpenAIProvider.probe_model_multimodal(), which probes via /chat/completions. For Copilot you’re explicitly routing chat traffic through /responses, so the inherited probe is likely to fail and then persist supports_image=False/supports_video=False for discovered models (since fetch_models() returns ModelInfo with supports_multimodal=None, triggering the auto-probe in ProviderManager.maybe_probe_multimodal). Consider overriding probe_model_multimodal() for this provider (or disabling auto-probe for it) so Copilot models aren’t incorrectly marked as non-multimodal due to an endpoint mismatch.
| {provider.support_connection_check && | ||
| (!isOauthProvider || provider.is_authenticated) && ( | ||
| <Button |
There was a problem hiding this comment.
The footer correctly hides the “Test connection” button until an OAuth provider is authenticated, but handleSubmit() still performs a connection test whenever provider.support_connection_check is true. For OAuth providers this can prevent saving non-auth settings (e.g. generate_kwargs) before login, because /models/{id}/test will fail with “GitHub authorization required”. Consider aligning handleSubmit() with this same (!isOauthProvider || provider.is_authenticated) gating (or otherwise skipping the connection test when saving config for unauthenticated OAuth providers).
| return { | ||
| **schema, | ||
| "anyOf": [schema, {"type": "null"}], | ||
| } |
There was a problem hiding this comment.
_make_optional_schema_nullable() builds an invalid schema for oneOf: it adds an anyOf field that contains the entire schema dict (including the existing oneOf) rather than appending {"type":"null"} to the oneOf variants. This can produce schemas with both oneOf and anyOf and may even self-reference, which can break tool schema validation downstream. Consider converting oneOf -> anyOf by copying the oneOf list (plus {type:null}) and removing the original oneOf key (or otherwise ensuring no recursion / mixed combinators).
| return { | |
| **schema, | |
| "anyOf": [schema, {"type": "null"}], | |
| } | |
| # Convert oneOf variants to anyOf and append a null variant, | |
| # removing the original oneOf key to avoid mixed combinators. | |
| new_schema = {**schema} | |
| new_schema.pop("oneOf", None) | |
| new_schema["anyOf"] = [*one_of, {"type": "null"}] | |
| return new_schema |
|
👍，我还没时间测试。另一个小问题：是否考虑了支持凭证序列化，这样重启服务之后不需要再做一次认证 |
|
It seems this implementation is based on reverse-engineered internal APIs. Is there any risk of triggering anti-abuse systems or resulting in account restriction? Should we consider using official Copilot SDK instead? |
Description
[Describe what this PR does and why]
Related Issue: Fixes #(issue_number) or Relates to #(issue_number)
Security Considerations: [If applicable, e.g. channel auth, env/config handling]
Type of Change
Component(s) Affected
Checklist
pre-commit run --all-fileslocally and it passespytestor as relevant) and they passTesting