From 9c3dc119e9d160189a4a53d44b51405634e16040 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 7 May 2026 08:54:11 +0000 Subject: [PATCH 1/3] Initial plan From e54306237408dd97fe97bb4318a48bf857afe9cf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 7 May 2026 09:20:22 +0000 Subject: [PATCH 2/3] Detect "Bad credentials"/401 errors and trigger re-auth Agent-Logs-Url: https://github.com/microsoft/vscode-pull-request-github/sessions/d7437db6-d6f8-4e32-80f2-d39c6660a4f2 Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> --- src/github/credentials.ts | 52 +++++++++++++- src/github/loggingOctokit.ts | 39 +++++++++- src/test/github/loggingOctokit.test.ts | 99 ++++++++++++++++++++++++++ 3 files changed, 188 insertions(+), 2 deletions(-) create mode 100644 src/test/github/loggingOctokit.test.ts diff --git a/src/github/credentials.ts b/src/github/credentials.ts index d4422d3607..12efafef79 100644 --- a/src/github/credentials.ts +++ b/src/github/credentials.ts @@ -61,6 +61,12 @@ export class CredentialStore extends Disposable { private _scopes: string[] = SCOPES_OLD; private _scopesEnterprise: string[] = SCOPES_OLD; private _isSamling: boolean = false; + private _handlingAuthError: Map> = new Map(); + private _lastAuthErrorHandledAt: Map = new Map(); + // Cooldown long enough to absorb retries from in-flight requests that were + // issued with the now-invalid token, but short enough that a token that + // is invalidated again soon after re-auth will still trigger another prompt. + private static readonly AUTH_ERROR_COOLDOWN_MS = 60_000; private _onDidChangeSessions: vscode.EventEmitter = new vscode.EventEmitter(); public readonly onDidChangeSessions = this._onDidChangeSessions.event; @@ -285,6 +291,48 @@ export class CredentialStore extends Disposable { return this.doCreate({ forceNewSession: reason ? { detail: reason } : true }); } + /** + * Handles authentication errors that surface from API calls (e.g. "Bad credentials" + * or 401 Unauthorized). Triggers a re-authentication prompt for the affected + * provider, deduplicating concurrent requests so we don't show multiple prompts + * when many in-flight calls fail at once. + */ + public async handleAuthError(authProviderId: AuthProvider): Promise { + // Only prompt if we currently believe we are authenticated for this provider. + // Otherwise the regular sign-in flow will handle it. + if (!this.isAuthenticated(authProviderId)) { + return { canceled: true }; + } + const inFlight = this._handlingAuthError.get(authProviderId); + if (inFlight) { + return inFlight; + } + // In-flight requests that were issued with the now-invalid token may continue + // to fail with auth errors for a short period after a successful re-auth. + // Suppress re-prompting for a cooldown window to avoid repeatedly nagging the + // user. + const lastHandled = this._lastAuthErrorHandledAt.get(authProviderId); + if (lastHandled !== undefined && (Date.now() - lastHandled) < CredentialStore.AUTH_ERROR_COOLDOWN_MS) { + return { canceled: true }; + } + Logger.appendLine(`Detected invalid GitHub${getGitHubSuffix(authProviderId)} credentials; prompting for re-authentication.`, CredentialStore.ID); + /* __GDPR__ + "auth.badCredentials" : {} + */ + this._telemetry.sendTelemetryEvent('auth.badCredentials'); + const reason = vscode.l10n.t('Your GitHub{0} authentication session is no longer valid. Please sign in again.', getGitHubSuffix(authProviderId)); + const promise = (async () => { + try { + return await this.recreate(reason); + } finally { + this._handlingAuthError.delete(authProviderId); + this._lastAuthErrorHandledAt.set(authProviderId, Date.now()); + } + })(); + this._handlingAuthError.set(authProviderId, promise); + return promise; + } + public async reset() { this._githubAPI = undefined; this._githubEnterpriseAPI = undefined; @@ -561,7 +609,9 @@ export class CredentialStore extends Disposable { }, }); - const rateLogger = new RateLogger(this._telemetry, isEnterprise(authProviderId)); + const rateLogger = new RateLogger(this._telemetry, isEnterprise(authProviderId), (_e) => { + void this.handleAuthError(authProviderId); + }); const github: GitHub = { octokit: new LoggingOctokit(octokit, rateLogger), graphql: new LoggingApolloClient(graphql, rateLogger), diff --git a/src/github/loggingOctokit.ts b/src/github/loggingOctokit.ts index f1c6e9cc81..a6c9295ab6 100644 --- a/src/github/loggingOctokit.ts +++ b/src/github/loggingOctokit.ts @@ -44,6 +44,35 @@ function isObject(value: unknown): value is Record { return typeof value === 'object' && value !== null; } +/** + * Detects whether an error from a REST (Octokit) or GraphQL (Apollo) call + * indicates that the GitHub authentication token is no longer valid. This + * happens when the token has been revoked or has expired and surfaces as + * either a 401 status, a "Bad credentials" message (REST), or a + * "401 Unauthorized" network error (GraphQL). + */ +export function isAuthError(e: unknown): boolean { + if (!isObject(e)) { + return false; + } + if (e.status === 401) { + return true; + } + const networkError = e.networkError; + if (isObject(networkError) && networkError.statusCode === 401) { + return true; + } + if (typeof e.message === 'string') { + if (e.message.includes('Bad credentials')) { + return true; + } + if (e.message.includes('401 Unauthorized')) { + return true; + } + } + return false; +} + export function getErrorCode(e: unknown): string | undefined { if (!isObject(e)) { return undefined; @@ -94,7 +123,7 @@ export class RateLogger { private hasLoggedLowRateLimit: boolean = false; private readonly _isInsiders: boolean; - constructor(private readonly telemetry: ITelemetry, private readonly errorOnFlood: boolean) { + constructor(private readonly telemetry: ITelemetry, private readonly errorOnFlood: boolean, private readonly authErrorHandler?: (e: unknown) => void) { this._isInsiders = vscode.env.appName.toLowerCase().includes('insider'); } @@ -187,6 +216,14 @@ export class RateLogger { } */ this.telemetry.sendTelemetryErrorEvent('pr.apiCallFailed', properties); + + if (this.authErrorHandler && isAuthError(e)) { + try { + this.authErrorHandler(e); + } catch { + // Ignore errors from the auth error handler so they don't propagate. + } + } }); } diff --git a/src/test/github/loggingOctokit.test.ts b/src/test/github/loggingOctokit.test.ts new file mode 100644 index 0000000000..715fc75461 --- /dev/null +++ b/src/test/github/loggingOctokit.test.ts @@ -0,0 +1,99 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { default as assert } from 'assert'; +import { isAuthError, RateLogger } from '../../github/loggingOctokit'; +import { MockTelemetry } from '../mocks/mockTelemetry'; + +describe('loggingOctokit', () => { + describe('isAuthError', () => { + it('returns true for an Octokit-style 401 with "Bad credentials" message', () => { + const e: any = new Error('Bad credentials'); + e.status = 401; + assert.strictEqual(isAuthError(e), true); + }); + + it('returns true for an error whose message is exactly "Bad credentials"', () => { + assert.strictEqual(isAuthError(new Error('Bad credentials')), true); + }); + + it('returns true for an error whose message includes "Bad credentials"', () => { + assert.strictEqual(isAuthError(new Error('HttpError: Bad credentials - https://docs.github.com/rest')), true); + }); + + it('returns true for a GraphQL networkError with statusCode 401', () => { + const e: any = new Error('Network error'); + e.networkError = { statusCode: 401 }; + assert.strictEqual(isAuthError(e), true); + }); + + it('returns true for a GraphQL error message including "401 Unauthorized"', () => { + assert.strictEqual(isAuthError(new Error('Response not successful: Received status code 401 Unauthorized')), true); + }); + + it('returns false for unrelated errors', () => { + const e: any = new Error('Not Found'); + e.status = 404; + assert.strictEqual(isAuthError(e), false); + assert.strictEqual(isAuthError(new Error('Server Error')), false); + assert.strictEqual(isAuthError(undefined), false); + assert.strictEqual(isAuthError(null), false); + assert.strictEqual(isAuthError('Bad credentials'), false); + }); + }); + + describe('RateLogger.logApiError', () => { + it('invokes the auth error handler when the API call fails with a 401/Bad credentials', async () => { + const telemetry = new MockTelemetry(); + let handlerCalls = 0; + const handler = () => { handlerCalls++; }; + const rateLogger = new RateLogger(telemetry, false, handler); + + const e: any = new Error('Bad credentials'); + e.status = 401; + rateLogger.logApiError('/test', Promise.reject(e)); + + // allow microtasks to run + await new Promise(resolve => setImmediate(resolve)); + assert.strictEqual(handlerCalls, 1); + }); + + it('does not invoke the auth error handler for non-auth errors', async () => { + const telemetry = new MockTelemetry(); + let handlerCalls = 0; + const handler = () => { handlerCalls++; }; + const rateLogger = new RateLogger(telemetry, false, handler); + + const e: any = new Error('Not Found'); + e.status = 404; + rateLogger.logApiError('/test', Promise.reject(e)); + + await new Promise(resolve => setImmediate(resolve)); + assert.strictEqual(handlerCalls, 0); + }); + + it('swallows exceptions thrown by the auth error handler', async () => { + const telemetry = new MockTelemetry(); + const handler = () => { throw new Error('handler failure'); }; + const rateLogger = new RateLogger(telemetry, false, handler); + + const e: any = new Error('Bad credentials'); + e.status = 401; + // Should not throw. + rateLogger.logApiError('/test', Promise.reject(e)); + await new Promise(resolve => setImmediate(resolve)); + }); + + it('works without an auth error handler', async () => { + const telemetry = new MockTelemetry(); + const rateLogger = new RateLogger(telemetry, false); + + const e: any = new Error('Bad credentials'); + e.status = 401; + rateLogger.logApiError('/test', Promise.reject(e)); + await new Promise(resolve => setImmediate(resolve)); + }); + }); +}); From 83d068eff0cdd26d5b1f0b6166c38c92cea9d56a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 7 May 2026 10:01:32 +0000 Subject: [PATCH 3/3] handleAuthError: re-auth only the affected provider Agent-Logs-Url: https://github.com/microsoft/vscode-pull-request-github/sessions/56935e9d-de2d-4ee5-a5b6-44ba4fb409b4 Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> --- src/github/credentials.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/github/credentials.ts b/src/github/credentials.ts index 12efafef79..b633428403 100644 --- a/src/github/credentials.ts +++ b/src/github/credentials.ts @@ -323,7 +323,10 @@ export class CredentialStore extends Disposable { const reason = vscode.l10n.t('Your GitHub{0} authentication session is no longer valid. Please sign in again.', getGitHubSuffix(authProviderId)); const promise = (async () => { try { - return await this.recreate(reason); + // Force re-auth only for the affected provider, not both. Going through + // recreate()/doCreate() would prompt re-auth for both GitHub.com and + // GitHub Enterprise when both are configured. + return await this.initialize(authProviderId, { forceNewSession: { detail: reason } }); } finally { this._handlingAuthError.delete(authProviderId); this._lastAuthErrorHandledAt.set(authProviderId, Date.now());