From f0096f022423e92803543e0d1eee043a40dd2a64 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Fri, 24 Jul 2026 09:56:58 -0300 Subject: [PATCH] fix(resilience): terminal-skip spares the recoverable GitHub Copilot no_refresh_token state (#8389) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cause: #8182's terminal-connection guard in checkConnection() returns early for any testStatus in {credits_exhausted, banned, expired} to stop the sweep from wasting CPU/network probing connections that can never self-heal. But testStatus="expired" + errorCode="no_refresh_token" is exactly the state the pre-existing GitHub Copilot self-heal targets (isGitHubAccessTokenOnlyConnection + canClearGitHubNoRefreshTokenState, ~line 83-97 / 413-492): a Copilot connection with no OAuth refresh token but a still-valid copilotToken, which the sweep is supposed to flip back to "active". With the new guard placed ahead of that block unconditionally, the self-heal became unreachable for exactly the state it exists to clear. Impact: healthy GitHub Copilot connections that once lost their OAuth refresh token got stuck at testStatus="expired" in the dashboard forever, even though their Copilot sub-token kept working and the sweep would have cleared the stale status back to "active" every cycle before #8182. Fix: carve out the exact recoverable shape from the terminal-skip guard — testStatus==="expired" && errorCode==="no_refresh_token" && isGitHubAccessTokenOnlyConnection(conn) — so the guard still skips every other terminal case (credits_exhausted, banned, and "expired" for any other reason) untouched, matching #8182's original intent. Validation: tests/unit/token-health-no-refresh-token-expired-5326.test.ts was red (1 fail / 4 pass) before the fix — "checkConnection clears stale no_refresh_token state for usable GitHub Copilot connections" asserted testStatus flips back to "active" but got "expired". Green after the fix (6/6, including a new boundary test proving a GitHub Copilot connection expired for any OTHER reason, e.g. errorCode "invalid_grant", is still skipped untouched). Also reran the adjacent checkConnection/tokenHealthCheck suites (token-health-check.test.ts, token-health-check-circuit-breaker.test.ts, apikey-connection-health-check.test.ts, token-health-check-sweep.test.ts, token-health-check-tickms-defined.test.ts, tokenHealthCheck-batchSize.test.ts, codex-oauth-refresh-persist-6352.test.ts, oauth-providers-error-handling.test.ts) — all green, confirming #8182's terminal-skip behavior is otherwise unchanged. npm run typecheck:core clean. Refs #8182 Refs #8286 Refs #5326 --- src/lib/tokenHealthCheck.ts | 17 +++++++- ...alth-no-refresh-token-expired-5326.test.ts | 41 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/src/lib/tokenHealthCheck.ts b/src/lib/tokenHealthCheck.ts index e7ad393a58..6a0091ca7b 100644 --- a/src/lib/tokenHealthCheck.ts +++ b/src/lib/tokenHealthCheck.ts @@ -405,8 +405,23 @@ export async function checkConnection(conn) { // CPU and network on every sweep cycle. Mirrors isTerminalConnectionStatus // in src/sse/services/auth.ts and TERMINAL_CONNECTION_STATUSES in // src/lib/quota/connectionRecovery.ts. + // + // #5326 exception: a GitHub Copilot access-token-only connection parked in + // "expired" with errorCode "no_refresh_token" is NOT actually terminal — it's + // the exact target of the self-heal below (canClearGitHubNoRefreshTokenState), + // which clears that stale status back to "active" once the Copilot sub-token + // proves usable. Treating it as terminal here made that self-heal unreachable, + // leaving healthy Copilot connections stuck at "expired" forever. + const isRecoverableGithubCopilotNoRefresh = + conn.testStatus === "expired" && + conn.errorCode === "no_refresh_token" && + isGitHubAccessTokenOnlyConnection(conn); const terminalStatuses = new Set(["credits_exhausted", "banned", "expired"]); - if (typeof conn.testStatus === "string" && terminalStatuses.has(conn.testStatus.toLowerCase())) { + if ( + typeof conn.testStatus === "string" && + terminalStatuses.has(conn.testStatus.toLowerCase()) && + !isRecoverableGithubCopilotNoRefresh + ) { return; } diff --git a/tests/unit/token-health-no-refresh-token-expired-5326.test.ts b/tests/unit/token-health-no-refresh-token-expired-5326.test.ts index 33a9dd8d61..a79934bc05 100644 --- a/tests/unit/token-health-no-refresh-token-expired-5326.test.ts +++ b/tests/unit/token-health-no-refresh-token-expired-5326.test.ts @@ -174,3 +174,44 @@ test("checkConnection clears stale no_refresh_token state for usable GitHub Copi assert.equal(updated?.lastError ?? null, null); assert.ok(updated?.lastHealthCheckAt); }); + +// Boundary regression for #8182 vs #5326: the terminal-skip guard added by #8182 +// must keep skipping a GitHub Copilot connection that is "expired" for a DIFFERENT +// reason than the recoverable no_refresh_token self-heal above (e.g. a manually +// banned/invalidated account). Only the EXACT no_refresh_token shape is exempted +// from the terminal skip — this proves the #5326 fix did not reopen #8182's +// wasted-probe fix for every "expired" GitHub connection. +test("checkConnection still skips a GitHub Copilot connection expired for a non-no_refresh_token reason (#8182 boundary)", async () => { + await resetStorage(); + + const connection = await providersDb.createProviderConnection({ + provider: "github", + authType: "oauth", + name: "GitHub Genuinely Expired Account", + accessToken: "github-access-token", + refreshToken: null, + providerSpecificData: { + copilotToken: "copilot-token", + copilotTokenExpiresAt: Math.floor((Date.now() + 60 * 60 * 1000) / 1000), + }, + testStatus: "expired", + errorCode: "invalid_grant", + lastError: "Manually invalidated by operator.", + isActive: true, + }); + + await tokenHealthCheck.checkConnection(connection); + + const updated = await providersDb.getProviderConnectionById(getCreatedConnectionId(connection)); + assert.equal( + updated?.testStatus, + "expired", + "terminal-skip must still apply outside the exact no_refresh_token shape" + ); + assert.equal(updated?.errorCode, "invalid_grant"); + assert.equal( + updated?.lastHealthCheckAt ?? null, + null, + "checkConnection must return early without touching the row at all" + ); +});