feat: implement cookie-based Copilot authentication#31
Conversation
- Remove WebView-based OAuth login (AuthManager, UsageFetcher, LoginView) - Implement browser cookie extraction from Chrome/Brave/Arc/Edge - Add multi-profile support for cookie discovery - Refactor CopilotProvider to use cookies for GitHub API auth - Add thread-safe caching to TokenManager with serial queue - Display Copilot quota (1500 requests) in Quota Status section - Display Copilot add-on costs in Pay-as-you-go section with 30-day history Co-authored-by: Claude <no-reply@claude.ai> Co-authored-by: Claude (Sisyphus, oMo) <no-reply@claude.ai> Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
There was a problem hiding this comment.
Pull request overview
This PR successfully refactors GitHub Copilot integration from WebView-based OAuth to cookie-based authentication, improving security and simplifying the authentication flow. The changes remove three WebView-dependent files and update the authentication mechanism to read cookies directly from Chromium-based browsers (Chrome, Brave, Arc, Edge).
Changes:
- Removed WebView-based authentication infrastructure (AuthManager, UsageFetcher, LoginView)
- Implemented cookie-based authentication with multi-browser profile support
- Enhanced thread safety with caching in TokenManager using serial dispatch queue
- Integrated CopilotProvider into the unified multi-provider system
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| LoginView.swift | Deleted WebView-based login UI (no longer needed) |
| UsageFetcher.swift | Deleted WebView-based usage fetcher (replaced by cookie-based API calls) |
| AuthManager.swift | Deleted WebView OAuth manager (replaced by BrowserCookieService) |
| TokenManager.swift | Added thread-safe caching with serial queue and 30-second cache validity |
| BrowserCookieService.swift | Enhanced with multi-profile discovery for Chrome/Brave/Arc/Edge browsers |
| CopilotProvider.swift | Refactored to use cookie-based authentication via URLSession instead of WebView |
| StatusBarController.swift | Removed WebView-specific menu items and integrated Copilot into generic provider system |
| AppDelegate.swift | Removed WebView window management and notification observers |
| ProviderManager.swift | Re-enabled CopilotProvider in default providers list |
| project.pbxproj | Removed deleted files from build configuration |
| AGENTS.md | Updated build commands with DerivedData placeholder pattern |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add Copilot Add-on to Pay-as-you-go section with overage cost and Usage History submenu - Add custom Copilot submenu in Quota Status with progress bar, used/limit display - Add copilotOverageCost, copilotOverageRequests, copilotUsedRequests, copilotLimitRequests to DetailedUsage - Update CopilotProvider to include overage data in ProviderResult - Change Copilot displayName from 'Copilot Add-on' to 'Copilot' for Quota Status Co-authored-by: Claude (Sisyphus, oMo) <no-reply@anthropic.com> Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
- Added serial dispatch queue (cacheQueue) for thread-safe access - Changed cachedUserEmail and cachedCustomerId to computed properties - Backing storage uses underscore-prefixed private variables Co-authored-by: Claude (Sisyphus, oMo) <no-reply@anthropic.com> Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
- Keep thread-safe caching in TokenManager.swift - Merge updates from main (AGENTS.md, StatusBarController, Info.plist, OpenCodeZenProvider) Co-authored-by: Claude (Sisyphus, oMo) <no-reply@anthropic.com> Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
CopilotMonitor/CopilotMonitor/App/StatusBarController.swift:416
- The calculatePayAsYouGoTotal function only sums up costs from .payAsYouGo usage types, but Copilot uses a .quotaBased type with overage costs stored in details.copilotOverageCost. This means Copilot's overage costs are not included in the pay-as-you-go total calculation. The function should also check for copilotOverageCost in the details of quota-based providers and include it in the total.
private func calculatePayAsYouGoTotal(providerResults: [ProviderIdentifier: ProviderResult]) -> Double {
var total = 0.0
for (_, result) in providerResults {
if case .payAsYouGo(_, let cost, _) = result.usage, let cost = cost {
total += cost
}
}
return total
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| switch self { | ||
| case .copilot: | ||
| return "Copilot Add-on" | ||
| return "Copilot" |
There was a problem hiding this comment.
The display name was changed from "Copilot Add-on" to "Copilot", but the StatusBarController still uses "Copilot Add-on" in multiple places (lines 505, 512, 560 in StatusBarController.swift). This creates inconsistency where the provider's displayName is "Copilot" but the menu shows "Copilot Add-on". Either revert this change to keep "Copilot Add-on" or update the StatusBarController to use the provider's displayName property instead of hardcoding "Copilot Add-on".
| return "Copilot" | |
| return "Copilot Add-on" |
There was a problem hiding this comment.
❌ Not fixing - Intentional Design: The displayName 'Copilot' is used in the Quota Status section (quota-based display), while 'Copilot Add-on' in StatusBarController refers to the Pay-as-you-go section (overage billing). These are semantically different contexts:
- Quota Status: Shows base quota usage (1500 requests) → uses 'Copilot'
- Pay-as-you-go: Shows overage costs beyond quota → uses 'Copilot Add-on'
This is consistent with GitHub's billing model where the add-on costs are separate from the base quota.
| submenu.addItem(authItem) | ||
| } | ||
|
|
||
| addOnItem.submenu = submenu |
There was a problem hiding this comment.
The "Open Billing" menu item that was present in the old Copilot submenu has been removed from the new implementation. The openBillingClicked method still exists at line 927, but is no longer connected to any menu item. If this was an intentional removal, consider removing the unused openBillingClicked method. Otherwise, add the "Open Billing" menu item back to the Copilot submenu before the closing of the submenu.
There was a problem hiding this comment.
✅ Will investigate: You're right that openBillingClicked() is orphaned. I'll either:
- Remove the unused method if billing page access is no longer needed
- Or re-add the 'Open Billing' menu item
Since cookie-based auth still provides access to GitHub, keeping the billing link is useful. Will add it back to the Copilot submenu.
| private var _cachedUserEmail: String? | ||
| private var _cachedCustomerId: String? | ||
|
|
||
| /// Initialize with WebView for API access | ||
| /// - Parameter webView: WebView instance from AuthManager for authenticated requests | ||
| init(webView: WKWebView) { | ||
| self.webView = webView | ||
| private var cachedUserEmail: String? { | ||
| get { cacheQueue.sync { _cachedUserEmail } } | ||
| set { cacheQueue.sync { _cachedUserEmail = newValue } } | ||
| } | ||
|
|
||
| private var cachedCustomerId: String? { | ||
| get { cacheQueue.sync { _cachedCustomerId } } | ||
| set { cacheQueue.sync { _cachedCustomerId = newValue } } | ||
| } |
There was a problem hiding this comment.
The cachedUserEmail is defined and used in the DetailedUsage result (line 86), but it is never populated anywhere in the code. The old implementation had a fetchUserEmail method that called the GitHub /api/v3/user endpoint to retrieve the email, but this has been removed. Without fetching and setting the email, it will always be nil in the UI. Consider adding a method to fetch the user's email from the GitHub API using the cookies, or remove the email field from the result if it's no longer needed.
There was a problem hiding this comment.
✅ Valid issue - Will fix: The email fetching was lost during the WebView → cookie refactoring. I'll add a method to fetch user email from GitHub API using the session cookies. This is needed for the 'Email: xxx@email.com' display in the menu.
| final class TokenManager: @unchecked Sendable { | ||
| static let shared = TokenManager() | ||
|
|
||
| /// Serial queue for thread-safe file access | ||
| private let queue = DispatchQueue(label: "com.opencodeproviders.TokenManager") | ||
|
|
||
| /// Cached auth data with timestamp | ||
| private var cachedAuth: OpenCodeAuth? | ||
| private var cacheTimestamp: Date? | ||
| private let cacheValiditySeconds: TimeInterval = 30 // Cache for 30 seconds | ||
|
|
||
| /// Cached antigravity accounts | ||
| private var cachedAntigravityAccounts: AntigravityAccounts? | ||
| private var antigravityCacheTimestamp: Date? |
There was a problem hiding this comment.
The @unchecked Sendable conformance is used here, but the cache properties (cachedAuth, cacheTimestamp, cachedAntigravityAccounts, antigravityCacheTimestamp) are not properly isolated. While these are accessed through queue.sync, which provides synchronization, the @unchecked Sendable annotation should be used carefully. Consider documenting why @unchecked Sendable is safe here, or use proper Sendable isolation by making the class an actor instead.
There was a problem hiding this comment.
❌ Not fixing - By Design: The @unchecked Sendable is intentional and safe because:
- All cache property access is serialized through
queue.sync {}dispatch - Converting to an actor would require async/await throughout the codebase, which is a larger refactor
- The serial queue provides the same isolation guarantees as an actor
Added a comment would add unnecessary verbosity. The queue.sync pattern is a well-known Swift concurrency primitive.
| private func debugLog(_ message: String) { | ||
| #if DEBUG | ||
| let msg = "[\(Date())] BrowserCookieService: \(message)\n" | ||
| if let data = msg.data(using: .utf8) { | ||
| let path = "/tmp/cookie_debug.log" | ||
| if FileManager.default.fileExists(atPath: path) { | ||
| if let handle = FileHandle(forWritingAtPath: path) { | ||
| handle.seekToEndOfFile() | ||
| handle.write(data) | ||
| handle.closeFile() | ||
| } | ||
| } else { | ||
| try? data.write(to: URL(fileURLWithPath: path)) | ||
| } | ||
| } | ||
| #endif | ||
| } |
There was a problem hiding this comment.
The debug log implementation writes to a file but doesn't properly handle errors when the file handle operations fail. The closeFile() call on line 26 could throw, and file write operations could fail silently. While this is debug-only code, consider wrapping the file operations in a do-catch block to ensure resources are properly cleaned up even if writes fail.
There was a problem hiding this comment.
❌ Not fixing: Same as previous comment - this is DEBUG-only logging code (#if DEBUG). The current Swift API (seekToEndOfFile(), write(), closeFile()) doesn't throw in this version. Adding try-catch for non-throwing operations would cause compiler warnings. This code is not included in release builds.
- Restored getDebugEnvironmentInfo() method required by StatusBarController - Method returns debug environment info as string for error dialogs Co-authored-by: Claude (Sisyphus, oMo) <no-reply@anthropic.com> Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Summary
Fixes #15. This PR refactors GitHub Copilot integration to use browser cookies instead of WebView-based OAuth, improving security and simplifying the authentication flow.
Changes
Removed (WebView-based authentication)
AuthManager.swift- WebView OAuth managerUsageFetcher.swift- WebView-based usage fetcherLoginView.swift- WebView sign-in UIAdded/Modified
Features
Testing