Fix BYOK regression for non-OAuth Copilot token pathways (eval proxy)#318092
Merged
Conversation
Introduce hasCopilotTokenSource on IAuthenticationService, defaulted to !!anyGitHubSession in BaseAuthenticationService and overridden to true in StaticGitHubAuthenticationService (proxy/HMAC, eval harness). Replace PR-introduced anyGitHubSession guards added in #317428 at sites that gate on 'can we mint a Copilot token?' so the eval proxy pathway is no longer short-circuited: - languageModelAccess._getToken - authentication.contribution.waitForChatEnabled - copilotCli (3 sites) - remoteEmbeddingsComputer Group-B sites that intentionally gate on a real signed-in GitHub user (conversationFeature search/intent provider registration, byokUtilityModel notification) keep using anyGitHubSession.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a BYOK regression where several Copilot token-dependent pathways were incorrectly gated on the presence of a cached GitHub OAuth session (anyGitHubSession), preventing model publication in non-OAuth/proxy/HMAC/eval-harness scenarios where getCopilotToken() can still succeed.
Changes:
- Introduces
IAuthenticationService.hasCopilotTokenSource(defaulting to!!anyGitHubSessioninBaseAuthenticationService) and overrides it totrueinStaticGitHubAuthenticationService. - Switches key “can we mint a Copilot token?” gates from
anyGitHubSessiontohasCopilotTokenSource(LM access, chat enablement waiting, Copilot CLI model publishing, remote embeddings). - Adds/updates tests and test doubles/mocks to include the new predicate.
Show a summary per file
| File | Description |
|---|---|
| extensions/copilot/src/platform/authentication/common/authentication.ts | Adds hasCopilotTokenSource to the auth interface and a default implementation on BaseAuthenticationService. |
| extensions/copilot/src/platform/authentication/common/staticGitHubAuthenticationService.ts | Overrides hasCopilotTokenSource to support non-OAuth token pathways. |
| extensions/copilot/src/extension/conversation/vscode-node/languageModelAccess.ts | Updates token gating to use hasCopilotTokenSource before calling getCopilotToken(). |
| extensions/copilot/src/extension/authentication/vscode-node/authentication.contribution.ts | Uses hasCopilotTokenSource to avoid waiting for a token in BYOK/air-gapped scenarios. |
| extensions/copilot/src/extension/chatSessions/copilotcli/node/copilotCli.ts | Uses hasCopilotTokenSource to gate CLI model fetching/publishing. |
| extensions/copilot/src/platform/embeddings/common/remoteEmbeddingsComputer.ts | Uses hasCopilotTokenSource to gate embeddings computation. |
| extensions/copilot/src/platform/authentication/test/node/authentication.spec.ts | Adds coverage ensuring static auth reports a token source even without a GitHub session. |
| extensions/copilot/src/platform/ignore/node/test/mockAuthenticationService.ts | Updates mock auth service to include hasCopilotTokenSource. |
| extensions/copilot/src/lib/vscode-node/test/getInlineCompletions.spec.ts | Updates test auth service to include hasCopilotTokenSource. |
| extensions/copilot/src/extension/chatSessions/copilotcli/node/test/copilotCliModels.spec.ts | Updates test double to expose hasCopilotTokenSource. |
Copilot's findings
- Files reviewed: 10/10 changed files
- Comments generated: 2
…oxy scenarios - Update languageModelAccess log to say 'Copilot token source' (matches the new gate). - In remoteEmbeddingsComputer, return empty embeddings instead of throwing when the Dotcom path lacks a GitHub access token (proxy/HMAC eval harness), matching the rest of the function's empty-fallback behavior.
Contributor
Author
|
/requires-eval-assessment |
dmitrivMS
added
the
~requires-eval-assessment
alexdima
approved these changes
May 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes microsoft/vscode-copilot-evaluation#3977
Problem
PR #317428 added
anyGitHubSessionguards in front of several Copilot token pathways to handle the air-gapped/BYOK case. That works for end-user OAuth scenarios, but it conflates "has a cached GitHub OAuth session" with "can obtain a Copilot token". In eval-harness/proxy/HMAC scenarios these are uncorrelated:getCopilotToken()resolves successfully via the alternate auth path, butanyGitHubSessionisundefined, so the new guard short-circuits before the real auth pathway runs and thecopilot/*LM provider publishes zero models.Fix
Add a
hasCopilotTokenSource: booleanpredicate toIAuthenticationService. It is defaulted onBaseAuthenticationServiceto!!this._anyGitHubSession(so the realAuthenticationServicekeeps existing behavior) and overridden totrueonStaticGitHubAuthenticationService, which exists precisely to represent non-OAuth Copilot token pathways (proxy/HMAC, eval harness). Group-A call sites that gate on "can we mint a Copilot token?" are switched tohasCopilotTokenSource:languageModelAccess._getToken(the original eval regression site)authentication.contribution.waitForChatEnabledcopilotCli(_fetchAndCacheModels,getModels,provideLanguageModelChatInformation)remoteEmbeddingsComputerGroup-B sites that intentionally gate on a real signed-in GitHub user keep using
anyGitHubSession(they pair withcopilotToken?.isNoAuthUserand are explicitly about Copilot-as-signed-in-user):conversationFeatureAI search / settings-search / participant-detection provider registrationbyokUtilityModel.contribution(the whole notification is about being signed out)Tests
Added a test in
authentication.spec.tsverifyingStaticGitHubAuthenticationService.hasCopilotTokenSource === trueeven whenanyGitHubSessionis undefined. Updated test doubles (mockAuthenticationService,getInlineCompletions.spec,copilotCliModels.spec) to expose the new property.All impacted vitest specs pass; 0 TypeScript errors.