-
Notifications
You must be signed in to change notification settings - Fork 614
feat: enhance GitHub Copilot Device Flow with OAuth token management and API token retrieval #1021
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
…and API token retrieval - Fixed request header for managing OAuth tokens and retrieving API tokens. - Enhanced model definitions and added new models for better compatibility.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRefactors GitHub Copilot authentication: adds token exchange and validation interfaces/methods, expands device-flow polling/error handling, introduces a global singleton device flow with disposal, and updates provider and OAuth presenters to use the singleton and emit UI refresh events. Changes
Sequence Diagram(s)sequenceDiagram
participant Frontend
participant OAuthPresenter
participant DeviceFlow
participant GithubCopilotProvider
participant TokenAPI
rect rgb(230, 240, 255)
note over Frontend,OAuthPresenter: Device Flow login with pre-check
Frontend->>OAuthPresenter: initiate login
OAuthPresenter->>DeviceFlow: getGlobalGitHubCopilotDeviceFlow()
OAuthPresenter->>DeviceFlow: checkExistingAuth(apiKey?)
alt token exists
DeviceFlow-->>OAuthPresenter: oauth token
OAuthPresenter->>GithubCopilotProvider: save token & persist
else token missing/invalid
OAuthPresenter->>DeviceFlow: startDeviceFlow()
DeviceFlow->>TokenAPI: request device code
TokenAPI-->>DeviceFlow: device_code + interval
DeviceFlow->>DeviceFlow: poll for authorization (up to 100 attempts)
DeviceFlow->>TokenAPI: exchange code -> oauth token
TokenAPI-->>DeviceFlow: oauth token
DeviceFlow-->>OAuthPresenter: oauth token
end
OAuthPresenter->>GithubCopilotProvider: getCopilotToken()
alt device flow available
GithubCopilotProvider->>DeviceFlow: use oauth token to get API token
else fallback
GithubCopilotProvider->>TokenAPI: use API key flow
end
GithubCopilotProvider-->>OAuthPresenter: api token
OAuthPresenter->>Frontend: emit providerUpdated
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts (1)
378-383: Stop logging Authorization credentials
console.log('Headers:', headers)prints the rawAuthorization: Bearer ...token to stdout. That is a direct credential leak, violates the “avoid logging sensitive info” guideline, and would exfiltrate users’ Copilot API tokens in any log sink.As per coding guidelines, redact or drop the header logging:
- console.log(` Headers:`, headers) - console.log(` Request Body:`, JSON.stringify(requestBody, null, 2)) + const { Authorization, ...safeHeaders } = headers + console.log(` Headers:`, { ...safeHeaders, Authorization: '[redacted]' }) + console.log( + ` Request Body:`, + JSON.stringify( + { + ...requestBody, + messages: '[omitted]' + }, + null, + 2 + ) + )Also applies to: 540-545
src/main/presenter/githubCopilotDeviceFlow.ts (1)
65-74: Persist the OAuth token after device flow
startDeviceFlowreturns the access token but never assigns it tothis.oauthToken. As a resultgetCopilotToken()immediately throws “No OAuth token available” on every call, forcing all callers to fall back to less robust paths and spamming warning logs. Persist the token when it is issued.- const accessToken = await this.pollForAccessToken(deviceCodeResponse) - - return accessToken + const accessToken = await this.pollForAccessToken(deviceCodeResponse) + this.oauthToken = accessToken + + return accessToken
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/presenter/githubCopilotDeviceFlow.ts(9 hunks)src/main/presenter/llmProviderPresenter/index.ts(1 hunks)src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts(11 hunks)src/main/presenter/oauthPresenter.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/githubCopilotDeviceFlow.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/oauthPresenter.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/githubCopilotDeviceFlow.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/oauthPresenter.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/githubCopilotDeviceFlow.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/oauthPresenter.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪(如适用)、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/githubCopilotDeviceFlow.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/oauthPresenter.ts
src/main/presenter/llmProviderPresenter/index.ts
📄 CodeRabbit inference engine (.cursor/rules/llm-agent-loop.mdc)
src/main/presenter/llmProviderPresenter/index.ts:src/main/presenter/llmProviderPresenter/index.tsshould manage the overall Agent loop, conversation history, tool execution viaMcpPresenter, and frontend communication viaeventBus.
The main Agent loop inllmProviderPresenter/index.tsshould handle multi-round LLM calls and tool usage, maintaining conversation state and controlling the loop withneedContinueConversationandtoolCallCount.
The main Agent loop should send standardizedSTREAM_EVENTS(RESPONSE,END,ERROR) to the frontend viaeventBus.
The main Agent loop should buffer text content, handle tool call events, format tool results for the next LLM call, and manage conversation continuation logic.
Files:
src/main/presenter/llmProviderPresenter/index.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/githubCopilotDeviceFlow.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/oauthPresenter.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/githubCopilotDeviceFlow.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/oauthPresenter.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Use PascalCase for TypeScript types and classes
Files:
src/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/githubCopilotDeviceFlow.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/oauthPresenter.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place Electron main-process presenters under src/main/presenter/ (Window, Tab, Thread, Mcp, Config, LLMProvider)
Files:
src/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/githubCopilotDeviceFlow.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/oauthPresenter.ts
**/*.{ts,tsx,js,jsx,vue,css,scss,md,json,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Prettier style: single quotes, no semicolons, print width 100; run pnpm run format
Files:
src/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/githubCopilotDeviceFlow.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/oauthPresenter.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,vue}: Use OxLint for JS/TS code; keep lint clean
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE for constants
Files:
src/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/githubCopilotDeviceFlow.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.tssrc/main/presenter/oauthPresenter.ts
src/main/presenter/llmProviderPresenter/providers/*.ts
📄 CodeRabbit inference engine (.cursor/rules/llm-agent-loop.mdc)
src/main/presenter/llmProviderPresenter/providers/*.ts: Each file insrc/main/presenter/llmProviderPresenter/providers/*.tsshould handle interaction with a specific LLM API, including request/response formatting, tool definition conversion, native/non-native tool call management, and standardizing output streams to a common event format.
Provider implementations must use acoreStreammethod that yields standardized stream events to decouple the main loop from provider-specific details.
ThecoreStreammethod in each Provider must perform a single streaming API request per conversation round and must not contain multi-round tool call loop logic.
Provider files should implement helper methods such asformatMessages,convertToProviderTools,parseFunctionCalls, andprepareFunctionCallPromptas needed for provider-specific logic.
All provider implementations must parse provider-specific data chunks and yield standardized events for text, reasoning, tool calls, usage, errors, stop reasons, and image data.
When a provider does not support native function calling, it must prepare messages using prompt wrapping (e.g.,prepareFunctionCallPrompt) before making the API call.
When a provider supports native function calling, MCP tools must be converted to the provider's format (e.g., usingconvertToProviderTools) and included in the API request.
Provider implementations should aggregate and yield usage events as part of the standardized stream.
Provider implementations should yield image data events in the standardized format when applicable.
Provider implementations should yield reasoning events in the standardized format when applicable.
Provider implementations should yield tool call events (tool_call_start,tool_call_chunk,tool_call_end) in the standardized format.
Provider implementations should yield stop events with appropriatestop_reasonin the standardized format.
Provider implementations should yield error events in the standardized format...
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
🧠 Learnings (3)
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Provider implementations should yield error events in the standardized format when errors occur.
Applied to files:
src/main/presenter/llmProviderPresenter/index.ts
📚 Learning: 2025-10-14T08:02:59.495Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: AGENTS.md:0-0
Timestamp: 2025-10-14T08:02:59.495Z
Learning: Applies to src/main/presenter/LLMProvider/**/*.ts : Implement the two-layer LLM provider (Agent Loop + Provider) under src/main/presenter/LLMProvider
Applied to files:
src/main/presenter/llmProviderPresenter/index.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Provider implementations must use a `coreStream` method that yields standardized stream events to decouple the main loop from provider-specific details.
Applied to files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
🧬 Code graph analysis (3)
src/main/presenter/llmProviderPresenter/index.ts (1)
scripts/fetch-provider-db.mjs (1)
error(14-14)
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts (2)
src/main/presenter/githubCopilotDeviceFlow.ts (2)
GitHubCopilotDeviceFlow(44-588)getGlobalGitHubCopilotDeviceFlow(619-624)src/shared/types/presenters/legacy.presenters.d.ts (2)
LLM_PROVIDER(573-593)MODEL_META(558-572)
src/main/presenter/oauthPresenter.ts (4)
src/main/presenter/githubCopilotDeviceFlow.ts (1)
getGlobalGitHubCopilotDeviceFlow(619-624)src/main/presenter/index.ts (1)
presenter(223-223)src/main/eventbus.ts (1)
eventBus(151-151)src/main/presenter/githubCopilotOAuth.ts (1)
createGitHubCopilotOAuth(224-280)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-check (x64)
|
@codex review |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| // Increase polling interval | ||
| if (this.pollingInterval) { | ||
| clearInterval(this.pollingInterval) | ||
| this.pollingInterval = setInterval(poll, (deviceCodeResponse.interval + 5) * 1000) | ||
| this.pollingInterval = setInterval(poll, currentInterval * 1000) |
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.
Increase polling delay when GitHub responds with
slow_down
The new currentInterval variable is never updated before being reapplied when the device authorization endpoint returns slow_down. As a result the polling interval stays at the original deviceCodeResponse.interval, violating the OAuth device flow requirement to back off by at least 5 s when a rate-limit warning is returned. In practice this will continue hammering the endpoint at the same rate, making repeated slow_down responses likely and causing the flow to hit the new 100-attempt limit or let the device code expire without ever retrieving an access token. Increment currentInterval before rescheduling so the retry delay actually increases.
Useful? React with 👍 / 👎.
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 comment here on codex is correct. Upon receiving slow_down, the timer is only reset using the original value of currentInterval, but currentInterval is never updated, failing to implement "backoff." When being rate-limited, requests are still made at the original interval, potentially continuously triggering rate limiting until timeout, resulting in some users still being unable to complete device stream login.
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.
done
| protected async fetchProviderModels(): Promise<MODEL_META[]> { | ||
| // Use aggregated Provider DB as authoritative source for model list | ||
| const dbModels = this.configPresenter.getDbProviderModels(this.provider.id).map((m) => ({ | ||
| id: m.id, | ||
| name: m.name, | ||
| group: m.group || 'GitHub Copilot', | ||
| providerId: this.provider.id, | ||
| isCustom: false, | ||
| contextLength: m.contextLength, | ||
| maxTokens: m.maxTokens, | ||
| vision: m.vision || false, | ||
| functionCall: m.functionCall || false, | ||
| reasoning: m.reasoning || false, | ||
| ...(m.type ? { type: m.type } : {}) | ||
| })) | ||
| // GitHub Copilot 支持的模型列表 | ||
| const models: MODEL_META[] = [ | ||
| { | ||
| id: 'gpt-5', | ||
| name: 'GPT-5', | ||
| group: 'GitHub Copilot', | ||
| providerId: this.provider.id, | ||
| isCustom: false, | ||
| contextLength: 128000, | ||
| maxTokens: 8192, | ||
| vision: true, | ||
| functionCall: true, | ||
| reasoning: false | ||
| }, | ||
| { | ||
| id: 'gpt-5-mini', | ||
| name: 'GPT-5 mini', | ||
| group: 'GitHub Copilot', | ||
| providerId: this.provider.id, | ||
| isCustom: false, | ||
| contextLength: 128000, | ||
| maxTokens: 16384, | ||
| vision: true, | ||
| functionCall: true, | ||
| reasoning: false | ||
| }, | ||
| { | ||
| id: 'gpt-4.1', | ||
| name: 'GPT-4.1', | ||
| group: 'GitHub Copilot', | ||
| providerId: this.provider.id, | ||
| isCustom: false, | ||
| contextLength: 64000, | ||
| maxTokens: 4096, | ||
| vision: true, | ||
| functionCall: true, | ||
| reasoning: false | ||
| }, | ||
| { | ||
| id: 'gpt-4o-2024-05-13', | ||
| name: 'GPT-4o', | ||
| group: 'GitHub Copilot', | ||
| providerId: this.provider.id, | ||
| isCustom: false, | ||
| contextLength: 64000, | ||
| maxTokens: 4096, | ||
| vision: true, | ||
| functionCall: true, | ||
| reasoning: false | ||
| }, | ||
| { | ||
| id: 'gpt-4', | ||
| name: 'GPT-4', | ||
| group: 'GitHub Copilot', | ||
| providerId: this.provider.id, | ||
| isCustom: false, | ||
| contextLength: 32768, | ||
| maxTokens: 4096, | ||
| vision: false, | ||
| functionCall: true, | ||
| reasoning: false | ||
| }, | ||
| { | ||
| id: 'gpt-3.5-turbo', | ||
| name: 'GPT-3.5 Turbo', | ||
| group: 'GitHub Copilot', | ||
| providerId: this.provider.id, | ||
| isCustom: false, | ||
| contextLength: 12288, | ||
| maxTokens: 4096, | ||
| vision: false, | ||
| functionCall: true, | ||
| reasoning: false | ||
| }, | ||
| { | ||
| id: 'o1', | ||
| name: 'o1', | ||
| group: 'GitHub Copilot', | ||
| providerId: this.provider.id, | ||
| isCustom: false, | ||
| contextLength: 20000, | ||
| maxTokens: 32768, | ||
| vision: false, | ||
| functionCall: false, | ||
| reasoning: true | ||
| }, | ||
| { | ||
| id: 'o3-mini', | ||
| name: 'o3-mini', | ||
| group: 'GitHub Copilot', | ||
| providerId: this.provider.id, | ||
| isCustom: false, | ||
| contextLength: 64000, | ||
| maxTokens: 65536, | ||
| vision: false, | ||
| functionCall: false, | ||
| reasoning: true | ||
| }, | ||
| { | ||
| id: 'claude-sonnet-4', | ||
| name: 'Claude Sonnet 4', | ||
| group: 'GitHub Copilot', | ||
| providerId: this.provider.id, | ||
| isCustom: false, | ||
| contextLength: 200000, | ||
| maxTokens: 8192, | ||
| vision: true, | ||
| functionCall: true, | ||
| reasoning: false | ||
| }, | ||
| { | ||
| id: 'claude-sonnet-4.5', | ||
| name: 'Claude Sonnet 4.5', | ||
| group: 'GitHub Copilot', | ||
| providerId: this.provider.id, | ||
| isCustom: false, | ||
| contextLength: 200000, | ||
| maxTokens: 8192, | ||
| vision: true, | ||
| functionCall: true, | ||
| reasoning: false | ||
| }, | ||
| { | ||
| id: 'claude-3.5-sonnet', | ||
| name: 'Claude Sonnet 3.5', | ||
| group: 'GitHub Copilot', | ||
| providerId: this.provider.id, | ||
| isCustom: false, | ||
| contextLength: 200000, | ||
| maxTokens: 8192, | ||
| vision: true, | ||
| functionCall: true, | ||
| reasoning: false | ||
| }, | ||
| { | ||
| id: 'claude-3.7-sonnet', | ||
| name: 'Claude Sonnet 3.7', | ||
| group: 'GitHub Copilot', | ||
| providerId: this.provider.id, | ||
| isCustom: false, | ||
| contextLength: 90000, | ||
| maxTokens: 8192, | ||
| vision: true, | ||
| functionCall: true, | ||
| reasoning: false | ||
| }, | ||
| { | ||
| id: 'claude-3.7-sonnet-thought', | ||
| name: 'Claude Sonnet 3.7 Thinking', | ||
| group: 'GitHub Copilot', | ||
| providerId: this.provider.id, | ||
| isCustom: false, | ||
| contextLength: 90000, | ||
| maxTokens: 8192, | ||
| vision: true, | ||
| functionCall: true, | ||
| reasoning: false | ||
| }, | ||
| { | ||
| id: 'gemini-2.5-pro', | ||
| name: 'Gemini 2.5 Pro', | ||
| group: 'GitHub Copilot', | ||
| providerId: this.provider.id, | ||
| isCustom: false, | ||
| contextLength: 128000, | ||
| maxTokens: 8192, | ||
| vision: true, | ||
| functionCall: true, | ||
| reasoning: false | ||
| }, | ||
| { | ||
| id: 'gemini-2.0-flash-001', | ||
| name: 'Gemini 2.0 Flash', | ||
| group: 'GitHub Copilot', | ||
| providerId: this.provider.id, | ||
| isCustom: false, | ||
| contextLength: 128000, | ||
| maxTokens: 8192, | ||
| vision: true, | ||
| functionCall: true, | ||
| reasoning: false | ||
| } | ||
| ] | ||
|
|
||
| return dbModels | ||
| return models | ||
| } |
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.
fetchProviderModels here needs to be implemented using the latest publicdb to gradually replace the hardcoding. You can refer to the implementation on the dashscope side. If the model configuration needs to be modified, you can refer to PublicProviderConf and add the copilot model configuration.
|
Thank you for bringing this to my attention. I'll work on adding a patch.
|
yyhhyyyyyy
left a comment
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.
These places can't be checked by pnpm run lint. Please fix them together.
| } catch (error) { | ||
| this.oauthToken = null | ||
| return null | ||
| } |
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 won't pass linting here.
| } catch (error) { | ||
| this.oauthToken = null | ||
| } | ||
| } |
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 won't pass linting here.
| } catch (error) { | ||
| // 继续轮询,网络错误可能是暂时的 | ||
| } |
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 won't pass linting here.
| } catch (error) { | ||
| throw error | ||
| } |
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 won't pass linting here.
| } catch (error) { | ||
| console.warn('[GitHub Copilot] Device flow failed, using provider API key') | ||
| } |
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 won't pass linting here.
| } catch (e) { | ||
| console.log(` Could not read error response: ${e}`) | ||
| // ignore | ||
| } |
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 won't pass linting here.
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/main/presenter/githubCopilotOAuth.ts (2)
28-28: Don’t log full authUrl (contains state).Log only origin/path or a generic message.
- console.log('Starting GitHub OAuth with URL:', authUrl) + try { + const u = new URL(authUrl) + console.log('Starting GitHub OAuth:', `${u.origin}${u.pathname}?…`) + } catch { console.log('Starting GitHub OAuth') }
45-49: Stop logging full OAuth redirect/navigate URLs and authorization URLs (leaks code/state).These logs will include authorization code and state parameters. Replace with generic messages or redact query parameters.
Applies to lines: 28, 46, 58, 64
+ // Build authorization URL + const authUrl = this.buildAuthUrl() + try { + const u = new URL(authUrl) + console.log('Starting GitHub OAuth with:', `${u.origin}${u.pathname}`) + } catch { + console.log('Starting GitHub OAuth') + } - console.log('Starting GitHub OAuth with URL:', authUrl) - this.authWindow.webContents.on('will-redirect', (_event, url) => { - console.log('Redirecting to:', url) + this.authWindow.webContents.on('will-redirect', (_event, url) => { + try { + const u = new URL(url) + console.log('Redirecting to:', `${u.origin}${u.pathname}`) + } catch { + console.log('Redirecting to OAuth callback') + } this.handleCallback(url, resolve, reject) }) - this.authWindow.webContents.on('did-navigate', (_event, url) => { - console.log('Navigated to:', url) + this.authWindow.webContents.on('did-navigate', (_event, url) => { + console.log('Navigated to OAuth page/callback') this.handleCallback(url, resolve, reject) }) - this.authWindow.webContents.setWindowOpenHandler(({ url }) => { - console.log('New window requested for:', url) + this.authWindow.webContents.setWindowOpenHandler(({ url }) => { + console.log('New OAuth window requested') this.handleCallback(url, resolve, reject) return { action: 'deny' } })src/main/presenter/llmProviderPresenter/oauthHelper.ts (2)
100-108: Don’t broadcast the auth code to all renderer windows.Sending the raw code via event bus to ALL_WINDOWS increases exposure. Send a status-only event, keep the code inside the promise flow.
- console.log('OAuth success, received authorization code') - eventBus.send(CONFIG_EVENTS.OAUTH_LOGIN_SUCCESS, SendTarget.ALL_WINDOWS, code) - resolve(code) + console.log('OAuth success, authorization code received') + eventBus.send(CONFIG_EVENTS.OAUTH_LOGIN_SUCCESS, SendTarget.ALL_WINDOWS) + resolve(code)
56-61: Fix close-handler: incorrect resolve check → may not reject on user cancel.The code checks the function
resolveinstead of completion state. Track completion and reject on window close if not completed.- this.authWindow.on('closed', () => { - this.authWindow = null - if (!resolve) { - reject(new Error('用户取消了登录')) - } - }) + let completed = false + // set completed=true on success/error paths + // ... + this.authWindow.on('closed', () => { + this.authWindow = null + if (!completed) reject(new Error('User cancelled login')) + }) @@ - console.log('OAuth success, received authorization code') + console.log('OAuth success, authorization code received') + completed = true + resolve(code) @@ - this.closeAuthWindow() + this.closeAuthWindow() + completed = trueAlso applies to: 100-108, 116-117
src/main/presenter/oauthPresenter.ts (1)
492-501: Add OAuth state parameter and verify it on callback (CSRF protection).Current flow lacks a state token and accepts any localhost request with CORS=*. Include a random state in the auth URL and verify on callback before exchanging the code.
+ import { randomBytes } from 'crypto' @@ export class OAuthPresenter { private authWindow: BrowserWindow | null = null private callbackServer: http.Server | null = null private callbackPort = 3000 + private oauthState: string | null = null @@ private async startOAuthFlow(config: OAuthConfig): Promise<string> { return new Promise((resolve, reject) => { + // Generate state per attempt + this.oauthState = randomBytes(16).toString('hex') @@ - const authUrl = this.buildAuthUrl(config) + const authUrl = this.buildAuthUrl(config) @@ private buildAuthUrl(config: OAuthConfig): string { const params = new URLSearchParams({ client_id: config.clientId, redirect_uri: config.redirectUri, response_type: config.responseType, - scope: config.scope + scope: config.scope, + state: this.oauthState || '' }) @@ - if (url.pathname === '/auth/callback') { - const code = url.searchParams.get('code') - const error = url.searchParams.get('error') + if (url.pathname === '/auth/callback') { + const code = url.searchParams.get('code') + const error = url.searchParams.get('error') + const state = url.searchParams.get('state') + if (!state || !this.oauthState || state !== this.oauthState) { + res.writeHead(400, { 'Content-Type': 'text/plain' }) + res.end('Invalid state') + return + } @@ - this.handleServerCallback(code, null) + this.handleServerCallback(code, null) + this.oauthState = null @@ - this.handleServerCallback(null, error) + this.handleServerCallback(null, error) + this.oauthState = nullAdditionally, restrict CORS to the expected origin if possible.
Also applies to: 268-276, 303-374
src/main/presenter/githubCopilotDeviceFlow.ts (1)
315-321: Fix undefined page variable GITHUB_DEVICE_URL in injected HTML.The script references an undefined variable inside the rendered page.
- const githubUrl = GITHUB_DEVICE_URL; + const githubUrl = '${GITHUB_DEVICE_URL}'; @@ - alert('Please manually visit: ${GITHUB_DEVICE_URL}'); + alert('Please manually visit: ${GITHUB_DEVICE_URL}');(The first line was missing string interpolation.)
Also applies to: 334-336
♻️ Duplicate comments (1)
src/main/presenter/oauthPresenter.ts (1)
448-449: Remove/redact logs that print full OAuth URLs (auth code leakage).These lines will log the authorization code in cleartext. Replace with generic messages or redact query params.
- console.log('Opening OAuth URL:', authUrl) + try { const u = new URL(authUrl); console.log('Opening OAuth URL:', `${u.origin}${u.pathname}?…`) } + catch { console.log('Opening OAuth URL') } @@ - console.log('OAuth window navigated to:', navigationUrl) + console.log('OAuth window navigated (callback reached)') @@ - console.log('Callback server received request:', url.href) + console.log('Callback server received request (callback hit)')#!/bin/bash # Verify no logs leak full URLs with query (codes/states) rg -n -C1 -P "console\.log\([^)]*(auth|OAuth|Callback)[^)]*:\s*.*https?://.*(\?|%3F)" src/main/presenterAlso applies to: 476-483, 306-307
🧹 Nitpick comments (8)
src/main/presenter/llmProviderPresenter/oauthHelper.ts (1)
63-68: Unify logs/error messages in English per guidelines.Replace mixed-language error strings to keep logs/messages consistent and avoid i18n drift.
- reject(new Error(`加载授权页面失败: ${errorDescription}`)) + reject(new Error(`Failed to load authorization page: ${errorDescription}`)) @@ - reject(new Error('未收到授权码')) + reject(new Error('Authorization code not received')) @@ - reject(new Error('解析回调URL失败')) + reject(new Error('Failed to parse callback URL'))As per coding guidelines.
Also applies to: 100-115
src/main/presenter/oauthPresenter.ts (2)
309-313: Avoid permissive CORS for callback server.Using Access-Control-Allow-Origin: * is unnecessary here and widens attack surface. Restrict to the known web callback origin or omit entirely.
- res.setHeader('Access-Control-Allow-Origin', '*') + res.setHeader('Access-Control-Allow-Origin', 'https://deepchatai.cn')
380-387: Handle port conflicts or randomize callback port.Fixed port 3000 can be occupied; add retry on EADDRINUSE or use an ephemeral port (0) and reflect it in the flow if used externally.
- this.callbackServer.listen(this.callbackPort, 'localhost', () => { + this.callbackServer.listen(this.callbackPort, 'localhost', () => { console.log(`OAuth callback server started on http://localhost:${this.callbackPort}`) resolve() }) + this.callbackServer.on('error', (error: any) => { + if (error?.code === 'EADDRINUSE') { + this.callbackPort = 0 + this.callbackServer!.listen(this.callbackPort, 'localhost', () => { + const addr = this.callbackServer!.address() + console.log('OAuth callback server rebound on', addr) + resolve() + }) + } + })src/main/presenter/githubCopilotDeviceFlow.ts (2)
552-559: Set internal OAuth token when obtained.Ensure subsequent getCopilotToken() calls work without requiring an external setter.
- if (data.access_token) { + if (data.access_token) { if (this.pollingInterval) { clearInterval(this.pollingInterval) this.pollingInterval = null } + this.oauthToken = data.access_token resolve(data.access_token) return }
566-571: Avoid overlapping polls from setInterval.Network latency can cause concurrent poll executions. Prefer a self-scheduling setTimeout (or an in-flight guard).
- this.pollingInterval = setInterval(poll, deviceCodeResponse.interval * 1000) - // Execute first poll immediately - poll() + const schedule = () => { + this.pollingInterval = setTimeout(poll, currentInterval * 1000) as any + } + const poll = async () => { + // ...existing logic... + // at end of non-terminal branches: + schedule() + } + schedule()Also applies to: 468-564
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts (3)
380-389: Implement tool conversion helpers per provider contract.Add convertToProviderTools/prepareFunctionCallPrompt, and pass provider-formatted tools. Current code forwards raw tools.
- ...(tools && tools.length > 0 && { tools }) + ...(tools && tools.length > 0 && { tools: this.convertToProviderTools(tools) })// Add to the class private convertToProviderTools(tools: MCPToolDefinition[]): any[] { // Map MCP tools to Copilot tool schema if supported; otherwise return [] return tools.map(t => ({ type: 'function', function: { name: t.name, description: t.description, parameters: t.parameters } })) }As per coding guidelines.
Also applies to: 553-561
476-479: Emit a stop event when stream ends.Yield a standardized stop event on [DONE] for consumers.
- if (data === '[DONE]') return + if (data === '[DONE]') { + yield { type: 'stop', stop_reason: 'eos' } + return + }
170-355: Revisit hardcoded fallback model list.List mixes non-Copilot models (Claude/Gemini) and speculative entries (GPT‑5). Prefer publicdb only or a minimal safe fallback to avoid UX confusion.
- const models: MODEL_META[] = [ /* many entries */ ] + const models: MODEL_META[] = [ + { id: 'gpt-4o-2024-05-13', name: 'GPT-4o', group: 'GitHub Copilot', providerId: this.provider.id, isCustom: false, contextLength: 64000, maxTokens: 4096, vision: true, functionCall: true, reasoning: false } + ]Or gate behind a feature flag.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/presenter/githubCopilotDeviceFlow.ts(9 hunks)src/main/presenter/githubCopilotOAuth.ts(2 hunks)src/main/presenter/llmProviderPresenter/oauthHelper.ts(1 hunks)src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts(11 hunks)src/main/presenter/oauthPresenter.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/main/presenter/githubCopilotOAuth.tssrc/main/presenter/llmProviderPresenter/oauthHelper.tssrc/main/presenter/oauthPresenter.tssrc/main/presenter/githubCopilotDeviceFlow.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/main/presenter/githubCopilotOAuth.tssrc/main/presenter/llmProviderPresenter/oauthHelper.tssrc/main/presenter/oauthPresenter.tssrc/main/presenter/githubCopilotDeviceFlow.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/presenter/githubCopilotOAuth.tssrc/main/presenter/llmProviderPresenter/oauthHelper.tssrc/main/presenter/oauthPresenter.tssrc/main/presenter/githubCopilotDeviceFlow.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪(如适用)、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/main/presenter/githubCopilotOAuth.tssrc/main/presenter/llmProviderPresenter/oauthHelper.tssrc/main/presenter/oauthPresenter.tssrc/main/presenter/githubCopilotDeviceFlow.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/githubCopilotOAuth.tssrc/main/presenter/llmProviderPresenter/oauthHelper.tssrc/main/presenter/oauthPresenter.tssrc/main/presenter/githubCopilotDeviceFlow.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/githubCopilotOAuth.tssrc/main/presenter/llmProviderPresenter/oauthHelper.tssrc/main/presenter/oauthPresenter.tssrc/main/presenter/githubCopilotDeviceFlow.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Use PascalCase for TypeScript types and classes
Files:
src/main/presenter/githubCopilotOAuth.tssrc/main/presenter/llmProviderPresenter/oauthHelper.tssrc/main/presenter/oauthPresenter.tssrc/main/presenter/githubCopilotDeviceFlow.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place Electron main-process presenters under src/main/presenter/ (Window, Tab, Thread, Mcp, Config, LLMProvider)
Files:
src/main/presenter/githubCopilotOAuth.tssrc/main/presenter/llmProviderPresenter/oauthHelper.tssrc/main/presenter/oauthPresenter.tssrc/main/presenter/githubCopilotDeviceFlow.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
**/*.{ts,tsx,js,jsx,vue,css,scss,md,json,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Prettier style: single quotes, no semicolons, print width 100; run pnpm run format
Files:
src/main/presenter/githubCopilotOAuth.tssrc/main/presenter/llmProviderPresenter/oauthHelper.tssrc/main/presenter/oauthPresenter.tssrc/main/presenter/githubCopilotDeviceFlow.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,vue}: Use OxLint for JS/TS code; keep lint clean
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE for constants
Files:
src/main/presenter/githubCopilotOAuth.tssrc/main/presenter/llmProviderPresenter/oauthHelper.tssrc/main/presenter/oauthPresenter.tssrc/main/presenter/githubCopilotDeviceFlow.tssrc/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
src/main/presenter/llmProviderPresenter/providers/*.ts
📄 CodeRabbit inference engine (.cursor/rules/llm-agent-loop.mdc)
src/main/presenter/llmProviderPresenter/providers/*.ts: Each file insrc/main/presenter/llmProviderPresenter/providers/*.tsshould handle interaction with a specific LLM API, including request/response formatting, tool definition conversion, native/non-native tool call management, and standardizing output streams to a common event format.
Provider implementations must use acoreStreammethod that yields standardized stream events to decouple the main loop from provider-specific details.
ThecoreStreammethod in each Provider must perform a single streaming API request per conversation round and must not contain multi-round tool call loop logic.
Provider files should implement helper methods such asformatMessages,convertToProviderTools,parseFunctionCalls, andprepareFunctionCallPromptas needed for provider-specific logic.
All provider implementations must parse provider-specific data chunks and yield standardized events for text, reasoning, tool calls, usage, errors, stop reasons, and image data.
When a provider does not support native function calling, it must prepare messages using prompt wrapping (e.g.,prepareFunctionCallPrompt) before making the API call.
When a provider supports native function calling, MCP tools must be converted to the provider's format (e.g., usingconvertToProviderTools) and included in the API request.
Provider implementations should aggregate and yield usage events as part of the standardized stream.
Provider implementations should yield image data events in the standardized format when applicable.
Provider implementations should yield reasoning events in the standardized format when applicable.
Provider implementations should yield tool call events (tool_call_start,tool_call_chunk,tool_call_end) in the standardized format.
Provider implementations should yield stop events with appropriatestop_reasonin the standardized format.
Provider implementations should yield error events in the standardized format...
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
🧬 Code graph analysis (2)
src/main/presenter/oauthPresenter.ts (4)
src/main/presenter/githubCopilotDeviceFlow.ts (1)
getGlobalGitHubCopilotDeviceFlow(621-626)src/main/presenter/index.ts (1)
presenter(223-223)src/main/eventbus.ts (1)
eventBus(151-151)src/main/presenter/githubCopilotOAuth.ts (1)
createGitHubCopilotOAuth(224-277)
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts (2)
src/main/presenter/githubCopilotDeviceFlow.ts (2)
GitHubCopilotDeviceFlow(44-590)getGlobalGitHubCopilotDeviceFlow(621-626)src/shared/types/presenters/legacy.presenters.d.ts (3)
LLM_PROVIDER(573-593)IConfigPresenter(380-542)MODEL_META(558-572)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-check (x64)
🔇 Additional comments (5)
src/main/presenter/githubCopilotOAuth.ts (2)
134-136: LGTM: generic success log.Good switch to a non-sensitive message. Ensure earlier redirect/navigate logs are also sanitized (see separate comment).
268-273: LGTM: safe final config logging.Boolean flags for clientId/clientSecret avoid leaking secrets; matches guidelines.
src/main/presenter/llmProviderPresenter/oauthHelper.ts (1)
105-107: LGTM: generic OAuth success log.Avoids leaking the code. Good.
src/main/presenter/githubCopilotDeviceFlow.ts (1)
517-523: LGTM: backoff on slow_down implemented correctly.Updating currentInterval and rescheduling addresses prior rate-limit issues.
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts (1)
118-136: Good: targeted error messages and 403 specialization.Clear guidance for 401/403/404 and inclusion of response text is helpful while not logging secrets.
Also applies to: 426-453, 589-616
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
Show resolved
Hide resolved
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
Outdated
Show resolved
Hide resolved
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts (2)
367-389: Remove underscore prefix from_maxTokenssince it's actively used.The underscore prefix typically signals an intentionally unused parameter, but
_maxTokensis used on line 387. This is misleading and violates common naming conventions.Apply this diff:
async *coreStream( messages: ChatMessage[], modelId: string, _modelConfig: ModelConfig, temperature: number, - _maxTokens: number, + maxTokens: number, tools: MCPToolDefinition[] ): AsyncGenerator<LLMCoreStreamEvent, void, unknown> { if (!modelId) throw new Error('Model ID is required') try { const token = await this.getCopilotToken() const formattedMessages = this.formatMessages(messages) const requestBody = { intent: true, n: 1, model: modelId, messages: formattedMessages, stream: true, temperature: temperature ?? 0.7, - max_tokens: _maxTokens || 4096, + max_tokens: maxTokens || 4096, ...(tools && tools.length > 0 && { tools }) }Also update the log statement on line 406:
console.log( - ` Request Body: { messages: ${formattedMessages.length}, model: "${modelId}", temperature: ${temperature}, max_tokens: ${_maxTokens} }` + ` Request Body: { messages: ${formattedMessages.length}, model: "${modelId}", temperature: ${temperature}, max_tokens: ${maxTokens} }` )
535-573: Remove underscore prefix from_maxTokensparameter.Same issue as in
coreStream: the parameter is actively used (line 551), so the underscore prefix is misleading.Apply this diff:
async completions( messages: ChatMessage[], modelId: string, temperature?: number, - _maxTokens?: number + maxTokens?: number ): Promise<LLMResponse> { if (!modelId) throw new Error('Model ID is required') try { const token = await this.getCopilotToken() const formattedMessages = this.formatMessages(messages) const requestBody = { intent: true, n: 1, model: modelId, messages: formattedMessages, - max_tokens: _maxTokens || 4096, + max_tokens: maxTokens || 4096, stream: false, temperature: temperature ?? 0.7 }Also update line 571:
console.log( - ` Request Body: { messages: ${formattedMessages.length}, model: "${modelId}", temperature: ${temperature}, max_tokens: ${_maxTokens} }` + ` Request Body: { messages: ${formattedMessages.length}, model: "${modelId}", temperature: ${temperature}, max_tokens: ${maxTokens} }` )
♻️ Duplicate comments (2)
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts (2)
149-358: PublicDB integration is partially complete; full migration still pending.The code now prioritizes publicdb models (lines 151-167), which is good progress. However, the extensive hardcoded fallback list (lines 174-355) should eventually be removed once publicdb is fully populated with Copilot models.
As noted in the previous review, reference PublicProviderConf to add complete Copilot model configurations.
684-695: Device-flow-only setups are blocked by the apiKey check.The early return on lines 687-692 prevents
check()from succeeding when only device flow is configured. SincegetCopilotToken()already handles device flow as a priority (line 694), this guard should allow empty apiKey when device flow is valid.Apply this diff to allow device-flow-only authentication:
- // 检查是否有 API Key - if (!this.provider.apiKey || !this.provider.apiKey.trim()) { - return { - isOk: false, - errorMsg: '请先使用 GitHub OAuth 登录以获取访问令牌' - } - } + // 检查是否有 device flow 或 API Key + if ((!this.provider.apiKey || !this.provider.apiKey.trim()) && !this.deviceFlow) { + return { + isOk: false, + errorMsg: '请先使用 GitHub OAuth 登录以获取访问令牌' + } + } await this.getCopilotToken() return { isOk: true, errorMsg: null }This allows the check to proceed when device flow is configured, even without apiKey.
🧹 Nitpick comments (1)
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts (1)
48-54: Consider logging or notifying when existing auth is invalidated.The existing auth check silently resets
apiKeywhen validation fails. Users may be confused if their stored token suddenly stops working without explanation.Apply this diff to add a warning log:
// 检查现有认证状态 if (this.provider.apiKey) { const existingToken = await this.deviceFlow.checkExistingAuth(this.provider.apiKey) if (!existingToken) { + console.warn('[GitHub Copilot] Existing OAuth token is invalid or expired, clearing apiKey') this.provider.apiKey = '' } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts(11 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪(如适用)、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
src/main/presenter/llmProviderPresenter/providers/*.ts
📄 CodeRabbit inference engine (.cursor/rules/llm-agent-loop.mdc)
src/main/presenter/llmProviderPresenter/providers/*.ts: Each file insrc/main/presenter/llmProviderPresenter/providers/*.tsshould handle interaction with a specific LLM API, including request/response formatting, tool definition conversion, native/non-native tool call management, and standardizing output streams to a common event format.
Provider implementations must use acoreStreammethod that yields standardized stream events to decouple the main loop from provider-specific details.
ThecoreStreammethod in each Provider must perform a single streaming API request per conversation round and must not contain multi-round tool call loop logic.
Provider files should implement helper methods such asformatMessages,convertToProviderTools,parseFunctionCalls, andprepareFunctionCallPromptas needed for provider-specific logic.
All provider implementations must parse provider-specific data chunks and yield standardized events for text, reasoning, tool calls, usage, errors, stop reasons, and image data.
When a provider does not support native function calling, it must prepare messages using prompt wrapping (e.g.,prepareFunctionCallPrompt) before making the API call.
When a provider supports native function calling, MCP tools must be converted to the provider's format (e.g., usingconvertToProviderTools) and included in the API request.
Provider implementations should aggregate and yield usage events as part of the standardized stream.
Provider implementations should yield image data events in the standardized format when applicable.
Provider implementations should yield reasoning events in the standardized format when applicable.
Provider implementations should yield tool call events (tool_call_start,tool_call_chunk,tool_call_end) in the standardized format.
Provider implementations should yield stop events with appropriatestop_reasonin the standardized format.
Provider implementations should yield error events in the standardized format...
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Use PascalCase for TypeScript types and classes
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place Electron main-process presenters under src/main/presenter/ (Window, Tab, Thread, Mcp, Config, LLMProvider)
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
**/*.{ts,tsx,js,jsx,vue,css,scss,md,json,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Prettier style: single quotes, no semicolons, print width 100; run pnpm run format
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,vue}: Use OxLint for JS/TS code; keep lint clean
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE for constants
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
🧬 Code graph analysis (1)
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts (2)
src/main/presenter/githubCopilotDeviceFlow.ts (2)
GitHubCopilotDeviceFlow(44-590)getGlobalGitHubCopilotDeviceFlow(621-626)src/shared/types/presenters/legacy.presenters.d.ts (3)
LLM_PROVIDER(573-593)IConfigPresenter(380-542)MODEL_META(558-572)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-check (x64)
🔇 Additional comments (4)
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts (4)
400-407: Great improvement on request logging!The logging now only emits safe metadata (message count, model, temperature, max_tokens) instead of the full request body. This properly addresses the PII risk flagged in the previous review.
428-455: Excellent error handling with detailed user guidance.The enhanced error handling includes:
- Safe response.text() extraction with try-catch
- Detailed 403 error messages with actionable solutions
- User-friendly explanations for subscription and auth issues
565-572: Logging improvements look good.Similar to
coreStream, the logging now only emits metadata instead of full request bodies, addressing PII concerns.
699-718: Well-structured error messages with actionable guidance.The error handling provides detailed, user-friendly messages for common failure scenarios (404, 401, 403, network issues) with specific resolution steps.
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
Show resolved
Hide resolved
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.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts (2)
70-72: Handle initialization errors and await async operation.The method calls
init()which is async and can throw, but doesn't await it or handle errors. This can cause unhandled promise rejections.As per coding guidelines
Apply this diff to properly handle the async operation:
- public onProxyResolved(): void { - this.init() + public async onProxyResolved(): Promise<void> { + try { + await this.init() + } catch (error) { + console.error('[GitHub Copilot] Failed to reinitialize after proxy resolution:', error) + } }
122-136: Translate error messages to English per coding guidelines.The error messages are in Chinese, violating the project guideline: "Use English for all logs and comments".
As per coding guidelines
Apply this diff to translate the error messages:
if (response.status === 404) { - errorMessage = `GitHub Copilot 访问被拒绝 (404)。请检查: -1. 您的 GitHub 账户是否有有效的 GitHub Copilot 订阅 -2. OAuth token 权限不足 - 需要 'read:org' 权限访问 Copilot API -3. 请重新进行 OAuth 登录以获取正确的权限范围 -4. 访问 https://github.com/features/copilot 检查订阅状态` + errorMessage = `GitHub Copilot access denied (404). Please check: +1. Your GitHub account has a valid GitHub Copilot subscription +2. OAuth token permissions - 'read:org' scope required for Copilot API access +3. Re-authenticate via OAuth to obtain correct permission scopes +4. Visit https://github.com/features/copilot to check subscription status` } else if (response.status === 401) { - errorMessage = `GitHub OAuth token 无效或已过期 (401)。请重新登录授权并确保获取了正确的权限范围。` + errorMessage = `GitHub OAuth token is invalid or expired (401). Please re-authenticate and ensure correct permission scopes are obtained.` } else if (response.status === 403) { - errorMessage = `GitHub Copilot 访问被禁止 (403)。请检查: -1. 您的 GitHub Copilot 订阅是否有效且处于活跃状态 -2. 是否达到了 API 使用限制 -3. OAuth token 是否包含 'read:org' 权限范围 -4. 如果是组织账户,请确保组织已启用 Copilot 并且您有访问权限` + errorMessage = `GitHub Copilot access forbidden (403). Please check: +1. Your GitHub Copilot subscription is valid and active +2. API rate limits have not been exceeded +3. OAuth token includes 'read:org' permission scope +4. For organization accounts, ensure Copilot is enabled and you have access permissions` }
♻️ Duplicate comments (1)
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts (1)
542-542: Same issues as incoreStreammethod.This method has the same two issues identified in
coreStream:
- Line 542: Misleading
_maxTokensparameter naming (used at Line 554 and 574)- Lines 607-616: Chinese error messages violating coding guidelines
- Lines 606-622: Duplicate error handling logic from
coreStream(lines 440-457)Please apply the same fixes as suggested for the
coreStreammethod. Additionally, extract the shared error handling logic into a private helper method likehandleApiErrorResponse(response, errorBody)to eliminate code duplication between both methods.Also applies to: 606-622
🧹 Nitpick comments (1)
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts (1)
49-54: Log when clearing invalid API key for debugging.When
checkExistingAuthreturns null and the API key is cleared, no log entry explains why. This makes debugging authentication issues difficult.Apply this diff to add explanatory logging:
if (this.provider.apiKey) { const existingToken = await this.deviceFlow.checkExistingAuth(this.provider.apiKey) if (!existingToken) { + console.warn('[GitHub Copilot] Existing API key validation failed, clearing invalid token') this.provider.apiKey = '' } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts(11 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪(如适用)、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
src/main/presenter/llmProviderPresenter/providers/*.ts
📄 CodeRabbit inference engine (.cursor/rules/llm-agent-loop.mdc)
src/main/presenter/llmProviderPresenter/providers/*.ts: Each file insrc/main/presenter/llmProviderPresenter/providers/*.tsshould handle interaction with a specific LLM API, including request/response formatting, tool definition conversion, native/non-native tool call management, and standardizing output streams to a common event format.
Provider implementations must use acoreStreammethod that yields standardized stream events to decouple the main loop from provider-specific details.
ThecoreStreammethod in each Provider must perform a single streaming API request per conversation round and must not contain multi-round tool call loop logic.
Provider files should implement helper methods such asformatMessages,convertToProviderTools,parseFunctionCalls, andprepareFunctionCallPromptas needed for provider-specific logic.
All provider implementations must parse provider-specific data chunks and yield standardized events for text, reasoning, tool calls, usage, errors, stop reasons, and image data.
When a provider does not support native function calling, it must prepare messages using prompt wrapping (e.g.,prepareFunctionCallPrompt) before making the API call.
When a provider supports native function calling, MCP tools must be converted to the provider's format (e.g., usingconvertToProviderTools) and included in the API request.
Provider implementations should aggregate and yield usage events as part of the standardized stream.
Provider implementations should yield image data events in the standardized format when applicable.
Provider implementations should yield reasoning events in the standardized format when applicable.
Provider implementations should yield tool call events (tool_call_start,tool_call_chunk,tool_call_end) in the standardized format.
Provider implementations should yield stop events with appropriatestop_reasonin the standardized format.
Provider implementations should yield error events in the standardized format...
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Use PascalCase for TypeScript types and classes
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place Electron main-process presenters under src/main/presenter/ (Window, Tab, Thread, Mcp, Config, LLMProvider)
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
**/*.{ts,tsx,js,jsx,vue,css,scss,md,json,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Prettier style: single quotes, no semicolons, print width 100; run pnpm run format
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,vue}: Use OxLint for JS/TS code; keep lint clean
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE for constants
Files:
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
🧬 Code graph analysis (1)
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts (2)
src/main/presenter/githubCopilotDeviceFlow.ts (2)
GitHubCopilotDeviceFlow(44-590)getGlobalGitHubCopilotDeviceFlow(621-626)src/shared/types/presenters/legacy.presenters.d.ts (3)
LLM_PROVIDER(573-593)IConfigPresenter(380-542)MODEL_META(558-572)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-check (x64)
🔇 Additional comments (3)
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts (3)
13-16: LGTM! Device flow integration properly scaffolded.The imports and property declaration correctly set up the device flow infrastructure.
Also applies to: 35-35
648-741: LGTM! Wrapper methods correctly delegate tocompletions.The
summaries,generateText, andsummaryTitlesmethods are clean wrappers that properly construct message arrays and delegate to thecompletionsmethod.
177-360: Update hardcoded models to align with GitHub Copilot's current 2025 support list.GitHub Copilot Chat as of 2025 supports models from OpenAI (GPT-4.1, GPT-5, GPT-5 mini, o3), Anthropic (Claude Sonnet 4.5, 4, 3.7, 3.7 Thinking, Claude Opus variants), Google (Gemini 2.5 Pro, 2.0 Flash), and xAI (Grok Code Fast 1). The hardcoded fallback list has gaps—notably missing Claude Opus variants and Grok, and includes models not listed in current documentation (e.g.,
o1, possiblygpt-4o-2024-05-13,gpt-3.5-turbo). Additionally, verify model ID naming (e.g.,o3-minivs the web documentation's reference). Several deprecations are effective Oct 23, 2025, so review and clean up the list accordingly.
| _modelConfig: ModelConfig, | ||
| temperature: number, | ||
| maxTokens: number, | ||
| _maxTokens: number, |
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.
🛠️ Refactor suggestion | 🟠 Major
Remove misleading underscore prefix from used parameter.
The _maxTokens parameter uses an underscore prefix which conventionally signals an unused parameter, but it's actually used at Line 390. This misleading naming can confuse code reviewers and maintainers.
Apply this diff to use a clear parameter name:
async *coreStream(
messages: ChatMessage[],
modelId: string,
_modelConfig: ModelConfig,
temperature: number,
- _maxTokens: number,
+ maxTokens: number,
tools: MCPToolDefinition[]
): AsyncGenerator<LLMCoreStreamEvent, void, unknown> {And update the usage at Line 390:
- max_tokens: _maxTokens || 4096,
+ max_tokens: maxTokens || 4096,Also update the log at Line 409:
- ` Request Body: { messages: ${formattedMessages.length}, model: "${modelId}", temperature: ${temperature}, max_tokens: ${_maxTokens} }`
+ ` Request Body: { messages: ${formattedMessages.length}, model: "${modelId}", temperature: ${temperature}, max_tokens: ${maxTokens} }`🤖 Prompt for AI Agents
In src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
around line 375, the parameter named `_maxTokens` is misleadingly prefixed with
an underscore even though it is used; rename the parameter to `maxTokens`,
update its use at line 390 to reference `maxTokens` instead of `_maxTokens`, and
update the log at line 409 to include `maxTokens` (remove the underscore) so
names accurately reflect usage.
| if (response.status === 403) { | ||
| throw new Error( | ||
| `GitHub Copilot 访问被拒绝 (403)。\n\n可能的原因:\n` + | ||
| `1. GitHub Copilot 订阅已过期或未激活\n` + | ||
| `2. 需要重新认证以获取正确的访问权限\n` + | ||
| `3. API访问策略已更新,需要使用最新的认证方式\n` + | ||
| `4. 您的账户可能没有访问此API的权限\n\n` + | ||
| `建议解决方案:\n` + | ||
| `- 访问 https://github.com/settings/copilot 检查订阅状态\n` + | ||
| `- 在DeepChat设置中重新进行 GitHub Copilot 登录\n` + | ||
| `- 确保您的 GitHub 账户有有效的 Copilot 订阅\n` + | ||
| `- 如果是企业账户,请联系管理员确认访问权限` | ||
| ) | ||
| } | ||
|
|
||
| throw new Error(`GitHub Copilot API error: ${response.status} ${response.statusText}`) | ||
| throw new Error( | ||
| `GitHub Copilot API error: ${response.status} ${response.statusText} - ${errorBody}` | ||
| ) |
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.
Translate error messages to English and extract duplicate error handling.
Two issues:
- Error message is in Chinese, violating the guideline: "Use English for all logs and comments"
- The same 403 error handling logic is duplicated in the
completionsmethod (lines 606-617)
As per coding guidelines
Apply this diff to translate the error message:
if (response.status === 403) {
throw new Error(
- `GitHub Copilot 访问被拒绝 (403)。\n\n可能的原因:\n` +
- `1. GitHub Copilot 订阅已过期或未激活\n` +
- `2. 需要重新认证以获取正确的访问权限\n` +
- `3. API访问策略已更新,需要使用最新的认证方式\n` +
- `4. 您的账户可能没有访问此API的权限\n\n` +
- `建议解决方案:\n` +
- `- 访问 https://github.com/settings/copilot 检查订阅状态\n` +
- `- 在DeepChat设置中重新进行 GitHub Copilot 登录\n` +
- `- 确保您的 GitHub 账户有有效的 Copilot 订阅\n` +
- `- 如果是企业账户,请联系管理员确认访问权限`
+ `GitHub Copilot access denied (403).\n\nPossible causes:\n` +
+ `1. GitHub Copilot subscription expired or not activated\n` +
+ `2. Re-authentication required to obtain correct access permissions\n` +
+ `3. API access policy updated, latest authentication method required\n` +
+ `4. Your account may not have permission to access this API\n\n` +
+ `Suggested solutions:\n` +
+ `- Visit https://github.com/settings/copilot to check subscription status\n` +
+ `- Re-authenticate GitHub Copilot in DeepChat settings\n` +
+ `- Ensure your GitHub account has a valid Copilot subscription\n` +
+ `- For enterprise accounts, contact your administrator to confirm access permissions`
)
}Consider extracting the duplicate error handling logic from coreStream and completions into a private helper method to improve maintainability.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (response.status === 403) { | |
| throw new Error( | |
| `GitHub Copilot 访问被拒绝 (403)。\n\n可能的原因:\n` + | |
| `1. GitHub Copilot 订阅已过期或未激活\n` + | |
| `2. 需要重新认证以获取正确的访问权限\n` + | |
| `3. API访问策略已更新,需要使用最新的认证方式\n` + | |
| `4. 您的账户可能没有访问此API的权限\n\n` + | |
| `建议解决方案:\n` + | |
| `- 访问 https://github.com/settings/copilot 检查订阅状态\n` + | |
| `- 在DeepChat设置中重新进行 GitHub Copilot 登录\n` + | |
| `- 确保您的 GitHub 账户有有效的 Copilot 订阅\n` + | |
| `- 如果是企业账户,请联系管理员确认访问权限` | |
| ) | |
| } | |
| throw new Error(`GitHub Copilot API error: ${response.status} ${response.statusText}`) | |
| throw new Error( | |
| `GitHub Copilot API error: ${response.status} ${response.statusText} - ${errorBody}` | |
| ) | |
| if (response.status === 403) { | |
| throw new Error( | |
| `GitHub Copilot access denied (403).\n\nPossible causes:\n` + | |
| `1. GitHub Copilot subscription expired or not activated\n` + | |
| `2. Re-authentication required to obtain correct access permissions\n` + | |
| `3. API access policy updated, latest authentication method required\n` + | |
| `4. Your account may not have permission to access this API\n\n` + | |
| `Suggested solutions:\n` + | |
| `- Visit https://github.com/settings/copilot to check subscription status\n` + | |
| `- Re-authenticate GitHub Copilot in DeepChat settings\n` + | |
| `- Ensure your GitHub account has a valid Copilot subscription\n` + | |
| `- For enterprise accounts, contact your administrator to confirm access permissions` | |
| ) | |
| } | |
| throw new Error( | |
| `GitHub Copilot API error: ${response.status} ${response.statusText} - ${errorBody}` | |
| ) |
🤖 Prompt for AI Agents
In src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
around lines 440-457 (and note duplicate 403 handling in completions at ~lines
606-617), the 403 error message is in Chinese and the same logic is duplicated;
translate the 403 message and all logs to English, and extract the repeated
response error handling into a private helper method on the provider class
(e.g., handleApiError(response, errorBody)) that performs the 403-specialized
message (in English) and otherwise throws a consistent Error including
response.status, response.statusText and errorBody; replace the inline blocks in
coreStream and completions to call this helper so both paths use the same logic.
| if (errorMsg.includes('404')) { | ||
| errorMsg = `GitHub Copilot 访问被拒绝 (404)。请检查: | ||
| 1. 您的 GitHub 账户是否有有效的 GitHub Copilot 订阅 | ||
| 2. 访问 https://github.com/features/copilot 检查订阅状态 | ||
| 3. 如果是组织账户,请确保组织已启用 Copilot 并且您有访问权限` | ||
| } else if (errorMsg.includes('401')) { | ||
| errorMsg = `GitHub OAuth token 无效或已过期 (401)。请重新登录授权并确保获取了正确的权限范围。` | ||
| } else if (errorMsg.includes('403')) { | ||
| errorMsg = `GitHub Copilot 访问被禁止 (403)。请检查: | ||
| 1. 您的 GitHub Copilot 订阅是否有效且处于活跃状态 | ||
| 2. 是否达到了 API 使用限制 | ||
| 3. OAuth token 是否包含 'read:org' 权限范围 | ||
| 4. 如果是组织账户,请确保组织已启用 Copilot 并且您有访问权限` | ||
| } else if (errorMsg.includes('fetch failed') || errorMsg.includes('network')) { | ||
| errorMsg = `网络连接失败。请检查: | ||
| 1. 网络连接是否正常 | ||
| 2. 代理设置是否正确 | ||
| 3. 防火墙是否阻止了 GitHub API 访问` | ||
| } |
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.
Translate error messages to English per coding guidelines.
The error messages are in Chinese, violating the project guideline: "Use English for all logs and comments".
As per coding guidelines
Apply this diff to translate the error messages:
if (errorMsg.includes('404')) {
- errorMsg = `GitHub Copilot 访问被拒绝 (404)。请检查:
-1. 您的 GitHub 账户是否有有效的 GitHub Copilot 订阅
-2. 访问 https://github.com/features/copilot 检查订阅状态
-3. 如果是组织账户,请确保组织已启用 Copilot 并且您有访问权限`
+ errorMsg = `GitHub Copilot access denied (404). Please check:
+1. Your GitHub account has a valid GitHub Copilot subscription
+2. Visit https://github.com/features/copilot to check subscription status
+3. For organization accounts, ensure Copilot is enabled and you have access permissions`
} else if (errorMsg.includes('401')) {
- errorMsg = `GitHub OAuth token 无效或已过期 (401)。请重新登录授权并确保获取了正确的权限范围。`
+ errorMsg = `GitHub OAuth token is invalid or expired (401). Please re-authenticate and ensure correct permission scopes are obtained.`
} else if (errorMsg.includes('403')) {
- errorMsg = `GitHub Copilot 访问被禁止 (403)。请检查:
-1. 您的 GitHub Copilot 订阅是否有效且处于活跃状态
-2. 是否达到了 API 使用限制
-3. OAuth token 是否包含 'read:org' 权限范围
-4. 如果是组织账户,请确保组织已启用 Copilot 并且您有访问权限`
+ errorMsg = `GitHub Copilot access forbidden (403). Please check:
+1. Your GitHub Copilot subscription is valid and active
+2. API rate limits have not been exceeded
+3. OAuth token includes 'read:org' permission scope
+4. For organization accounts, ensure Copilot is enabled and you have access permissions`
} else if (errorMsg.includes('fetch failed') || errorMsg.includes('network')) {
- errorMsg = `网络连接失败。请检查:
-1. 网络连接是否正常
-2. 代理设置是否正确
-3. 防火墙是否阻止了 GitHub API 访问`
+ errorMsg = `Network connection failed. Please check:
+1. Network connection is working properly
+2. Proxy settings are configured correctly
+3. Firewall is not blocking GitHub API access`
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (errorMsg.includes('404')) { | |
| errorMsg = `GitHub Copilot 访问被拒绝 (404)。请检查: | |
| 1. 您的 GitHub 账户是否有有效的 GitHub Copilot 订阅 | |
| 2. 访问 https://github.com/features/copilot 检查订阅状态 | |
| 3. 如果是组织账户,请确保组织已启用 Copilot 并且您有访问权限` | |
| } else if (errorMsg.includes('401')) { | |
| errorMsg = `GitHub OAuth token 无效或已过期 (401)。请重新登录授权并确保获取了正确的权限范围。` | |
| } else if (errorMsg.includes('403')) { | |
| errorMsg = `GitHub Copilot 访问被禁止 (403)。请检查: | |
| 1. 您的 GitHub Copilot 订阅是否有效且处于活跃状态 | |
| 2. 是否达到了 API 使用限制 | |
| 3. OAuth token 是否包含 'read:org' 权限范围 | |
| 4. 如果是组织账户,请确保组织已启用 Copilot 并且您有访问权限` | |
| } else if (errorMsg.includes('fetch failed') || errorMsg.includes('network')) { | |
| errorMsg = `网络连接失败。请检查: | |
| 1. 网络连接是否正常 | |
| 2. 代理设置是否正确 | |
| 3. 防火墙是否阻止了 GitHub API 访问` | |
| } | |
| if (errorMsg.includes('404')) { | |
| errorMsg = `GitHub Copilot access denied (404). Please check: | |
| 1. Your GitHub account has a valid GitHub Copilot subscription | |
| 2. Visit https://github.com/features/copilot to check subscription status | |
| 3. For organization accounts, ensure Copilot is enabled and you have access permissions` | |
| } else if (errorMsg.includes('401')) { | |
| errorMsg = `GitHub OAuth token is invalid or expired (401). Please re-authenticate and ensure correct permission scopes are obtained.` | |
| } else if (errorMsg.includes('403')) { | |
| errorMsg = `GitHub Copilot access forbidden (403). Please check: | |
| 1. Your GitHub Copilot subscription is valid and active | |
| 2. API rate limits have not been exceeded | |
| 3. OAuth token includes 'read:org' permission scope | |
| 4. For organization accounts, ensure Copilot is enabled and you have access permissions` | |
| } else if (errorMsg.includes('fetch failed') || errorMsg.includes('network')) { | |
| errorMsg = `Network connection failed. Please check: | |
| 1. Network connection is working properly | |
| 2. Proxy settings are configured correctly | |
| 3. Firewall is not blocking GitHub API access` | |
| } |
🤖 Prompt for AI Agents
In src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
around lines 696 to 714, the error message strings are written in Chinese; per
project guidelines they must be in English — replace each Chinese message with
an English equivalent preserving the same error logic, details and list items
(404, 401, 403, network/fetch cases), keep error codes and explanatory bullets
intact, and ensure wording is clear and concise (e.g., "GitHub Copilot access
denied (404). Check: 1. Your GitHub account has an active Copilot subscription
2. Visit https://github.com/features/copilot to verify 3. For org accounts
ensure Copilot is enabled and you have access", etc.).
yyhhyyyyyy
left a comment
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.
LGTM
* style(settings): format about page link handler (#1016) * style(ollama): format model config handlers (#1018) * fix: think content scroll issue (#1023) * fix: remove shimmer for think content * chore: update screen shot and fix scroll issue * chore: update markdown renderer * fix: import button bug and prevent backup overwriting during import (#1024) * fix(sync): fix import button bug and prevent backup overwriting during import * fix(sync): fix import button bug and prevent backup overwriting during import * fix(sync): fix import button bug and prevent backup overwriting during import * refactor(messageList): refactor message list ui components (#1026) * feat: remove new thread button, add clean button. * refactor(messageList): refactor message list ui components * feat: add configurable fields for chat settings - Introduced ConfigFieldHeader component for consistent field headers. - Added ConfigInputField, ConfigSelectField, ConfigSliderField, and ConfigSwitchField components for various input types. - Created types for field configurations in types.ts to standardize field definitions. - Implemented useChatConfigFields composable to manage field configurations dynamically. - Added useModelCapabilities and useModelTypeDetection composables for handling model-specific capabilities and requirements. - Developed useSearchConfig and useThinkingBudget composables for managing search and budget configurations. * feat: implement input history management in prompt input - Added `useInputHistory` composable for managing input history and navigation. - Implemented methods for setting, clearing, and confirming history placeholders. - Integrated arrow key navigation for browsing through input history. feat: enhance mention data handling in prompt input - Created `useMentionData` composable to aggregate mention data from selected files and MCP resources. - Implemented watchers to update mention data based on selected files, MCP resources, tools, and prompts. feat: manage prompt input configuration with store synchronization - Developed `usePromptInputConfig` composable for managing model configuration. - Implemented bidirectional sync between local config and chat store. - Added debounced watcher to reduce updates and improve performance. feat: streamline TipTap editor operations in prompt input - Introduced `usePromptInputEditor` composable for managing TipTap editor lifecycle and content transformation. - Implemented methods for handling mentions, pasting content, and clearing editor content. feat: handle file operations in prompt input - Created `usePromptInputFiles` composable for managing file selection, paste, and drag-drop operations. - Implemented methods for processing files, handling dropped files, and clearing selected files. feat: manage rate limit status in prompt input - Developed `useRateLimitStatus` composable for displaying and polling rate limit status. - Implemented methods for handling rate limit events and computing status icons, classes, and tooltips. * refactor(artifacts): migrate component logic to composables and update documentation - Refactor ArtifactDialog.vue to use composables for view mode, viewport size, code editor, and export functionality - Simplify HTMLArtifact.vue by removing drag-resize logic and using fixed viewport dimensions - Clean up MermaidArtifact.vue styling and structure - Update component refactoring guide to reflect new patterns and best practices - Adjust prompt input composable to allow delayed editor initialization - Update internationalization files for new responsive label * fix(lint): unused variables * fix(format): format code * CodeRabbit Generated Unit Tests: Add renderer unit tests for components and composables * feat: implement input history management in chat input component - Added `useInputHistory` composable for managing input history and placeholder navigation. - Implemented methods for setting, clearing, and confirming history placeholders. - Integrated arrow key navigation for cycling through input history. feat: enhance mention data handling in chat input - Created `useMentionData` composable to manage mention data aggregation. - Implemented watchers for selected files and MCP resources/tools/prompts to update mention data. feat: manage prompt input configuration and synchronization - Developed `usePromptInputConfig` composable for managing model configuration. - Implemented bidirectional sync between local config refs and chat store. - Added debounced watcher to reduce updates to the store. feat: manage prompt input editor operations - Introduced `usePromptInputEditor` composable for handling TipTap editor operations. - Implemented content transformation, mention insertion, and paste handling. - Added methods for handling editor updates and restoring focus. feat: handle prompt input files management - Created `usePromptInputFiles` composable for managing file operations in prompt input. - Implemented file selection, paste, drag-drop, and prompt files integration. feat: implement rate limit status management - Developed `useRateLimitStatus` composable for managing rate limit status display and polling. - Added methods for retrieving rate limit status icon, class, tooltip, and wait time formatting. * feat: enhance chat input component with context length management and settings integration * feat: update model configuration and enhance error handling in providers * feat: add MCP tools list component and integrate with chat settings feat: enhance artifact dialog with improved error handling and localization fix: update Mermaid artifact rendering error handling and localization fix: improve input settings error handling and state management fix: update drag and drop composable to handle drag events correctly fix: update Vitest configuration for better project structure and alias resolution * fix(i18n): add unknownError translation --------- Co-authored-by: deepinsect <deepinsect@github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat: add Poe provider integration and icon support (#1028) * feat: add Poe provider integration and icon support * chore: format and lint --------- Co-authored-by: zerob13 <zerob13@gmail.com> * fix: make auto scroll works (#1030) * fix: allow settings window links to open externally (#1029) * fix(settings): allow target blank links * fix: harden settings window link handling * feat: enhance GitHub Copilot Device Flow with OAuth token management and API token retrieval (#1021) * feat: enhance GitHub Copilot Device Flow with OAuth token management and API token retrieval - Fixed request header for managing OAuth tokens and retrieving API tokens. - Enhanced model definitions and added new models for better compatibility. * fix: remove privacy related log * fix: OAuth 2.0 for slow_down response * fix: handle lint errors * fix: provider fetched from publicdb * fix(githubCopilotProvider): update request body logging format for clarity * fix(githubCopilotProvider): improve error handling and logging in device flow * feat(theme): fix message paragraph gap and toolcall block (#1031) Co-authored-by: deepinsect <deepinsect@github.com> * fix: scroll to bottom (#1034) * fix: add debounce for renderer * feat: add max wait for renderer * chore(deps): upgrade markdown renderer add worker support * chore: bump markdown version * fix(build): use es module worker format (#1037) * feat: remove function deleteOllamaModel (#1036) * feat: remove function deleteOllamaModel * fix(build): use es module worker format (#1037) --------- Co-authored-by: duskzhen <zerob13@gmail.com> * perf: update dependencies to use stream-monaco and bump vue-renderer-markdown version (#1038) * feat(theme): add markdown layout style and table style (#1039) * feat(theme): add markdown layout style and table style * fix(lint): remove props --------- Co-authored-by: deepinsect <deepinsect@github.com> * feat: support effort and verbosity (#1040) * chore: bump up version * feat: add jiekou.ai as LLM provider (#1041) * feat: add jiekou.ai as LLM provider * fix: change api type to jiekou --------- Co-authored-by: zerob13 <zerob13@gmail.com> * chore: update provider db --------- Co-authored-by: 韦伟 <xweimvp@gmail.com> Co-authored-by: Happer <ericted8810us@gmail.com> Co-authored-by: deepinsect <deepinsect@github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: cp90 <153345481+cp90-pixel@users.noreply.github.com> Co-authored-by: Cedric <14017092+douyixuan@users.noreply.github.com> Co-authored-by: Simon He <57086651+Simon-He95@users.noreply.github.com> Co-authored-by: yyhhyyyyyy <yyhhyyyyyy8@gmail.com> Co-authored-by: cnJasonZ <gbdzxalbb@qq.com>
#1014
Summary by CodeRabbit
New Features
Bug Fixes