-
Notifications
You must be signed in to change notification settings - Fork 16
feat(provider): Add GitHub Copilot as an OAuth Provider #30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Implements GitHub Copilot integration with: - Device Flow OAuth authentication (no browser redirect needed) - Custom Copilot Chat API integration (bypasses LiteLLM) - Configurable X-Initiator header control (user vs agent mode) - Support for both github.com and GitHub Enterprise - Vision request support - Streaming and non-streaming responses Key features from the referenced implementations: - Based on https://github.com/sst/opencode-copilot-auth for OAuth flow - Agent header forcing feature from Tarquinen/dotfiles plugin - COPILOT_AGENT_PERCENTAGE env var for random user/agent ratio New files: - copilot_auth_base.py: GitHub Device Flow OAuth handler - copilot_provider.py: Custom API integration Closes #29
|
Ah, time to review my own handiwork on the GitHub Copilot provider! Let's see what past-me was thinking when putting together 1200+ lines of OAuth magic and API integration. Diving in now... 🔍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self-Review Assessment
Well, well, well... time to see what past-me was thinking when putting together this 1200+ line Copilot provider. Overall, I'm pleasantly surprised - the implementation is solid, well-documented, and follows the existing patterns in the codebase. But of course, I found a few "what was I thinking?" moments.
The Good:
- Clean separation between
CopilotAuthBase(authentication) andCopilotProvider(API integration) - Proper Device Flow OAuth implementation with nice user experience (Rich console output)
- X-Initiator header logic correctly implements the referenced feature request
- Atomic file writes for credential storage - past-me was thinking about reliability
- GitHub Enterprise support is a nice touch
The "Past-Me, Why?":
- Some dead code and unused variables lurking around (queue system that was never implemented, orphaned imports)
- Inconsistent version numbers in User-Agent headers - pick one!
- A confusing loop that does nothing but
pass
Architectural Reflections
The two-class design (CopilotAuthBase + CopilotProvider) makes sense given the complexity of Copilot's two-token system. However, I should consider if some of the queue/backoff tracking code in CopilotAuthBase is actually needed or if it was copy-pasted from another provider without full implementation.
Key Fixes I Should Make
- Clean up dead code: Remove unused queue system variables or implement them
- Standardize version numbers: Use consistent User-Agent versions across all requests
- Remove confusing no-op code: That vision check loop that does nothing
- Improve bare except clauses: Use
except OSError:instead of bareexcept: - Address credential naming mismatch: The env var naming between credential_manager and copilot_auth_base could cause confusion
This self-review was generated by an AI assistant.
| try: | ||
| os.close(tmp_fd) | ||
| except: | ||
| pass | ||
| if tmp_path and os.path.exists(tmp_path): | ||
| try: | ||
| os.unlink(tmp_path) | ||
| except: | ||
| pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, past-me used bare except: clauses here. While I get the intention (cleanup should not fail loudly), this catches everything including KeyboardInterrupt and SystemExit. A more specific except OSError: would be safer.
| try: | |
| os.close(tmp_fd) | |
| except: | |
| pass | |
| if tmp_path and os.path.exists(tmp_path): | |
| try: | |
| os.unlink(tmp_path) | |
| except: | |
| pass | |
| if tmp_fd is not None: | |
| try: | |
| os.close(tmp_fd) | |
| except OSError: | |
| pass | |
| if tmp_path and os.path.exists(tmp_path): | |
| try: | |
| os.unlink(tmp_path) | |
| except OSError: | |
| pass |
| # [BACKOFF TRACKING] Track consecutive failures per credential | ||
| self._refresh_failures: Dict[str, int] = {} | ||
| self._next_refresh_after: Dict[str, float] = {} | ||
|
|
||
| # [QUEUE SYSTEM] Sequential refresh processing | ||
| self._refresh_queue: asyncio.Queue = asyncio.Queue() | ||
| self._queued_credentials: set = set() | ||
| self._unavailable_credentials: set = set() | ||
| self._queue_tracking_lock = asyncio.Lock() | ||
| self._queue_processor_task: Optional[asyncio.Task] = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see past-me set up a queue system with _refresh_queue, _queued_credentials, and _queue_processor_task, but... I never actually implemented the queue processor logic anywhere in this file. These instance variables are initialized but appear unused. Was this meant for a future optimization, or did I forget to implement it? Should probably either implement it or remove these dead declarations to avoid confusion.
| # Headers that mimic the official Copilot client | ||
| COPILOT_HEADERS = { | ||
| "User-Agent": "GitHubCopilotChat/0.32.4", | ||
| "Editor-Version": "vscode/1.105.1", | ||
| "Editor-Plugin-Version": "copilot-chat/0.32.4", | ||
| "Copilot-Integration-Id": "vscode-chat", | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent version numbers detected! I used GitHubCopilotChat/0.32.4 and copilot-chat/0.32.4 here in COPILOT_HEADERS, but then used GitHubCopilotChat/0.35.0 in the Device Flow requests (lines 451, 509). Past-me should pick one version and stick with it for consistency - probably the newer 0.35.0 everywhere.
| # Check for vision content in messages | ||
| content = msg.get("content", []) | ||
| if isinstance(content, list): | ||
| for part in content: | ||
| if isinstance(part, dict) and part.get("type") == "image_url": | ||
| pass # Vision doesn't affect initiator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code block does nothing - it loops through image content and then just passes. The comment says "Vision doesn't affect initiator" but if that's the case, why check at all? This is either dead code from an abandoned feature, or I forgot to implement something. Either way, it's confusing for future readers (including future-me). Should be removed or actually implemented.
| "antigravity": "ANTIGRAVITY", | ||
| "qwen_code": "QWEN_CODE", | ||
| "iflow": "IFLOW", | ||
| "copilot": "COPILOT", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heads up: There's a naming mismatch that might confuse users. The credential manager's _discover_env_oauth_credentials() looks for COPILOT_ACCESS_TOKEN and COPILOT_REFRESH_TOKEN patterns, but copilot_auth_base.py primarily uses COPILOT_GITHUB_TOKEN. This means the automatic env credential discovery won't find Copilot credentials unless users also set the _REFRESH_TOKEN variant.
Might want to either:
- Align the naming conventions, or
- Add special handling for Copilot in the discovery logic, or
- Document this clearly in
.env.example
|
@mirrobot-agent continue with the implementation. Make sure to reference the linked source repositories for how it should work. Address any applicable feedback - i expect feature-complete implementation. |
Summary
Implements GitHub Copilot as a new OAuth-based provider, fulfilling feature request #29.
Key Features
COPILOT_FORCE_AGENT_HEADER=true- Always use agent modeCOPILOT_AGENT_PERCENTAGE=50- Random 50% chance of agent mode for first messagesCOPILOT_ENTERPRISE_URLfor self-hosted GitHubImplementation Details
Based on analysis of:
Files Changed
copilot_auth_base.pycopilot_provider.pyprovider_factory.pycredential_manager.pyproviders/__init__.py.env.exampleConfiguration
Default Models
How X-Initiator Works
The X-Initiator header affects Copilot's response behavior:
The logic:
COPILOT_AGENT_PERCENTAGEto randomly decideRelated Issue
Closes #29
This pull request was automatically generated by mirrobot-agent.
Important
Adds GitHub Copilot as an OAuth provider with API integration, supporting Device Flow authentication and custom response handling.
copilot_auth_base.py.copilot_provider.py, supporting both streaming and non-streaming responses..env.examplewith Copilot-specific variables for OAuth tokens, enterprise URL, and model configuration.credential_manager.pyto handle Copilot credentials from environment variables and local files.provider_factory.pyandproviders/__init__.py.This description was created by
for e65a71e. You can customize this summary. It will automatically update as commits are pushed.