Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for GitHub Copilot as a provider by implementing device flow login, token persistence, a dedicated Copilot client, and integrating it into the CLI and agent loop.
- Introduced
GithubCopilotClientfor handling Copilot API authentication and requests - Added device-flow login in
get-api-keyand updated CLI to persistGITHUBCOPILOT_API_KEY - Registered the new provider in
providers.tsand initialized the client inagent-loop.ts
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| codex-cli/src/utils/providers.ts | Updated provider list with GitHub Copilot entry |
| codex-cli/src/utils/openai-client.ts | Added GithubCopilotClient class and token-management logic |
| codex-cli/src/utils/get-api-key.tsx | Implemented getGithubCopilotApiKey for device-flow login |
| codex-cli/src/utils/agent/agent-loop.ts | Initialized GithubCopilotClient in the agent loop |
| codex-cli/src/cli.tsx | Integrated Copilot login flow, auth file persistence, and env handling |
Comments suppressed due to low confidence (3)
codex-cli/src/utils/providers.ts:56
- [nitpick] Use the official branding 'GitHubCopilot' (capital 'H' in 'Hub') for the provider name to match GitHub’s naming.
name: "GithubCopilot",
codex-cli/src/utils/providers.ts:58
- [nitpick] Consider using 'GITHUB_COPILOT_API_KEY' with an underscore for readability and to stay consistent with other environment variable names.
envKey: "GITHUBCOPILOT_API_KEY",
codex-cli/src/utils/providers.ts:57
- Verify that 'https://api.githubcopilot.com' is the correct API endpoint; the official Copilot API may use a different host or path.
baseURL: "https://api.githubcopilot.com",
| >(headers: T): T { | ||
| const copy = { ...headers } as Record<string, string> & T; | ||
| copy["User-Agent"] = "GithubCopilot/1.155.0"; | ||
| copy["editor-version"] = "vscode/1.85.1"; |
There was a problem hiding this comment.
[nitpick] The 'editor-version' header is hardcoded to a VSCode plugin version; consider reflecting the CLI’s version or making this header configurable.
| } | ||
|
|
||
| if (cli.flags.login) { | ||
| if (provider.toLowerCase() === "githubcopilot" && !apiKey) { |
There was a problem hiding this comment.
This block triggers the Copilot login flow unconditionally when no key is present, even without the '--login' flag; consider guarding it under 'cli.flags.login' to avoid unexpected prompts.
| if (provider.toLowerCase() === "githubcopilot" && !apiKey) { | |
| if (provider.toLowerCase() === "githubcopilot" && !apiKey && cli.flags.login) { |
| }); | ||
| } | ||
|
|
||
| if (this.provider.toLowerCase() === "githubcopilot") { |
There was a problem hiding this comment.
[nitpick] The GitHub Copilot client initialization mirrors logic in 'createOpenAIClient'; consider refactoring to reuse the factory for consistency and to reduce duplication.
Summary
GithubCopilotClientfor Copilot API requestsgithubcopilotprovider configurationTesting
pnpm --filter @openai/codex run formatpnpm --filter @openai/codex run lintpnpm --filter @openai/codex run test(fails: Process with PID XXXX failed to terminate within 500ms)https://chatgpt.com/codex/tasks/task_e_684f272bac248326a07facb43efee33f