-
-
Notifications
You must be signed in to change notification settings - Fork 294
test(copilot): add comprehensive test coverage [5/5] #384
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
Conversation
- Add auth directory management helper (GetAuthDir) - Add random hex string generator for request IDs - Add helper for generating unique machine identifiers
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Summary of ChangesHello @jeffnash, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request represents the final phase (5/5) of the GitHub Copilot feature stack, focusing on solidifying the integration with comprehensive test coverage. It introduces the full implementation of the Copilot authentication flow, API request handling, and model management, alongside robust unit tests to ensure the reliability and correctness of the new functionality. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request introduces a comprehensive integration for GitHub Copilot, including the complex device code authentication flow, management API endpoints, and the request executor. The implementation is very thorough, with good error handling, caching mechanisms, and support for various Copilot features like different account types and vision models. The addition of extensive tests for the new functionality is also a great contribution.
I've found a couple of critical copy-paste errors that would prevent compilation, and a minor issue in the example configuration. After addressing these, the feature should be in great shape.
internal/config/config.go
Outdated
| // CopilotKey represents the configuration for GitHub Copilot API access. | ||
| // Authentication is handled via device code OAuth flow, not API keys. | ||
| type CopilotKey struct { | ||
| // AccountType is the Copilot subscription type (individual, business, enterprise). | ||
| // Defaults to "individual" if not specified. | ||
| AccountType string `yaml:"account-type" json:"account-type"` | ||
|
|
||
| // ProxyURL overrides the global proxy setting for Copilot requests if provided. | ||
| ProxyURL string `yaml:"proxy-url,omitempty" json:"proxy-url,omitempty"` | ||
|
|
||
| // AgentInitiatorPersist, when true, forces subsequent Copilot requests sharing the | ||
| // same prompt_cache_key to send X-Initiator=agent after the first call. Default false. | ||
| AgentInitiatorPersist bool `yaml:"agent-initiator-persist" json:"agent-initiator-persist"` | ||
| } | ||
|
|
||
| // CopilotKey represents the configuration for GitHub Copilot API access. | ||
| // Authentication is handled via device code OAuth flow, not API keys. | ||
| type CopilotKey struct { | ||
| // AccountType is the Copilot subscription type (individual, business, enterprise). | ||
| // Defaults to "individual" if not specified. | ||
| AccountType string `yaml:"account-type" json:"account-type"` | ||
|
|
||
| // ProxyURL overrides the global proxy setting for Copilot requests if provided. | ||
| ProxyURL string `yaml:"proxy-url,omitempty" json:"proxy-url,omitempty"` | ||
|
|
||
| // AgentInitiatorPersist, when true, forces subsequent Copilot requests sharing the | ||
| // same prompt_cache_key to send X-Initiator=agent after the first call. Default false. | ||
| AgentInitiatorPersist bool `yaml:"agent-initiator-persist" json:"agent-initiator-persist"` | ||
| } |
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.
| } | ||
| accountType = parsed | ||
| } | ||
| } |
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.
config.example.yaml
Outdated
| # GitHub Copilot account configuration | ||
| # Note: Copilot uses OAuth device code authentication, NOT API keys or tokens. | ||
| # Do NOT paste your GitHub access token or Copilot bearer token here. | ||
| # Tokens are stored only in auth-dir JSON files, never in config.yaml. | ||
| # | ||
| # To authenticate: | ||
| # - CLI: run with -copilot-login flag | ||
| # - Web: use the /copilot-auth-url management endpoint | ||
| # | ||
| # After OAuth login, tokens are managed automatically and stored in auth-dir. | ||
| # The entries below only configure account type and optional proxy settings. | ||
| #copilot-api-key: | ||
| # - account-type: "individual" # Options: individual, business, enterprise | ||
| # proxy-url: "socks5://proxy.example.com:1080" # optional: proxy for Copilot requests | ||
|
|
||
| # # When set to true, this flag forces subsequent requests in a session (sharing the same prompt_cache_key) | ||
| # # to send the header "X-Initiator: agent" instead of "vscode". This mirrors VS Code's behavior for | ||
| # # long-running agent interactions and helps prevent hitting standard rate limits. | ||
| # agent-initiator-persist: true | ||
|
|
||
| # GitHub Copilot account configuration | ||
| # Note: Copilot uses OAuth device code authentication, NOT API keys or tokens. | ||
| # Do NOT paste your GitHub access token or Copilot bearer token here. | ||
| # Tokens are stored only in auth-dir JSON files, never in config.yaml. | ||
| # | ||
| # To authenticate: | ||
| # - CLI: run with -copilot-login flag | ||
| # - Web: use the /copilot-auth-url management endpoint | ||
| # | ||
| # After OAuth login, tokens are managed automatically and stored in auth-dir. | ||
| # The entries below only configure account type and optional proxy settings. | ||
| #copilot-api-key: | ||
| # - account-type: "individual" # Options: individual, business, enterprise | ||
| # proxy-url: "socks5://proxy.example.com:1080" # optional: proxy for Copilot requests | ||
|
|
||
| # # When set to true, this flag forces subsequent requests in a session (sharing the same prompt_cache_key) | ||
| # # to send the header "X-Initiator: agent" instead of "vscode". This mirrors VS Code's behavior for | ||
| # # long-running agent interactions and helps prevent hitting standard rate limits. | ||
| # agent-initiator-persist: true | ||
|
|
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.
| "client_id": GitHubClientID, | ||
| "scope": GitHubAppScopes, | ||
| } | ||
| bodyBytes, _ := json.Marshal(reqBody) |
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.
The error returned by json.Marshal is being ignored. While it's unlikely to fail with a map[string]string as input, it's good practice to handle potential errors. This also applies to a similar case on line 160.
bodyBytes, err := json.Marshal(reqBody)
if err != nil {
return nil, fmt.Errorf("%w: failed to marshal request body: %v", ErrDeviceCodeFailed, err)
}- Add Copilot section with agent-initiator-persist flag - Add scanner buffer size configuration - Add account type configuration option - Add copilot types with account type validation - Document configuration options in example config - Add tests for util and copilot types
- Add device-code OAuth flow with GitHub token exchange - Implement Copilot token acquisition and refresh logic - Add account type handling (individual/business/enterprise) - Add token persistence and storage management - Add CLI login command (cliproxy-api copilot login) - Register Copilot refresh handler in SDK - Validate account_type with warning for invalid values
acc7560 to
5967f1c
Compare
- Implement Copilot executor with header injection - Add VS Code version headers and integration IDs - Add agent header logic (X-Initiator detection) - Add vision request header for image inputs - Add static model registry with raptor-mini and oswe-vscode-prime - Add management API endpoints for auth files Note: Full model list and dynamic fetching added in gemini branch
…tching - Add full Copilot model registry (all GPT, Claude, Gemini, Grok models) - Implement dynamic model fetching from Copilot API with caching - Add Gemini reasoning capture and injection for tool calls - Add reasoning_opaque and reasoning_text handling for Gemini 3 models - Evict model and reasoning caches on auth removal - Add 30-second retry delay for 429 quota errors Credit: Reverse engineering insights adapted from github.com/aadishv/vscre
- Add executor tests for token handling and refresh logic - Add header tests for X-Initiator and vision detection - Add tests for account type validation and HTTP status errors - Add tests for model registry and alias generation
5967f1c to
737173a
Compare
Summary
Adds comprehensive test coverage for the Copilot integration.
Changes
Stack
This is PR 5/5 in the Copilot feature stack:
Depends on: #383 - merge that first