Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).

### Developer Experience

- Added deterministic coordinator and GitHub Copilot concurrency coverage for
account-scoped credential-refresh coalescing under parallel XCTest execution.
- Gated provider test doubles now suspend in-flight refreshes deterministically,
covering batch and single-account completions after account changes.
- The Mac XCTest suite is split into domain-focused test classes with narrowly
Expand Down
4 changes: 4 additions & 0 deletions CodexBarMac.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
A1000000000000000000000E /* RefreshTestSupport.swift in Sources */ = {isa = PBXBuildFile; fileRef = A2000000000000000000000E /* RefreshTestSupport.swift */; };
A1000000000000000000000F /* CredentialTestSupport.swift in Sources */ = {isa = PBXBuildFile; fileRef = A2000000000000000000000F /* CredentialTestSupport.swift */; };
A10000000000000000000010 /* NetworkTestSupport.swift in Sources */ = {isa = PBXBuildFile; fileRef = A20000000000000000000010 /* NetworkTestSupport.swift */; };
A10000000000000000000011 /* CredentialRefreshCoordinatorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A20000000000000000000011 /* CredentialRefreshCoordinatorTests.swift */; };
100000000000000000000047 /* CheckForUpdatesButton.swift in Sources */ = {isa = PBXBuildFile; fileRef = 200000000000000000000049 /* CheckForUpdatesButton.swift */; };
100000000000000000000048 /* Sparkle in Frameworks */ = {isa = PBXBuildFile; productRef = E00000000000000000000001 /* Sparkle */; };
100000000000000000000049 /* DashboardTextSize.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20000000000000000000004A /* DashboardTextSize.swift */; };
Expand Down Expand Up @@ -184,6 +185,7 @@
A2000000000000000000000E /* RefreshTestSupport.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RefreshTestSupport.swift; sourceTree = "<group>"; };
A2000000000000000000000F /* CredentialTestSupport.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CredentialTestSupport.swift; sourceTree = "<group>"; };
A20000000000000000000010 /* NetworkTestSupport.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NetworkTestSupport.swift; sourceTree = "<group>"; };
A20000000000000000000011 /* CredentialRefreshCoordinatorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CredentialRefreshCoordinatorTests.swift; sourceTree = "<group>"; };
/* End PBXFileReference section */

/* Begin PBXContainerItemProxy section */
Expand Down Expand Up @@ -369,6 +371,7 @@
A2000000000000000000000E /* RefreshTestSupport.swift */,
A2000000000000000000000F /* CredentialTestSupport.swift */,
A20000000000000000000010 /* NetworkTestSupport.swift */,
A20000000000000000000011 /* CredentialRefreshCoordinatorTests.swift */,
);
path = CodexBarMacTests;
sourceTree = "<group>";
Expand Down Expand Up @@ -486,6 +489,7 @@
A1000000000000000000000E /* RefreshTestSupport.swift in Sources */,
A1000000000000000000000F /* CredentialTestSupport.swift in Sources */,
A10000000000000000000010 /* NetworkTestSupport.swift in Sources */,
A10000000000000000000011 /* CredentialRefreshCoordinatorTests.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down
33 changes: 31 additions & 2 deletions CodexBarMac/Services/CopilotUsageProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,12 @@ public final class CopilotUsageProvider: UsageProvider {
private let oauthConfiguration: CopilotOAuthConfiguration
private let gitHubTokenResolver: GitHubTokenResolver
private let now: @Sendable () -> Date
private let onJoinInFlightRefresh: (@Sendable () -> Void)?
private let cliTokenCache = CopilotCLITokenCache()

public let providerID = ProviderID.copilot

public init(
public convenience init(
secretStore: any SecretStore = KeychainService(),
session: URLSession = .shared,
usageEndpoint: URL = URL(string: "https://api.github.com/copilot_internal/user")!,
Expand All @@ -39,6 +40,30 @@ public final class CopilotUsageProvider: UsageProvider {
oauthConfiguration: CopilotOAuthConfiguration = .bundled,
gitHubTokenResolver: (@Sendable (String?) throws -> String?)? = nil,
now: @escaping @Sendable () -> Date = { Date() }
) {
self.init(
secretStore: secretStore,
session: session,
usageEndpoint: usageEndpoint,
githubAPIBaseURL: githubAPIBaseURL,
tokenEndpoint: tokenEndpoint,
oauthConfiguration: oauthConfiguration,
gitHubTokenResolver: gitHubTokenResolver,
now: now,
onJoinInFlightRefresh: nil
)
}

init(
secretStore: any SecretStore,
session: URLSession,
usageEndpoint: URL,
githubAPIBaseURL: URL = URL(string: "https://api.github.com")!,
tokenEndpoint: URL,
oauthConfiguration: CopilotOAuthConfiguration,
gitHubTokenResolver: (@Sendable (String?) throws -> String?)? = nil,
now: @escaping @Sendable () -> Date,
onJoinInFlightRefresh: (@Sendable () -> Void)?
) {
self.secretStore = secretStore
self.session = session
Expand All @@ -50,6 +75,7 @@ public final class CopilotUsageProvider: UsageProvider {
try LocalCredentialDiscovery.gitHubAuthToken(for: username)
}
self.now = now
self.onJoinInFlightRefresh = onJoinInFlightRefresh
}

public func fetchUsage(for configuration: ProviderAccountConfiguration) async throws -> ProviderUsageResult {
Expand Down Expand Up @@ -449,7 +475,10 @@ public final class CopilotUsageProvider: UsageProvider {
_ credentials: CopilotCredentials,
keychainAccount: String
) async -> CopilotCredentialRefreshResult {
await Self.refreshCoordinator.run(for: keychainAccount) { [self] in
await Self.refreshCoordinator.run(
for: keychainAccount,
onJoinExistingTask: onJoinInFlightRefresh
) { [self] in
await performCredentialRefresh(credentials, keychainAccount: keychainAccount)
}
}
Expand Down
2 changes: 2 additions & 0 deletions CodexBarMac/Services/CredentialRefreshCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ actor CredentialRefreshCoordinator<Result: Sendable> {

func run(
for account: String,
onJoinExistingTask: (@Sendable () -> Void)? = nil,
operation: @escaping @Sendable () async -> Result
) async -> Result {
if let task = inFlightTasks[account] {
onJoinExistingTask?()
return await task.value
}

Expand Down
145 changes: 145 additions & 0 deletions CodexBarMacTests/CopilotProviderTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,99 @@ final class CopilotProviderTests: XCTestCase {
XCTAssertEqual(result.bars.first?.used, 25)
}

func testConcurrentCopilotFetchesCoalesceBrowserCredentialRefresh() async throws {
let now = Date(timeIntervalSince1970: 2_000_000_000)
let secretStore = InMemorySecretStore()
let configuration = ProviderAccountConfiguration(
id: "copilot.concurrent-refresh",
providerID: .copilot,
accountLabel: "octocat",
authMethod: .browserSession
)
try secretStore.saveSecret(
CopilotCredentialsParser.storedCredential(from: CopilotCredentials(
accessToken: "old-access",
username: "octocat",
refreshToken: "old-refresh",
expiresAt: 2_000_000_060,
refreshTokenExpiresAt: 2_100_000_000
)),
account: ProviderConfigurationStore.keychainAccount(for: configuration)
)

let refreshJoined = TestSignal()
let refreshGate = CopilotRefreshRequestGate()
let recorder = CopilotConcurrentRequestRecorder()
let sessionFixture = IsolatedTestURLSession { request in
if request.url?.path == "/github-token" {
recorder.recordRefreshRequest()
refreshGate.blockUntilReleased()
return (
HTTPURLResponse(
url: try XCTUnwrap(request.url),
statusCode: 200,
httpVersion: nil,
headerFields: nil
)!,
Data(#"{"access_token":"new-access","refresh_token":"new-refresh","expires_in":28800,"refresh_token_expires_in":15897600}"#.utf8)
)
}

recorder.recordUsageRequest(
authorization: request.value(forHTTPHeaderField: "Authorization")
)
return (
HTTPURLResponse(
url: try XCTUnwrap(request.url),
statusCode: 200,
httpVersion: nil,
headerFields: nil
)!,
Data(#"{"login":"octocat","copilot_plan":"individual_pro","quota_reset_date_utc":"2033-05-19T03:33:20Z","quota_snapshots":{"premium_interactions":{"entitlement":100,"remaining":75,"unlimited":false}}}"#.utf8)
)
}
defer {
refreshGate.release()
sessionFixture.invalidate()
}
let provider = CopilotUsageProvider(
secretStore: secretStore,
session: sessionFixture.session,
usageEndpoint: URL(string: "https://example.test/copilot-usage")!,
tokenEndpoint: URL(string: "https://example.test/github-token")!,
oauthConfiguration: CopilotOAuthConfiguration(clientID: "client", clientSecret: "secret"),
now: { now },
onJoinInFlightRefresh: { refreshJoined.signal() }
)

let results = try await withTestWatchdog(
timeout: .seconds(10),
failureMessage: "Copilot concurrent refresh did not finish within the test bound.",
onTimeout: {
refreshGate.release()
sessionFixture.invalidate()
}
) {
let first = Task { try await provider.fetchUsage(for: configuration) }
defer {
first.cancel()
refreshGate.release()
}
await refreshGate.waitUntilBlocked()

let second = Task { try await provider.fetchUsage(for: configuration) }
defer { second.cancel() }
await refreshJoined.wait()

refreshGate.release()
return try await [first.value, second.value]
}

XCTAssertEqual(recorder.refreshRequestCount, 1)
XCTAssertEqual(recorder.usageAuthorizations, ["token new-access", "token new-access"])
XCTAssertTrue(results.allSatisfy { $0.bars.first?.used == 25 })
}

func testCopilotUsageProviderDoesNotCacheActiveCLIAccountToken() async throws {
let tokenCounter = CopilotTokenResolverCounter()
let sessionConfiguration = URLSessionConfiguration.ephemeral
Expand Down Expand Up @@ -1305,3 +1398,55 @@ final class CopilotProviderTests: XCTestCase {
}

}

private final class CopilotRefreshRequestGate: @unchecked Sendable {
private let condition = NSCondition()
private let requestStarted = TestSignal()
private var released = false

deinit {}

func blockUntilReleased() {
condition.lock()
requestStarted.signal()
while !released {
condition.wait()
}
condition.unlock()
}

func waitUntilBlocked() async {
await requestStarted.wait()
}

func release() {
condition.lock()
released = true
condition.broadcast()
condition.unlock()
}
}

private final class CopilotConcurrentRequestRecorder: @unchecked Sendable {
private let lock = NSLock()
private var refreshRequests = 0
private var authorizations: [String?] = []

deinit {}

var refreshRequestCount: Int {
lock.withLock { refreshRequests }
}

var usageAuthorizations: [String?] {
lock.withLock { authorizations }
}

func recordRefreshRequest() {
lock.withLock { refreshRequests += 1 }
}

func recordUsageRequest(authorization: String?) {
lock.withLock { authorizations.append(authorization) }
}
}
Loading