fix(oauth): skip the dead flag when the chain moved, and return no token when the account is gone

Author: waleedlatif1

apps/sim/lib/oauth/credential-service.test.ts

@@ -87,6 +87,7 @@ import {
 import { isInstagramProvider, shouldProactivelyRefreshInstagramToken } from '@/lib/oauth/instagram'
 import { isMicrosoftProvider } from '@/lib/oauth/microsoft'
 import { fanOutSlackTokenChain } from '@/lib/oauth/slack'
+import { isTerminalRefreshError, markCredentialDead } from '@/lib/oauth/terminal-errors'
 import { GOOGLE_SERVICE_ACCOUNT_PROVIDER_ID } from '@/lib/oauth/types'
 
 const RAW_CREDENTIAL_ID = 'credential-raw-secret-id'
@@ -424,6 +425,7 @@ describe('OAuth access-token refresh headroom', () => {
 
   afterEach(() => {
     vi.useRealTimers()
+    vi.mocked(isTerminalRefreshError).mockReturnValue(false)
     vi.mocked(isInstagramProvider).mockReturnValue(false)
     vi.mocked(shouldProactivelyRefreshInstagramToken).mockReturnValue(false)
     vi.mocked(isMicrosoftProvider).mockReturnValue(false)
@@ -548,6 +550,47 @@ describe('OAuth access-token refresh headroom', () => {
     expect(guard).toContain('original-refresh-token')
   })
 
+  it('returns no token when the rotation write finds the account gone', async () => {
+    queueCredentialAccount(createOAuthAccount())
+    dbChainMockFns.returning.mockResolvedValueOnce([])
+    queueTableRows(account, [])
+    await expect(
+      resolveCredentialTokenBundle(RAW_CREDENTIAL_ID, RAW_USER_ID, 'test')
+    ).resolves.toBeNull()
+    expect(mocks.logger.warn).toHaveBeenCalledWith(
+      'Rotation write found no account; the credential is gone',
+      expect.anything()
+    )
+  })
+
+  it('does not flag a credential dead when a terminal failure follows a newer rotation', async () => {
+    queueCredentialAccount(createOAuthAccount())
+    vi.mocked(isTerminalRefreshError).mockReturnValue(true)
+    mocks.refreshOAuthToken.mockResolvedValue({ ok: false, errorCode: 'invalid_grant' })
+    queueTableRows(account, [
+      {
+        ...createOAuthAccount(3_600_000),
+        accessToken: 'winner-token',
+        refreshToken: 'winner-refresh-token',
+      },
+    ])
+    await expect(
+      resolveCredentialTokenBundle(RAW_CREDENTIAL_ID, RAW_USER_ID, 'test')
+    ).resolves.toEqual({ accessToken: 'winner-token' })
+    expect(markCredentialDead).not.toHaveBeenCalled()
+  })
+
+  it('flags a credential dead on a terminal failure when its chain did not move', async () => {
+    queueCredentialAccount(createOAuthAccount())
+    vi.mocked(isTerminalRefreshError).mockReturnValue(true)
+    mocks.refreshOAuthToken.mockResolvedValue({ ok: false, errorCode: 'invalid_grant' })
+    queueTableRows(account, [createOAuthAccount()])
+    await expect(
+      resolveCredentialTokenBundle(RAW_CREDENTIAL_ID, RAW_USER_ID, 'test')
+    ).resolves.toBeNull()
+    expect(markCredentialDead).toHaveBeenCalledWith(expect.any(String), 'invalid_grant')
+  })
+
   it('uses the stored chain when the rotation write loses to a newer one', async () => {
     queueCredentialAccount(createOAuthAccount())
     /** Another writer rotated first: no row still holds the token this refresh started from. */

apps/sim/lib/oauth/credential-service.ts

@@ -886,6 +886,37 @@ const REFRESH_LOCK_HEADROOM_MS = 15_000
 const REFRESH_LOCK_TTL_SEC = Math.ceil((TOKEN_REFRESH_TIMEOUT_MS + REFRESH_LOCK_HEADROOM_MS) / 1000)
 const REFRESH_FOLLOWER_MAX_WAIT_MS = REFRESH_LOCK_TTL_SEC * 1000
 
+interface StoredChain {
+  accessToken: string | null
+  accessTokenExpiresAt: Date | null
+  refreshToken: string | null
+}
+
+/** The chain an account row holds now, or nothing when the account is gone. */
+async function readStoredChain(accountId: string): Promise<StoredChain | undefined> {
+  const [stored] = await db
+    .select({
+      accessToken: account.accessToken,
+      accessTokenExpiresAt: account.accessTokenExpiresAt,
+      refreshToken: account.refreshToken,
+    })
+    .from(account)
+    .where(eq(account.id, accountId))
+    .limit(1)
+  return stored
+}
+
+/**
+ * The stored access token when it can still serve a request, as a follower would take it: a
+ * chain another writer just rotated carries one, and a token that has already expired is no
+ * answer at all.
+ */
+function usableStoredToken(stored: StoredChain, providerId: string): string | null {
+  return stored.accessToken && !isOAuthAccessTokenExpiring(stored.accessTokenExpiresAt, providerId)
+    ? stored.accessToken
+    : null
+}
+
 async function performCoalescedRefresh({
   accountId,
   providerId,
@@ -979,18 +1010,25 @@ async function performCoalescedRefresh({
               message: result.message,
             })
             if (result.errorCode && isTerminalRefreshError(result.errorCode)) {
-              // A refresh that lost a race with a concurrent connect fails with
-              // a revoked/rotated-out token even though the installation just
-              // got a live chain — dead-flagging then would take down a healthy
-              // credential for an hour.
+              // A refresh that lost a race with a concurrent connect or a newer
+              // rotation fails with a revoked/rotated-out token even though the
+              // account just got a live chain — dead-flagging then would take
+              // down a healthy credential for an hour.
               if (
                 slackChainVersion &&
                 (await hasSlackChainMoved(slackTeamId!, slackChainVersion))
               ) {
                 logger.info('Skipping dead flag: Slack chain moved during refresh', logContext)
-              } else {
-                await markCredentialDead(scopeKey, result.errorCode)
+                return null
               }
+              if (!slackTeamId) {
+                const stored = await readStoredChain(accountId)
+                if (stored && stored.refreshToken !== refreshToken) {
+                  logger.info('Skipping dead flag: chain moved during refresh', logContext)
+                  return usableStoredToken(stored, providerId)
+                }
+              }
+              await markCredentialDead(scopeKey, result.errorCode)
             }
             return null
           }
@@ -1041,16 +1079,16 @@ async function performCoalescedRefresh({
               .where(and(eq(account.id, accountId), eq(account.refreshToken, refreshToken)))
               .returning({ id: account.id })
             if (rotated.length === 0) {
+              const stored = await readStoredChain(accountId)
+              if (!stored) {
+                logger.warn('Rotation write found no account; the credential is gone', logContext)
+                return null
+              }
               logger.warn(
                 'Rotation write lost to a newer chain; using the stored token',
                 logContext
               )
-              const [stored] = await db
-                .select({ accessToken: account.accessToken })
-                .from(account)
-                .where(eq(account.id, accountId))
-                .limit(1)
-              return stored?.accessToken ?? result.accessToken
+              return usableStoredToken(stored, providerId)
             }
           }