Skip to content

Deduplicate GitHub Copilot accounts #140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Daltonganger wants to merge 4 commits into
base: main
Choose a base branch
from
+250 −11
Open

Deduplicate GitHub Copilot accounts #140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cff7b87
Deduplicate GitHub Copilot accounts
May 20, 2026
1af5491
Tighten GitHub Copilot dedupe
May 20, 2026
8b30e86
Require identity overlap for Copilot dedupe
May 20, 2026
4265975
Share Copilot dedupe safeguards
May 20, 2026
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
50 changes: 45 additions & 5 deletions CopilotMonitor/CLI/Providers/CopilotCLIProvider.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,20 @@ actor CopilotCLIProvider: ProviderProtocol {
let usage: ProviderUsage
let details: DetailedUsage
let sourcePriority: Int
let isPlaceholder: Bool

var dedupeInput: CopilotCandidateDedupeInput {
CopilotCandidateDedupeInput(
accountId: accountId,
email: details.email,
planType: details.planType,
totalEntitlement: usage.totalEntitlement,
remainingQuota: usage.remainingQuota,
usedRequests: details.copilotUsedRequests,
limitRequests: details.copilotLimitRequests,
isPlaceholder: isPlaceholder
)
}
}

// MARK: - ProviderProtocol Implementation
Expand Down Expand Up @@ -256,7 +270,8 @@ actor CopilotCLIProvider: ProviderProtocol {
accountId: login,
usage: providerUsage,
details: details,
sourcePriority: sourcePriority
sourcePriority: sourcePriority,
isPlaceholder: false
)
}

Expand Down Expand Up @@ -287,7 +302,8 @@ actor CopilotCLIProvider: ProviderProtocol {
accountId: info.accountId ?? info.login,
usage: providerUsage,
details: details,
sourcePriority: sourcePriority(info.source)
sourcePriority: sourcePriority(info.source),
isPlaceholder: false
)
}

Expand All @@ -297,10 +313,11 @@ actor CopilotCLIProvider: ProviderProtocol {
candidates: [CopilotAccountCandidate],
cookieCandidate: CopilotAccountCandidate?
) -> ProviderResult {
let candidates = removePlaceholderCandidatesWhenRealUsageExists(candidates)
let merged = CandidateDedupe.merge(
candidates,
accountId: { $0.accountId },
isSameUsage: { _, _ in false },
accountId: { CopilotCandidateDedupe.normalizedIdentity($0.accountId) },
isSameUsage: { isDuplicateCopilotUsage($0, $1) },
priority: { $0.sourcePriority }
)
let sorted = merged.sorted { $0.sourcePriority > $1.sourcePriority }
Expand Down Expand Up @@ -338,7 +355,7 @@ actor CopilotCLIProvider: ProviderProtocol {
)
}

let primaryDetails = cookieCandidate?.details ?? accountResults.first?.details
let primaryDetails = accountResults.first?.details ?? cookieCandidate?.details
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 20, 2026

Choose a reason for hiding this comment

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

P2 Badge Keep browser overage fields in CLI primary Copilot details

The same precedence change in the CLI provider makes details come from the first sorted account instead of the browser candidate; since token-derived candidates omit copilotOverageCost and copilotOverageRequests, CLI output can silently lose those fields whenever a token account outranks the browser account. This causes downstream tooling to read $0/missing add-on usage even when cookie-based usage data was successfully fetched.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 20, 2026

Choose a reason for hiding this comment

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

P2 Badge Keep browser overage fields in CLI primary Copilot output

The CLI provider now prefers accountResults.first?.details over cookieCandidate?.details, which can silently discard copilotOverageCost and copilotOverageRequests whenever a token-derived account is first in priority/order. In that case JSON consumers see missing/zero add-on usage despite successful cookie billing fetches, producing inaccurate automation/reporting outputs.

Useful? React with 👍 / 👎.


logger.info("CopilotCLIProvider: Finalized \(accountResults.count) account row(s)")

Expand All @@ -349,6 +366,29 @@ actor CopilotCLIProvider: ProviderProtocol {
)
}

private func removePlaceholderCandidatesWhenRealUsageExists(
_ candidates: [CopilotAccountCandidate]
) -> [CopilotAccountCandidate] {
let hasRealUsage = candidates.contains { candidate in
(candidate.usage.totalEntitlement ?? 0) > 0
}
guard hasRealUsage else { return candidates }

return candidates.filter { candidate in
!CopilotCandidateDedupe.shouldDropPlaceholder(
candidate.dedupeInput,
whenAnyCandidateHasRealUsage: hasRealUsage
)
}
}

private func isDuplicateCopilotUsage(
_ lhs: CopilotAccountCandidate,
_ rhs: CopilotAccountCandidate
) -> Bool {
CopilotCandidateDedupe.isSameAccountUsage(lhs.dedupeInput, rhs.dedupeInput)
}

Comment on lines +369 to +391
Copy link
Copy Markdown
Contributor

@op-gg-ai-devops op-gg-ai-devops Bot May 20, 2026

Choose a reason for hiding this comment

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

Minor: Style Duplicated dedup helpers across both Copilot providers: future fixes will drift
these four helpers (removePlaceholderCandidatesWhenRealUsageExists, isDuplicateCopilotUsage, copilotIdentityCandidates, normalizedCopilotIdentity) are now byte-for-byte identical to the ones in CopilotMonitor/CopilotMonitor/Providers/CopilotProvider.swift:395-466. AGENTS.md is pretty direct about this: "Don't make copy & paste duplicated code."

since the only thing tying them to the enclosing type is the private CopilotAccountCandidate struct (which itself has the same shape in both files), the cheap fix is to either:

  1. Lift CopilotAccountCandidate and the four helpers into a shared file (e.g. CopilotCandidateDedupe.swift) as a fileprivate-or-internal namespace, and have both providers call into it; or
  2. Make a small protocol CopilotCandidateOwner + default-implementation extension that both providers conform to.

otherwise the next time someone tweaks dedup behavior (and the previous review already shows that's a frequent area), they'll have to remember to mirror the change in both files — and the GUI/CLI drift problem the previous review flagged is right back.

// MARK: - Cookie-based Fetching (fallback)

private func fetchCustomerId(cookies: GitHubCookies) async -> String? {
Expand Down
77 changes: 77 additions & 0 deletions CopilotMonitor/CopilotMonitor/Models/ProviderResult.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1155,6 +1155,83 @@ struct CandidateDedupe {
}
}

struct CopilotCandidateDedupeInput {
let accountId: String?
let email: String?
let planType: String?
let totalEntitlement: Int?
let remainingQuota: Int?
let usedRequests: Int?
let limitRequests: Int?
let isPlaceholder: Bool
}

enum CopilotCandidateDedupe {
static func normalizedIdentity(_ value: String?) -> String? {
guard let value else { return nil }
let normalized = value
.trimmingCharacters(in: .whitespacesAndNewlines)
.lowercased()
return normalized.isEmpty ? nil : normalized
}

static func shouldDropPlaceholder(
_ candidate: CopilotCandidateDedupeInput,
whenAnyCandidateHasRealUsage hasRealUsage: Bool
) -> Bool {
guard hasRealUsage else { return false }
return candidate.isPlaceholder && (candidate.totalEntitlement ?? 0) == 0
}

static func isSameAccountUsage(
_ lhs: CopilotCandidateDedupeInput,
_ rhs: CopilotCandidateDedupeInput
) -> Bool {
guard lhs.totalEntitlement == rhs.totalEntitlement,
lhs.remainingQuota == rhs.remainingQuota,
(lhs.totalEntitlement ?? 0) > 0 else {
return false
}

if let lhsUsed = lhs.usedRequests,
let rhsUsed = rhs.usedRequests,
lhsUsed != rhsUsed {
return false
}

if let lhsLimit = lhs.limitRequests,
let rhsLimit = rhs.limitRequests,
lhsLimit != rhsLimit {
return false
}

let lhsPlan = normalizedIdentity(lhs.planType)
let rhsPlan = normalizedIdentity(rhs.planType)
if let lhsPlan, let rhsPlan, lhsPlan != rhsPlan {
return false
}

let lhsIdentity = identityCandidates(lhs)
let rhsIdentity = identityCandidates(rhs)
guard !lhsIdentity.isEmpty, !rhsIdentity.isEmpty else {
return false
}

return !lhsIdentity.isDisjoint(with: rhsIdentity)
}

private static func identityCandidates(_ candidate: CopilotCandidateDedupeInput) -> Set<String> {
var identities = Set<String>()
if let accountId = normalizedIdentity(candidate.accountId) {
identities.insert(accountId)
}
if let email = normalizedIdentity(candidate.email) {
identities.insert(email)
}
return identities
}
}

/// Shared numeric parser for API response dictionaries.
/// APIs may return Double, Int, NSNumber, or String for numeric fields.
enum APIValueParser {
Expand Down
53 changes: 47 additions & 6 deletions CopilotMonitor/CopilotMonitor/Providers/CopilotProvider.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,20 @@ final class CopilotProvider: ProviderProtocol {
let usage: ProviderUsage
let details: DetailedUsage
let sourcePriority: Int
let isPlaceholder: Bool

var dedupeInput: CopilotCandidateDedupeInput {
CopilotCandidateDedupeInput(
accountId: accountId,
email: details.email,
planType: details.planType,
totalEntitlement: usage.totalEntitlement,
remainingQuota: usage.remainingQuota,
usedRequests: details.copilotUsedRequests,
limitRequests: details.copilotLimitRequests,
isPlaceholder: isPlaceholder
)
}
}

private func sourcePriority(_ source: CopilotAuthSource?) -> Int {
Expand Down Expand Up @@ -289,7 +303,8 @@ final class CopilotProvider: ProviderProtocol {
accountId: login,
usage: providerUsage,
details: details,
sourcePriority: sourcePriority
sourcePriority: sourcePriority,
isPlaceholder: false
)
}

Expand Down Expand Up @@ -320,7 +335,8 @@ final class CopilotProvider: ProviderProtocol {
accountId: info.accountId ?? info.login,
usage: providerUsage,
details: details,
sourcePriority: sourcePriority(info.source)
sourcePriority: sourcePriority(info.source),
isPlaceholder: false
)
}

Expand All @@ -331,18 +347,20 @@ final class CopilotProvider: ProviderProtocol {
accountId: details.email,
usage: cachedResult.usage,
details: details,
sourcePriority: 0
sourcePriority: 0,
isPlaceholder: true
)
}

private func finalizeResult(
candidates: [CopilotAccountCandidate],
cookieCandidate: CopilotAccountCandidate?
) -> ProviderResult {
let candidates = removePlaceholderCandidatesWhenRealUsageExists(candidates)
Copy link
Copy Markdown
Contributor

@op-gg-ai-devops op-gg-ai-devops Bot May 20, 2026

Choose a reason for hiding this comment

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

Minor: Bug primaryDetails may use a filtered-out placeholder candidate
small consistency thing: cookieCandidate is captured before removePlaceholderCandidatesWhenRealUsageExists runs (line 342). when the cookie path fell back to an empty/placeholder result but a token candidate has real usage, the cookie candidate gets filtered out of accountResults — yet primaryDetails here still reaches for cookieCandidate?.details, so the top-level provider details could show stale/zeroed cookie data while the actual surviving account row is a token-based account.

probably safer to prefer accountResults.first?.details (highest-priority survivor) and only fall back to cookieCandidate?.details if accountResults is empty, or just check that the cookie candidate survived the placeholder filter before using it.

let merged = CandidateDedupe.merge(
candidates,
accountId: { $0.accountId },
isSameUsage: { _, _ in false },
accountId: { CopilotCandidateDedupe.normalizedIdentity($0.accountId) },
isSameUsage: { isDuplicateCopilotUsage($0, $1) },
priority: { $0.sourcePriority }
)
Comment on lines +359 to 365
Copy link
Copy Markdown
Contributor

@op-gg-ai-devops op-gg-ai-devops Bot May 20, 2026

Choose a reason for hiding this comment

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

Minor: Bug CLI provider still uses old dedup: GUI vs CLI behavior will drift
this PR upgrades the GUI CopilotProvider.finalizeResult to a proper identity-aware dedup, but the CLI sibling here still uses the old isSameUsage: { _, _ in false } and a raw $0.accountId key — so the CLI will still show duplicate Copilot rows for the same account discovered from cookies + tokens, and won't drop browser-cookie placeholders the way the GUI now does.

since both providers share the exact same CopilotAccountCandidate shape, the new normalizedCopilotIdentity / isDuplicateCopilotUsage / removePlaceholderCandidatesWhenRealUsageExists helpers could be lifted into a shared extension and reused here so the two paths don't drift. otherwise the CLI just inherits the bug this PR is fixing.

let sorted = merged.sorted { $0.sourcePriority > $1.sourcePriority }
Expand Down Expand Up @@ -380,7 +398,7 @@ final class CopilotProvider: ProviderProtocol {
)
}

let primaryDetails = cookieCandidate?.details ?? accountResults.first?.details
let primaryDetails = accountResults.first?.details ?? cookieCandidate?.details
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 20, 2026

Choose a reason for hiding this comment

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

P1 Badge Preserve cookie overage details for primary Copilot result

Selecting accountResults.first?.details before cookieCandidate?.details can drop valid overage data in multi-account setups: buildCandidateFromToken does not populate copilotOverageCost/copilotOverageRequests, so when a higher-priority token-backed account sorts first, the provider-level details lose the browser-fetched add-on fields even though cookieCandidate has them. In that case the app treats Copilot add-on spend as zero and can hide the add-on row, which is a billing-accuracy regression.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 20, 2026

Choose a reason for hiding this comment

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

P1 Badge Preserve cookie overage details as primary Copilot details

In finalizeResult, selecting accountResults.first?.details before cookieCandidate?.details can drop browser-derived overage fields when a token-backed account sorts first. Token candidates built in buildCandidateFromToken do not set copilotOverageCost/copilotOverageRequests, so this change can make the app report Copilot add-on spend as zero and hide the add-on spend row even when cookie usage fetched billing data successfully.

Useful? React with 👍 / 👎.


logger.info("CopilotProvider: Finalized \(accountResults.count) account row(s)")

Expand All @@ -391,6 +409,29 @@ final class CopilotProvider: ProviderProtocol {
)
}

private func removePlaceholderCandidatesWhenRealUsageExists(
_ candidates: [CopilotAccountCandidate]
) -> [CopilotAccountCandidate] {
let hasRealUsage = candidates.contains { candidate in
(candidate.usage.totalEntitlement ?? 0) > 0
}
guard hasRealUsage else { return candidates }

return candidates.filter { candidate in
!CopilotCandidateDedupe.shouldDropPlaceholder(
candidate.dedupeInput,
whenAnyCandidateHasRealUsage: hasRealUsage
)
}
Comment on lines +412 to +425
Copy link
Copy Markdown
Contributor

@op-gg-ai-devops op-gg-ai-devops Bot May 20, 2026

Choose a reason for hiding this comment

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

Minor: Style duplicate wrappers + redundant bool param: future bug fixes will need two-file edits
the heavy lifting is already in CopilotCandidateDedupe, but this wrapper and its sibling isDuplicateCopilotUsage are now byte-identical with the ones in CopilotCLIProvider.swift:368-389. same goes for the dedupeInput computed property on CopilotAccountCandidate. consider lifting the wrappers into CopilotCandidateDedupe (e.g. static func filterPlaceholders<T>(_:dedupeInput:)) so the two provider paths can't drift again on the next bug fix.

also shouldDropPlaceholder(_:whenAnyCandidateHasRealUsage:) re-checks hasRealUsage even though both callers already early-return on it via the guard hasRealUsage else { return candidates } above. you can drop the bool parameter and let the function just answer "is this a draggable placeholder", which is what it really means.

}

private func isDuplicateCopilotUsage(
_ lhs: CopilotAccountCandidate,
_ rhs: CopilotAccountCandidate
) -> Bool {
CopilotCandidateDedupe.isSameAccountUsage(lhs.dedupeInput, rhs.dedupeInput)
}

// MARK: - Customer ID Fetching

private func fetchCustomerId(cookies: GitHubCookies) async -> String? {
Expand Down
81 changes: 81 additions & 0 deletions CopilotMonitor/CopilotMonitorTests/CopilotProviderTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,87 @@ final class CopilotProviderTests: XCTestCase {
XCTAssertEqual(CopilotAuthSource.vscodeApps.priority, 0)
}

// MARK: - CopilotCandidateDedupe

func testCopilotDedupeRejectsMatchingUsageWithDifferentIdentities() {
let personal = makeDedupeInput(accountId: "personal", email: "personal@example.com")
let work = makeDedupeInput(accountId: "work", email: "work@example.com")

XCTAssertFalse(CopilotCandidateDedupe.isSameAccountUsage(personal, work))
}

func testCopilotDedupeAcceptsMatchingUsageWithCaseInsensitiveIdentity() {
let first = makeDedupeInput(accountId: "Foo", email: "Foo@Example.com")
let second = makeDedupeInput(accountId: "foo", email: "foo@example.com")

XCTAssertTrue(CopilotCandidateDedupe.isSameAccountUsage(first, second))
}

func testCopilotDedupeRejectsIdentitylessMatchingUsage() {
let identified = makeDedupeInput(accountId: "foo", email: "foo@example.com")
let identityless = makeDedupeInput(accountId: nil, email: nil)

XCTAssertFalse(CopilotCandidateDedupe.isSameAccountUsage(identified, identityless))
}

func testCopilotDedupeDropsOnlyPlaceholderWithoutRealUsage() {
let placeholder = makeDedupeInput(
accountId: nil,
email: nil,
entitlement: 0,
remaining: 0,
isPlaceholder: true
)
let emptyRealCandidate = makeDedupeInput(
accountId: nil,
email: nil,
entitlement: 0,
remaining: 0,
isPlaceholder: false
)

XCTAssertTrue(
CopilotCandidateDedupe.shouldDropPlaceholder(
placeholder,
whenAnyCandidateHasRealUsage: true
)
)
XCTAssertFalse(
CopilotCandidateDedupe.shouldDropPlaceholder(
emptyRealCandidate,
whenAnyCandidateHasRealUsage: true
)
)
XCTAssertFalse(
CopilotCandidateDedupe.shouldDropPlaceholder(
placeholder,
whenAnyCandidateHasRealUsage: false
)
)
}
Comment on lines +147 to +202
Copy link
Copy Markdown
Contributor

@op-gg-ai-devops op-gg-ai-devops Bot May 20, 2026

Choose a reason for hiding this comment

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

Minor: Info missing coverage: plan-mismatch and used/limit-mismatch branches
nice that the regression cases are locked in. one gap left: isSameAccountUsage has explicit branches for plan-type mismatch (lhsPlan != rhsPlan) and used/limit-request mismatch (lhsUsed != rhsUsed, lhsLimit != rhsLimit) that nothing in this file exercises. worth a couple more cases — same accountId+email, identical entitlement/remaining, but different plan or different copilotUsedRequests — so a future refactor can't accidentally collapse those branches without a red test.


private func makeDedupeInput(
accountId: String?,
email: String?,
entitlement: Int = 300,
remaining: Int = 284,
usedRequests: Int? = 16,
limitRequests: Int? = 300,
planType: String? = "Individual",
isPlaceholder: Bool = false
) -> CopilotCandidateDedupeInput {
CopilotCandidateDedupeInput(
accountId: accountId,
email: email,
planType: planType,
totalEntitlement: entitlement,
remainingQuota: remaining,
usedRequests: usedRequests,
limitRequests: limitRequests,
isPlaceholder: isPlaceholder
)
}

private func loadFixture(named: String) -> Data {
let bundle = Bundle(for: type(of: self))
guard let url = bundle.url(forResource: named, withExtension: nil) else {
Expand Down
Loading