Skip to content

Conversation

@douyixuan
Copy link
Contributor

@douyixuan douyixuan commented Oct 16, 2025

  • Fixed request header for managing OAuth tokens and retrieving API tokens.
  • Enhanced model definitions and added new models for better compatibility.

#1014

Summary by CodeRabbit

  • New Features

    • Added GitHub Copilot device-flow login with token validation and global device-flow instance.
    • Provider can reinitialize after proxy resolution; emits frontend update events when auth changes.
    • Falls back to alternate token path and model list when primary sources are unavailable.
  • Bug Fixes

    • Clearer, user-facing error messages for authentication, token exchange, polling and streaming failures.
    • Improved token validation, polling stability and resource cleanup for more reliable sign-in and usage.

…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.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 16, 2025

Note

Other AI code review bot(s) detected

CodeRabbit 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.

Walkthrough

Refactors 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

Cohort / File(s) Summary
Device Flow Token Management
src/main/presenter/githubCopilotDeviceFlow.ts
Added CopilotTokenResponse, ApiToken, CopilotConfig interfaces; added getCopilotToken() and checkExistingAuth(); expanded polling logic (attempts → 100, stabilized interval, slow_down handling); improved error messages and logging; added dispose(); added global accessors getGlobalGitHubCopilotDeviceFlow() and disposeGlobalGitHubCopilotDeviceFlow().
Provider Check Error Handling
src/main/presenter/llmProviderPresenter/index.ts
When modelId is absent, wraps fallback provider.check() in try/catch, logs fallback, and returns a structured { isOk: false, errorMsg } on failure instead of throwing.
GitHub Copilot Provider Refactor
src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
Initializes and prefers global deviceFlow for tokens; getCopilotToken() favors device flow with API-key fallback; renamed maxTokens_maxTokens in coreStream() and completions(); added onProxyResolved() hook; condensed logging, updated request headers/payloads, improved 403/401/404 handling, and added fallback model provisioning.
OAuth and Device Flow Integration
src/main/presenter/oauthPresenter.ts
Replaced local factory with getGlobalGitHubCopilotDeviceFlow(); added pre-check via checkExistingAuth() to avoid redundant login; uses eventBus to emit providerUpdated on successful login; refactored logging and error messages; persists tokens to provider config.
OAuth Logging Simplification
src/main/presenter/githubCopilotOAuth.ts
Simplified dev logging: replaced masked credential prints with boolean-configured flags and changed success message to a generic form.
OAuth Helper Logging
src/main/presenter/llmProviderPresenter/oauthHelper.ts
Replaced logging of the raw authorization code with a generic success message; flow and event dispatch unchanged.

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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • zerob13

Poem

🐇
I nibbled code beneath the moonlit log,
tokens traded through a patient fog.
A singleton hopped in, tidy and true,
polling soft beats until auth came through.
Hooray — fresh carrots for the new token stew! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "feat: enhance GitHub Copilot Device Flow with OAuth token management and API token retrieval" accurately reflects the primary changes in the changeset. The modifications center on adding OAuth token management capabilities, API token retrieval functionality (getCopilotToken, checkExistingAuth methods), implementing a global singleton pattern for device flow management, and integrating these enhancements into the GitHub Copilot provider. The title is specific and clear about what is being enhanced and why, avoiding vague terms while remaining concise. A developer reviewing the git history would understand that this PR introduces OAuth and API token handling improvements to the device flow authentication system.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 raw Authorization: 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

startDeviceFlow returns the access token but never assigns it to this.oauthToken. As a result getCopilotToken() 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

📥 Commits

Reviewing files that changed from the base of the PR and between 0302f2d and 65bd929.

📒 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.ts
  • src/main/presenter/githubCopilotDeviceFlow.ts
  • src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
  • src/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.ts
  • src/main/presenter/githubCopilotDeviceFlow.ts
  • src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
  • src/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.ts
  • src/main/presenter/githubCopilotDeviceFlow.ts
  • src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
  • src/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.ts
  • src/main/presenter/githubCopilotDeviceFlow.ts
  • src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
  • src/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.ts should manage the overall Agent loop, conversation history, tool execution via McpPresenter, and frontend communication via eventBus.
The main Agent loop in llmProviderPresenter/index.ts should handle multi-round LLM calls and tool usage, maintaining conversation state and controlling the loop with needContinueConversation and toolCallCount.
The main Agent loop should send standardized STREAM_EVENTS (RESPONSE, END, ERROR) to the frontend via eventBus.
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.ts
  • src/main/presenter/githubCopilotDeviceFlow.ts
  • src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
  • src/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.ts
  • src/main/presenter/githubCopilotDeviceFlow.ts
  • src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
  • src/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.ts
  • src/main/presenter/githubCopilotDeviceFlow.ts
  • src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
  • src/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.ts
  • src/main/presenter/githubCopilotDeviceFlow.ts
  • src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
  • src/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.ts
  • src/main/presenter/githubCopilotDeviceFlow.ts
  • src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
  • src/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.ts
  • src/main/presenter/githubCopilotDeviceFlow.ts
  • src/main/presenter/llmProviderPresenter/providers/githubCopilotProvider.ts
  • src/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 in src/main/presenter/llmProviderPresenter/providers/*.ts should 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 a coreStream method that yields standardized stream events to decouple the main loop from provider-specific details.
The coreStream method 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 as formatMessages, convertToProviderTools, parseFunctionCalls, and prepareFunctionCallPrompt as 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., using convertToProviderTools) 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 appropriate stop_reason in 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)

@zerob13
Copy link
Collaborator

zerob13 commented Oct 16, 2025

@codex review

@zerob13 zerob13 requested a review from yyhhyyyyyy October 16, 2025 09:34
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a 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

Comment on lines 412 to 521
// Increase polling interval
if (this.pollingInterval) {
clearInterval(this.pollingInterval)
this.pollingInterval = setInterval(poll, (deviceCodeResponse.interval + 5) * 1000)
this.pollingInterval = setInterval(poll, currentInterval * 1000)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Copy link
Collaborator

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines 149 to 335
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
}
Copy link
Collaborator

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.

@douyixuan
Copy link
Contributor Author

douyixuan commented Oct 17, 2025 via email

Copy link
Collaborator

@yyhhyyyyyy yyhhyyyyyy left a 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.

Comment on lines 126 to 129
} catch (error) {
this.oauthToken = null
return null
}
Copy link
Collaborator

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.

Comment on lines 138 to 141
} catch (error) {
this.oauthToken = null
}
}
Copy link
Collaborator

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.

Comment on lines 559 to 561
} catch (error) {
// 继续轮询,网络错误可能是暂时的
}
Copy link
Collaborator

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.

Comment on lines +180 to +182
} catch (error) {
throw error
}
Copy link
Collaborator

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.

Comment on lines 79 to 81
} catch (error) {
console.warn('[GitHub Copilot] Device flow failed, using provider API key')
}
Copy link
Collaborator

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.

Comment on lines 570 to 572
} catch (e) {
console.log(` Could not read error response: ${e}`)
// ignore
}
Copy link
Collaborator

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 resolve instead 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 = true

Also 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 = null

Additionally, 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/presenter

Also 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

📥 Commits

Reviewing files that changed from the base of the PR and between 65bd929 and cc787fc.

📒 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.ts
  • src/main/presenter/llmProviderPresenter/oauthHelper.ts
  • src/main/presenter/oauthPresenter.ts
  • src/main/presenter/githubCopilotDeviceFlow.ts
  • 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/githubCopilotOAuth.ts
  • src/main/presenter/llmProviderPresenter/oauthHelper.ts
  • src/main/presenter/oauthPresenter.ts
  • src/main/presenter/githubCopilotDeviceFlow.ts
  • 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/githubCopilotOAuth.ts
  • src/main/presenter/llmProviderPresenter/oauthHelper.ts
  • src/main/presenter/oauthPresenter.ts
  • src/main/presenter/githubCopilotDeviceFlow.ts
  • 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/githubCopilotOAuth.ts
  • src/main/presenter/llmProviderPresenter/oauthHelper.ts
  • src/main/presenter/oauthPresenter.ts
  • src/main/presenter/githubCopilotDeviceFlow.ts
  • 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/githubCopilotOAuth.ts
  • src/main/presenter/llmProviderPresenter/oauthHelper.ts
  • src/main/presenter/oauthPresenter.ts
  • src/main/presenter/githubCopilotDeviceFlow.ts
  • 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/githubCopilotOAuth.ts
  • src/main/presenter/llmProviderPresenter/oauthHelper.ts
  • src/main/presenter/oauthPresenter.ts
  • src/main/presenter/githubCopilotDeviceFlow.ts
  • 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/githubCopilotOAuth.ts
  • src/main/presenter/llmProviderPresenter/oauthHelper.ts
  • src/main/presenter/oauthPresenter.ts
  • src/main/presenter/githubCopilotDeviceFlow.ts
  • 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/githubCopilotOAuth.ts
  • src/main/presenter/llmProviderPresenter/oauthHelper.ts
  • src/main/presenter/oauthPresenter.ts
  • src/main/presenter/githubCopilotDeviceFlow.ts
  • 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/githubCopilotOAuth.ts
  • src/main/presenter/llmProviderPresenter/oauthHelper.ts
  • src/main/presenter/oauthPresenter.ts
  • src/main/presenter/githubCopilotDeviceFlow.ts
  • 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/githubCopilotOAuth.ts
  • src/main/presenter/llmProviderPresenter/oauthHelper.ts
  • src/main/presenter/oauthPresenter.ts
  • src/main/presenter/githubCopilotDeviceFlow.ts
  • 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 in src/main/presenter/llmProviderPresenter/providers/*.ts should 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 a coreStream method that yields standardized stream events to decouple the main loop from provider-specific details.
The coreStream method 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 as formatMessages, convertToProviderTools, parseFunctionCalls, and prepareFunctionCallPrompt as 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., using convertToProviderTools) 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 appropriate stop_reason in 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

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 _maxTokens since it's actively used.

The underscore prefix typically signals an intentionally unused parameter, but _maxTokens is 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 _maxTokens parameter.

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. Since getCopilotToken() 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 apiKey when 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

📥 Commits

Reviewing files that changed from the base of the PR and between cc787fc and 4891353.

📒 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 in src/main/presenter/llmProviderPresenter/providers/*.ts should 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 a coreStream method that yields standardized stream events to decouple the main loop from provider-specific details.
The coreStream method 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 as formatMessages, convertToProviderTools, parseFunctionCalls, and prepareFunctionCallPrompt as 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., using convertToProviderTools) 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 appropriate stop_reason in 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 in coreStream method.

This method has the same two issues identified in coreStream:

  1. Line 542: Misleading _maxTokens parameter naming (used at Line 554 and 574)
  2. Lines 607-616: Chinese error messages violating coding guidelines
  3. Lines 606-622: Duplicate error handling logic from coreStream (lines 440-457)

Please apply the same fixes as suggested for the coreStream method. Additionally, extract the shared error handling logic into a private helper method like handleApiErrorResponse(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 checkExistingAuth returns 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

📥 Commits

Reviewing files that changed from the base of the PR and between 4891353 and b1d4bc5.

📒 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 in src/main/presenter/llmProviderPresenter/providers/*.ts should 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 a coreStream method that yields standardized stream events to decouple the main loop from provider-specific details.
The coreStream method 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 as formatMessages, convertToProviderTools, parseFunctionCalls, and prepareFunctionCallPrompt as 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., using convertToProviderTools) 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 appropriate stop_reason in 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 to completions.

The summaries, generateText, and summaryTitles methods are clean wrappers that properly construct message arrays and delegate to the completions method.


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, possibly gpt-4o-2024-05-13, gpt-3.5-turbo). Additionally, verify model ID naming (e.g., o3-mini vs 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,
Copy link
Contributor

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.

Comment on lines +440 to +457
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}`
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Translate error messages to English and extract duplicate error handling.

Two issues:

  1. Error message is in Chinese, violating the guideline: "Use English for all logs and comments"
  2. The same 403 error handling logic is duplicated in the completions method (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.

Suggested change
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.

Comment on lines +696 to 714
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 访问`
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

Suggested change
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.).

Copy link
Collaborator

@yyhhyyyyyy yyhhyyyyyy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yyhhyyyyyy yyhhyyyyyy merged commit b3e5e93 into ThinkInAIXYZ:dev Oct 19, 2025
2 checks passed
zerob13 added a commit that referenced this pull request Oct 22, 2025
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants