fix(auth): prevent OAuth refresh lock races

src/utils/auth.refresh.test.ts

@@ -8,6 +8,9 @@ let stored: SecureStorageData = {}
 let refreshMode: 'success' | 'invalid_grant' | 'transient_error' = 'success'
 let refreshCalls = 0
 let storageWritesFail = false
+let lockReleaseErrorCode: string | null = null
+let refreshGate: Promise<void> | null = null
+let markRefreshStarted: (() => void) | null = null
 const originalAxiosPost = axios.post
 
 beforeAll(() => {
@@ -27,14 +30,23 @@ beforeAll(() => {
     }),
   }))
   mock.module('./lockfile.js', () => ({
-    lock: async () => async () => {},
+    lock: async () => async () => {
+      if (lockReleaseErrorCode) {
+        throw Object.assign(new Error('Lock is not acquired/owned by you'), {
+          code: lockReleaseErrorCode,
+        })
+      }
+    },
     lockSync: () => () => {},
   }))
   mock.module('../services/oauth/getOauthProfile.js', () => ({
     getOauthProfileFromOauthToken: async () => null,
   }))
   axios.post = mock(async () => {
     refreshCalls++
+    markRefreshStarted?.()
+    markRefreshStarted = null
+    if (refreshGate) await refreshGate
     if (refreshMode === 'invalid_grant') {
       throw Object.assign(new Error('invalid_grant'), {
         response: { data: { error: 'invalid_grant' } },
@@ -151,3 +163,110 @@ test('preserves the stored session after a transient refresh failure', async ()
   expect(stored.verbooOauth?.accessToken).toBe('rejected-access-4')
   expect(stored.verbooOauth?.refreshToken).toBe('old-refresh-4')
 })
+
+test('does not abort a successful refresh when the lock was already released', async () => {
+  stored = {
+    verbooOauth: {
+      accessToken: 'rejected-access-5',
+      refreshToken: 'old-refresh-5',
+      expiresAt: Date.now() + 600_000,
+      scopes: ['user:profile', 'user:inference'],
+    },
+  }
+  refreshMode = 'success'
+  refreshCalls = 0
+  storageWritesFail = false
+  lockReleaseErrorCode = 'ENOTACQUIRED'
+
+  // @ts-expect-error cache-busting query keeps module state isolated.
+  const releaseRaceModule = await import('./auth.js?refresh-release-race')
+  const { handleOAuth401ErrorWithOutcome } = releaseRaceModule
+
+  try {
+    const outcome = await handleOAuth401ErrorWithOutcome('rejected-access-5')
+
+    expect(outcome).toBe('refreshed')
+    expect(refreshCalls).toBe(1)
+    expect(stored.verbooOauth?.accessToken).toBe('fresh-access')
+  } finally {
+    lockReleaseErrorCode = null
+  }
+})
+
+test('does not hide an unexpected lock release failure', async () => {
+  stored = {
+    verbooOauth: {
+      accessToken: 'rejected-access-unexpected-release',
+      refreshToken: 'old-refresh-unexpected-release',
+      expiresAt: Date.now() + 600_000,
+      scopes: ['user:profile', 'user:inference'],
+    },
+  }
+  refreshMode = 'success'
+  refreshCalls = 0
+  storageWritesFail = false
+  lockReleaseErrorCode = 'EIO'
+
+  // @ts-expect-error cache-busting query keeps module state isolated.
+  const unexpectedReleaseModule = await import('./auth.js?unexpected-release')
+
+  try {
+    await expect(
+      unexpectedReleaseModule.handleOAuth401ErrorWithOutcome(
+        'rejected-access-unexpected-release',
+      ),
+    ).rejects.toMatchObject({ code: 'EIO' })
+  } finally {
+    lockReleaseErrorCode = null
+  }
+})
+
+test('shares one refresh between an expiry check and a simultaneous 401', async () => {
+  stored = {
+    verbooOauth: {
+      accessToken: 'rejected-access-6',
+      refreshToken: 'old-refresh-6',
+      expiresAt: Date.now() - 1,
+      scopes: ['user:profile', 'user:inference'],
+    },
+  }
+  refreshMode = 'success'
+  refreshCalls = 0
+  storageWritesFail = false
+  lockReleaseErrorCode = null
+
+  let releaseRefresh = () => {}
+  refreshGate = new Promise<void>((resolve) => {
+    releaseRefresh = resolve
+  })
+  const refreshStarted = new Promise<void>((resolve) => {
+    markRefreshStarted = resolve
+  })
+
+  // @ts-expect-error cache-busting query keeps module state isolated.
+  const singleFlightModule = await import('./auth.js?refresh-single-flight')
+  const {
+    checkAndRefreshOAuthTokenIfNeeded,
+    handleOAuth401ErrorWithOutcome,
+  } = singleFlightModule
+
+  const expiryCheck = checkAndRefreshOAuthTokenIfNeeded()
+  await refreshStarted
+  const rejectedRequest = handleOAuth401ErrorWithOutcome('rejected-access-6')
+
+  try {
+    await new Promise((resolve) => setTimeout(resolve, 20))
+    expect(refreshCalls).toBe(1)
+  } finally {
+    releaseRefresh()
+    refreshGate = null
+  }
+
+  const [expiryRecovered, rejectionOutcome] = await Promise.all([
+    expiryCheck,
+    rejectedRequest,
+  ])
+  expect(expiryRecovered).toBe(true)
+  expect(['refreshed', 'token_changed']).toContain(rejectionOutcome)
+  expect(stored.verbooOauth?.accessToken).toBe('fresh-access')
+})

src/utils/auth.ts

@@ -1465,7 +1465,7 @@ async function handleOAuth401ErrorImpl(
   }
 
   // Same token that failed - force refresh, bypassing local expiration check
-  return checkAndRefreshOAuthTokenIfNeededImpl(0, true, failedAccessToken)
+  return runOAuthRefreshCheck(0, true, failedAccessToken)
 }
 
 /**
@@ -1543,29 +1543,63 @@ function clearStoredVerbooOAuthIfRefreshTokenMatches(
   }
 }
 
-// In-flight promise for deduplicating concurrent calls
+// One in-flight refresh across proactive expiry checks and reactive 401s.
 let pendingRefreshCheck: Promise<OAuthRefreshOutcome> | null = null
 
 export function checkAndRefreshOAuthTokenIfNeeded(
   retryCount = 0,
   force = false,
 ): Promise<boolean> {
-  // Deduplicate concurrent non-retry, non-force calls
-  if (retryCount === 0 && !force) {
-    if (pendingRefreshCheck) {
-      return pendingRefreshCheck.then(didOAuthRefreshRecover)
+  return runOAuthRefreshCheck(retryCount, force).then(didOAuthRefreshRecover)
+}
+
+async function runOAuthRefreshCheck(
+  retryCount: number,
+  force: boolean,
+  failedAccessToken?: string,
+): Promise<OAuthRefreshOutcome> {
+  if (retryCount !== 0) {
+    return checkAndRefreshOAuthTokenIfNeededImpl(
+      retryCount,
+      force,
+      failedAccessToken,
+    )
+  }
+
+  const pending = pendingRefreshCheck
+  if (pending) {
+    const outcome = await pending
+    if (!force) return outcome
+
+    clearOAuthTokenCache()
+    const currentTokens = await getClaudeAIOAuthTokensAsync()
+    if (
+      failedAccessToken &&
+      currentTokens?.accessToken &&
+      currentTokens.accessToken !== failedAccessToken
+    ) {
+      return 'token_changed'
     }
 
-    const promise = checkAndRefreshOAuthTokenIfNeededImpl(retryCount, force)
-    pendingRefreshCheck = promise.finally(() => {
-      pendingRefreshCheck = null
-    })
-    return pendingRefreshCheck.then(didOAuthRefreshRecover)
+    if (outcome !== 'unchanged') return outcome
+
+    // A proactive check can legitimately decide that the token is still
+    // valid. A simultaneous server 401 must still force one refresh after it.
+    return runOAuthRefreshCheck(retryCount, force, failedAccessToken)
   }
 
-  return checkAndRefreshOAuthTokenIfNeededImpl(retryCount, force).then(
-    didOAuthRefreshRecover,
+  const refresh = checkAndRefreshOAuthTokenIfNeededImpl(
+    retryCount,
+    force,
+    failedAccessToken,
   )
+  const trackedRefresh = refresh.finally(() => {
+    if (pendingRefreshCheck === trackedRefresh) {
+      pendingRefreshCheck = null
+    }
+  })
+  pendingRefreshCheck = trackedRefresh
+  return trackedRefresh
 }
 
 async function checkAndRefreshOAuthTokenIfNeededImpl(
@@ -1731,8 +1765,24 @@ async function checkAndRefreshOAuthTokenIfNeededImpl(
     return 'transient_error'
   } finally {
     logEvent('tengu_oauth_token_refresh_lock_releasing', {})
-    await release()
-    logEvent('tengu_oauth_token_refresh_lock_released', {})
+    try {
+      await release()
+      logEvent('tengu_oauth_token_refresh_lock_released', {})
+    } catch (error) {
+      const code = (error as { code?: string }).code
+      if (code !== 'ENOTACQUIRED' && code !== 'ERELEASED') {
+        throw error
+      }
+
+      // proper-lockfile can report ownership loss when a stale lock was
+      // replaced and release callbacks complete out of order. The refresh
+      // result above is still authoritative; do not replace it with a raw
+      // release error.
+      logError(error)
+      logEvent('tengu_oauth_token_refresh_lock_release_ownership_lost', {
+        code: code as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS,
+      })
+    }
   }
 }