fix: prevent quota refresh from falsely resetting to 5000

src/context/AppContext.jsx

@@ -1,10 +1,15 @@
 import { createContext, useContext, useState, useCallback, useEffect, useMemo, useRef } from 'react'
-import { fetchOrg, fetchRepos, fetchContributors, fetchIssues, fetchRateLimit, fetchPulls } from '../services/github'
+import { fetchOrg, fetchRepos, fetchContributors, fetchIssues, fetchRateLimit, fetchPulls, bumpPatGeneration, asValidRateLimit } from '../services/github'
 import { buildAnalyticalModel, getTopRepositories } from '../services/analytics'
 import { saveAnalysis, loadAnalysis } from '../services/cache'
 
 const Ctx = createContext(null)
 
+function isStaleRateLimitIncrease(current, next) {
+  if (!current || next.limit !== current.limit || next.remaining <= current.remaining) return false
+  return current.reset > 0 && Date.now() < current.reset * 1000
+}
+
 function getStoredRateLimit() {
   const stored = localStorage.getItem('oe_rate_limit')
 
@@ -32,6 +37,8 @@ export function AppProvider({ children }) {
   const [issuesData, setIssuesData] = useState({})
   const [pullsData, setPullsData] = useState({})
   const [rateLimit, setRateLimit] = useState(getStoredRateLimit)
+  const rateLimitRef = useRef(rateLimit)
+  const patGenerationRef = useRef(0)
   const [loading, setLoading] = useState(false)
   const [loadMsg, setLoadMsg] = useState('')
   const [govLoading, setGovLoading] = useState(false)
@@ -100,8 +107,12 @@ export function AppProvider({ children }) {
 
   useEffect(() => {
     const handler = e => {
-      setRateLimit(e.detail)
-      localStorage.setItem('oe_rate_limit', JSON.stringify(e.detail))
+      const normalized = asValidRateLimit(e.detail)
+      if (!normalized) return
+      if (isStaleRateLimitIncrease(rateLimitRef.current, normalized)) return
+      rateLimitRef.current = normalized
+      setRateLimit(normalized)
+      localStorage.setItem('oe_rate_limit', JSON.stringify(normalized))
     }
 
     window.addEventListener('rate-limit-update', handler)
@@ -115,6 +126,7 @@ export function AppProvider({ children }) {
     if (!rateLimit?.reset) return
 
     const timeout = setTimeout(() => {
+      rateLimitRef.current = null
       localStorage.removeItem('oe_rate_limit')
       setRateLimit(null)
     }, Math.max(0, rateLimit.reset * 1000 - Date.now()))
@@ -123,16 +135,29 @@ export function AppProvider({ children }) {
   }, [rateLimit])
 
   const refreshRateLimit = useCallback(async () => {
+    const generation = patGenerationRef.current
     const rl = await fetchRateLimit(pat)
-    if (rl) {
-      setRateLimit(rl)
-      return true
-    }
-    return false
+    if (!rl) return false
+    if (generation !== patGenerationRef.current) return 'superseded'
+    if ((pat || '') !== (localStorage.getItem('oe_pat') || '')) return 'superseded'
+    // `/rate_limit` can lag behind live response headers. Ignore stale
+    // increases while the current window is still active.
+    if (isStaleRateLimitIncrease(rateLimitRef.current, rl)) return true
+    rateLimitRef.current = rl
+    setRateLimit(rl)
+    localStorage.setItem('oe_rate_limit', JSON.stringify(rl))
+    return true
   }, [pat])
   const savePat = useCallback(token => {
+    patGenerationRef.current += 1
+    bumpPatGeneration()
     setPat(token)
     token ? localStorage.setItem('oe_pat', token) : localStorage.removeItem('oe_pat')
+    // Old quota belongs to the old PAT. Clear it so the new PAT's higher
+    // remaining count is accepted instead of being blocked as stale.
+    rateLimitRef.current = null
+    localStorage.removeItem('oe_rate_limit')
+    setRateLimit(null)
   }, [])
 
   // Multi-org explore

src/context/AppContext.test.jsx

@@ -0,0 +1,148 @@
+import { act, render, screen } from '@testing-library/react'
+import { useState } from 'react'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import { AppProvider, useApp } from './AppContext'
+
+const { loadAnalysis, saveAnalysis, fetchRateLimit, bumpPatGeneration } = vi.hoisted(() => ({
+  loadAnalysis: vi.fn(),
+  saveAnalysis: vi.fn(),
+  fetchRateLimit: vi.fn(),
+  bumpPatGeneration: vi.fn(),
+}))
+
+vi.mock('../services/cache', () => ({ loadAnalysis, saveAnalysis }))
+vi.mock('../services/github', async importOriginal => {
+  const actual = await importOriginal()
+  return {
+    ...actual,
+    fetchOrg: vi.fn(),
+    fetchRepos: vi.fn(),
+    fetchContributors: vi.fn(),
+    fetchIssues: vi.fn(),
+    fetchPulls: vi.fn(),
+    fetchRateLimit,
+    bumpPatGeneration,
+  }
+})
+
+function RateLimitProbe() {
+  const { rateLimit, savePat, refreshRateLimit } = useApp()
+  const [refreshResult, setRefreshResult] = useState('')
+  return (
+    <>
+      <output data-testid="rate-limit">{JSON.stringify(rateLimit)}</output>
+      <output data-testid="refresh-result">{refreshResult}</output>
+      <button onClick={() => savePat('new-token')}>Save PAT</button>
+      <button onClick={() => refreshRateLimit().then(result => setRefreshResult(String(result)))}>
+        Refresh quota
+      </button>
+    </>
+  )
+}
+
+function dispatchRateLimit(detail) {
+  act(() => {
+    window.dispatchEvent(new CustomEvent('rate-limit-update', { detail }))
+  })
+}
+
+describe('AppContext rate-limit events', () => {
+  beforeEach(() => {
+    vi.useFakeTimers()
+    vi.setSystemTime(new Date('2026-10-05T12:00:00Z'))
+    localStorage.clear()
+    loadAnalysis.mockResolvedValue(null)
+    saveAnalysis.mockReset()
+    fetchRateLimit.mockReset()
+    bumpPatGeneration.mockReset()
+  })
+
+  afterEach(() => {
+    vi.useRealTimers()
+    localStorage.clear()
+  })
+
+  it('rejects same-window increases but accepts decreases and next-window updates', () => {
+    const reset = Math.floor(Date.now() / 1000) + 60
+    const current = { limit: 5000, remaining: 4200, used: 800, reset }
+    localStorage.setItem('oe_rate_limit', JSON.stringify(current))
+
+    render(
+      <AppProvider>
+        <RateLimitProbe />
+      </AppProvider>
+    )
+
+    dispatchRateLimit({ ...current, remaining: 5000, used: 0 })
+    expect(screen.getByTestId('rate-limit').textContent).toBe(JSON.stringify(current))
+    expect(JSON.parse(localStorage.getItem('oe_rate_limit'))).toEqual(current)
+
+    const decreased = { ...current, remaining: 4199, used: 801 }
+    dispatchRateLimit(decreased)
+    expect(screen.getByTestId('rate-limit').textContent).toBe(JSON.stringify(decreased))
+
+    vi.setSystemTime(new Date((reset + 1) * 1000))
+    const nextWindow = { ...current, remaining: 5000, used: 0, reset: reset + 3600 }
+    dispatchRateLimit(nextWindow)
+    expect(screen.getByTestId('rate-limit').textContent).toBe(JSON.stringify(nextWindow))
+  })
+
+  it('ignores invalid rate-limit events', () => {
+    const current = { limit: 5000, remaining: 4200, used: 800, reset: 1_800_000_000 }
+    localStorage.setItem('oe_rate_limit', JSON.stringify(current))
+
+    render(
+      <AppProvider>
+        <RateLimitProbe />
+      </AppProvider>
+    )
+
+    dispatchRateLimit({ ...current, limit: 'invalid' })
+
+    expect(screen.getByTestId('rate-limit').textContent).toBe(JSON.stringify(current))
+    expect(JSON.parse(localStorage.getItem('oe_rate_limit'))).toEqual(current)
+  })
+
+  it('clears the displayed and stored quota when a PAT is saved', () => {
+    const current = { limit: 5000, remaining: 4200, used: 800, reset: 1_800_000_000 }
+    localStorage.setItem('oe_pat', 'old-token')
+    localStorage.setItem('oe_rate_limit', JSON.stringify(current))
+
+    render(
+      <AppProvider>
+        <RateLimitProbe />
+      </AppProvider>
+    )
+
+    act(() => screen.getByRole('button', { name: 'Save PAT' }).click())
+
+    expect(screen.getByTestId('rate-limit').textContent).toBe('null')
+    expect(localStorage.getItem('oe_pat')).toBe('new-token')
+    expect(localStorage.getItem('oe_rate_limit')).toBeNull()
+    expect(bumpPatGeneration).toHaveBeenCalledOnce()
+  })
+
+  it('marks a refresh as superseded when the stored PAT changes mid-request', async () => {
+    let resolveRefresh
+    localStorage.setItem('oe_pat', 'old-token')
+    fetchRateLimit.mockImplementation(
+      () => new Promise(resolve => { resolveRefresh = resolve })
+    )
+
+    render(
+      <AppProvider>
+        <RateLimitProbe />
+      </AppProvider>
+    )
+
+    act(() => screen.getByRole('button', { name: 'Refresh quota' }).click())
+    localStorage.setItem('oe_pat', 'new-token')
+    await act(async () => {
+      resolveRefresh({ limit: 5000, remaining: 4000, used: 1000, reset: 1_800_000_000 })
+      await Promise.resolve()
+    })
+
+    expect(screen.getByTestId('refresh-result').textContent).toBe('superseded')
+    expect(localStorage.getItem('oe_rate_limit')).toBeNull()
+  })
+})

src/pages/SettingsPage.jsx

@@ -268,8 +268,9 @@ export default function SettingsPage() {
                   setIsRefreshing(true);
                   setRefreshError(false);
                   try {
-                    const success = await refreshRateLimit();
-                    if (!success) {
+                    const result = await refreshRateLimit();
+                    // 'superseded' means a PAT change discarded this result — not a failure.
+                    if (result === false) {
                       setRefreshError(true);
                       setTimeout(() => setRefreshError(false), 2000);
                     }

src/services/github.cacheKey.test.js

@@ -1,5 +1,5 @@
 import { describe, it, expect, vi, afterEach } from 'vitest'
-import { normalizeCacheKey, fetchOrg, fetchRepos, fetchContributors } from './github'
+import { cacheGet, normalizeCacheKey, fetchOrg, fetchRepos, fetchContributors } from './github'
 
 /** Minimal in-memory stand-in for the IndexedDB API surface used by github.js. */
 function installFakeIndexedDB() {
@@ -30,6 +30,12 @@ function installFakeIndexedDB() {
               queueMicrotask(() => req.onsuccess && req.onsuccess())
               return req
             },
+            delete(k) {
+              table.delete(k)
+              const req = {}
+              queueMicrotask(() => req.onsuccess && req.onsuccess())
+              return req
+            },
             clear() {
               table.clear()
               const req = {}
@@ -53,6 +59,7 @@ function installFakeIndexedDB() {
       return req
     },
   })
+  return tables
 }
 
 /** Counting fetch stub. Handler maps a URL to { status, body }. */
@@ -111,6 +118,14 @@ describe('normalizeCacheKey', () => {
     )
   })
 
+  it('preserves case in path segments beyond owner and repository names', () => {
+    expect(
+      normalizeCacheKey('https://api.github.com/repos/AOSSIE-Org/Repo-A/contents/ReadMe.md')
+    ).toBe(
+      'https://api.github.com/repos/aossie-org/repo-a/contents/ReadMe.md'
+    )
+  })
+
   it('sorts query parameters deterministically', () => {
     expect(
       normalizeCacheKey('https://api.github.com/orgs/aossie-org/repos?page=1&per_page=100')
@@ -133,6 +148,21 @@ describe('normalizeCacheKey', () => {
 })
 
 describe('case-insensitive caching', () => {
+  it('migrates an existing raw URL cache entry to its normalized key', async () => {
+    const tables = installFakeIndexedDB()
+    const oldKey = 'https://api.github.com/orgs/AOSSIE-Org'
+    const normalizedKey = normalizeCacheKey(oldKey)
+    const record = { k: oldKey, v: { login: 'AOSSIE-Org' }, ts: Date.now() }
+    tables.set('cache', new Map([[oldKey, record]]))
+
+    await expect(cacheGet(oldKey)).resolves.toEqual(record.v)
+    expect(tables.get('cache').get(normalizedKey)).toEqual({
+      ...record,
+      k: normalizedKey,
+    })
+    expect(tables.get('cache').has(oldKey)).toBe(false)
+  })
+
   it('re-searching an org with different case costs zero extra fetches', async () => {
     installFakeIndexedDB()
     const calls = installCountingFetch({ login: 'AOSSIE-Org', public_repos: 1 })
@@ -167,22 +197,38 @@ describe('case-insensitive caching', () => {
 })
 
 describe('empty-repository responses', () => {
-  for (const status of [204, 409]) {
-    it(`caches contributors ${status} (empty repo) so repeats cost nothing`, async () => {
-      installFakeIndexedDB()
-      const calls = installFetchStub(() => ({ status, body: null }))
-
-      const first = await fetchContributors('AOSSIE-Org', 'empty-repo', 'pat')
-      expect(first).toEqual([])
-      expect(calls).toHaveLength(1)
-
-      // Previously this threw inside res.json() on every call, was never
-      // cached, and burned one token per re-search.
-      const second = await fetchContributors('AOSSIE-Org', 'empty-repo', 'pat')
-      expect(second).toEqual([])
-      expect(calls).toHaveLength(1)
-    })
-  }
+  it('does not treat an empty object response as an empty list', async () => {
+    installFakeIndexedDB()
+    const calls = installFetchStub(() => ({ status: 204, body: null }))
+
+    await expect(fetchOrg('AOSSIE-Org', 'pat')).rejects.toThrow('HTTP_204')
+    await expect(fetchOrg('AOSSIE-Org', 'pat')).rejects.toThrow('HTTP_204')
+    expect(calls).toHaveLength(2)
+  })
+
+  it('caches contributors 204 (empty repo) so repeats cost nothing', async () => {
+    installFakeIndexedDB()
+    const calls = installFetchStub(() => ({ status: 204, body: null }))
+
+    const first = await fetchContributors('AOSSIE-Org', 'empty-repo', 'pat')
+    expect(first).toEqual([])
+    expect(calls).toHaveLength(1)
+
+    // Previously this threw inside res.json() on every call, was never
+    // cached, and burned one token per re-search.
+    const second = await fetchContributors('AOSSIE-Org', 'empty-repo', 'pat')
+    expect(second).toEqual([])
+    expect(calls).toHaveLength(1)
+  })
+
+  it('rejects 409 responses instead of caching them as empty results', async () => {
+    installFakeIndexedDB()
+    const calls = installFetchStub(() => ({ status: 409, body: null }))
+
+    await expect(fetchContributors('AOSSIE-Org', 'repo-a', 'pat')).rejects.toThrow('HTTP_409')
+    await expect(fetchContributors('AOSSIE-Org', 'repo-a', 'pat')).rejects.toThrow('HTTP_409')
+    expect(calls).toHaveLength(2)
+  })
 
   it('still throws RATE_LIMIT (403) and NOT_FOUND (404)', async () => {
     installFakeIndexedDB()