From a7f0dac9c7e0ecb4d2903d689e181ed44ef59221 Mon Sep 17 00:00:00 2001 From: Nasir Yousuf Date: Fri, 2 Oct 2026 23:18:38 +0530 Subject: [PATCH 1/3] fix: prevent quota refresh from falsely resetting to 5000 --- src/context/AppContext.jsx | 29 +++++++++++++----- src/services/github.js | 63 ++++++++++++++++++++++++++++++-------- 2 files changed, 73 insertions(+), 19 deletions(-) diff --git a/src/context/AppContext.jsx b/src/context/AppContext.jsx index 66f7de0d..24db44db 100644 --- a/src/context/AppContext.jsx +++ b/src/context/AppContext.jsx @@ -100,8 +100,10 @@ export function AppProvider({ children }) { useEffect(() => { const handler = e => { - setRateLimit(e.detail) - localStorage.setItem('oe_rate_limit', JSON.stringify(e.detail)) + const next = e.detail + if (!Number.isFinite(Number(next?.limit)) || !Number.isFinite(Number(next?.remaining))) return + setRateLimit(next) + localStorage.setItem('oe_rate_limit', JSON.stringify(next)) } window.addEventListener('rate-limit-update', handler) @@ -124,12 +126,25 @@ export function AppProvider({ children }) { const refreshRateLimit = useCallback(async () => { const rl = await fetchRateLimit(pat) - if (rl) { - setRateLimit(rl) - return true + if (!rl) return false + // `GET /rate_limit` does not consume quota and its `resources.core` + // body can lag behind the live `x-ratelimit-*` counters (reads full + // while search headers show consumed quota, with a different `reset` + // epoch). Never let a stale read inflate the remaining count while + // the current window is still active — only a new window expiry or a + // new limit (e.g. PAT added/removed) may legitimately raise it. + if ( + rateLimit && + rl.limit === rateLimit.limit && + rl.remaining > rateLimit.remaining + ) { + const windowActive = !rateLimit.reset || Date.now() < rateLimit.reset * 1000 + if (windowActive) return true } - return false - }, [pat]) + setRateLimit(rl) + localStorage.setItem('oe_rate_limit', JSON.stringify(rl)) + return true + }, [pat, rateLimit]) const savePat = useCallback(token => { setPat(token) token ? localStorage.setItem('oe_pat', token) : localStorage.removeItem('oe_pat') diff --git a/src/services/github.js b/src/services/github.js index a4180fa8..0e663c15 100644 --- a/src/services/github.js +++ b/src/services/github.js @@ -62,16 +62,12 @@ async function fetchWithCache(url, pat) { const res = await fetch(url, { headers }) - window.dispatchEvent( - new CustomEvent('rate-limit-update', { - detail: { - limit: Number(res.headers.get('x-ratelimit-limit')), - remaining: Number(res.headers.get('x-ratelimit-remaining')), - used: Number(res.headers.get('x-ratelimit-used')), - reset: Number(res.headers.get('x-ratelimit-reset')) - } - }) - ) + const live = readRateLimitHeaders(res.headers) + if (live) { + window.dispatchEvent( + new CustomEvent('rate-limit-update', { detail: live }) + ) + } if (res.status === 403) throw new Error('RATE_LIMIT') if (res.status === 404) throw new Error('NOT_FOUND') @@ -134,12 +130,55 @@ export async function fetchPulls(org, repo, pat) { return all } +/** Read live counters from response headers (authoritative per GitHub docs). + * Returns null when the headers are absent so callers can fall back. */ +function readRateLimitHeaders(h) { + if (!h || typeof h.get !== 'function') return null + const limit = Number(h.get('x-ratelimit-limit')) + const remaining = Number(h.get('x-ratelimit-remaining')) + if (!Number.isFinite(limit) || !Number.isFinite(remaining)) return null + if (limit < 0 || remaining < 0) return null + const rawUsed = Number(h.get('x-ratelimit-used')) + const rawReset = Number(h.get('x-ratelimit-reset')) + // `x-ratelimit-used` is not in Access-Control-Expose-Headers, so browsers + // always read it as null -> 0. Derive it instead of showing a false 0. + const used = Number.isFinite(rawUsed) && rawUsed > 0 ? rawUsed : limit - remaining + return { + limit, + remaining, + used, + reset: Number.isFinite(rawReset) ? rawReset : 0, + } +} + +/** Validate a rate-limit object from the `/rate_limit` body. */ +function asValidRateLimit(obj) { + if (!obj) return null + const limit = Number(obj.limit) + const remaining = Number(obj.remaining) + if (!Number.isFinite(limit) || !Number.isFinite(remaining)) return null + if (limit < 0 || remaining < 0) return null + const used = Number(obj.used) + const reset = Number(obj.reset) + return { + limit, + remaining, + used: Number.isFinite(used) ? used : limit - remaining, + reset: Number.isFinite(reset) ? reset : 0, + } +} + export async function fetchRateLimit(pat) { try { const headers = { Accept: 'application/vnd.github.v3+json' } if (pat) headers.Authorization = `token ${pat}` const res = await fetch('https://api.github.com/rate_limit', { headers }) - const data = await res.json() - return data.rate + const data = await res.json().catch(() => null) + // Headers are the authoritative source; the body (`rate` is closing + // down, and the body can disagree with the live counters) is fallback. + return readRateLimitHeaders(res.headers) + ?? asValidRateLimit(data?.resources?.core) + ?? asValidRateLimit(data?.rate) + ?? null } catch { return null } } From 7fc829f874da5faa6809e1205fe9114b158de0ca Mon Sep 17 00:00:00 2001 From: Nasir Yousuf Date: Sat, 3 Oct 2026 16:49:05 +0530 Subject: [PATCH 2/3] fix: address CodeRabbit rate-limit feedback --- src/context/AppContext.jsx | 20 +++++++++++++++----- src/services/github.js | 8 ++++++-- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/context/AppContext.jsx b/src/context/AppContext.jsx index 24db44db..ee9bf87b 100644 --- a/src/context/AppContext.jsx +++ b/src/context/AppContext.jsx @@ -32,6 +32,7 @@ export function AppProvider({ children }) { const [issuesData, setIssuesData] = useState({}) const [pullsData, setPullsData] = useState({}) const [rateLimit, setRateLimit] = useState(getStoredRateLimit) + const rateLimitRef = useRef(rateLimit) const [loading, setLoading] = useState(false) const [loadMsg, setLoadMsg] = useState('') const [govLoading, setGovLoading] = useState(false) @@ -102,6 +103,7 @@ export function AppProvider({ children }) { const handler = e => { const next = e.detail if (!Number.isFinite(Number(next?.limit)) || !Number.isFinite(Number(next?.remaining))) return + rateLimitRef.current = next setRateLimit(next) localStorage.setItem('oe_rate_limit', JSON.stringify(next)) } @@ -117,6 +119,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())) @@ -133,21 +136,28 @@ export function AppProvider({ children }) { // epoch). Never let a stale read inflate the remaining count while // the current window is still active — only a new window expiry or a // new limit (e.g. PAT added/removed) may legitimately raise it. + const currentRateLimit = rateLimitRef.current if ( - rateLimit && - rl.limit === rateLimit.limit && - rl.remaining > rateLimit.remaining + currentRateLimit && + rl.limit === currentRateLimit.limit && + rl.remaining > currentRateLimit.remaining ) { - const windowActive = !rateLimit.reset || Date.now() < rateLimit.reset * 1000 + const windowActive = currentRateLimit.reset > 0 && Date.now() < currentRateLimit.reset * 1000 if (windowActive) return true } + rateLimitRef.current = rl setRateLimit(rl) localStorage.setItem('oe_rate_limit', JSON.stringify(rl)) return true - }, [pat, rateLimit]) + }, [pat]) const savePat = useCallback(token => { 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 diff --git a/src/services/github.js b/src/services/github.js index 0e663c15..bb223405 100644 --- a/src/services/github.js +++ b/src/services/github.js @@ -134,8 +134,11 @@ export async function fetchPulls(org, repo, pat) { * Returns null when the headers are absent so callers can fall back. */ function readRateLimitHeaders(h) { if (!h || typeof h.get !== 'function') return null - const limit = Number(h.get('x-ratelimit-limit')) - const remaining = Number(h.get('x-ratelimit-remaining')) + const rawLimit = h.get('x-ratelimit-limit') + const rawRemaining = h.get('x-ratelimit-remaining') + if (rawLimit == null || rawRemaining == null) return null + const limit = Number(rawLimit) + const remaining = Number(rawRemaining) if (!Number.isFinite(limit) || !Number.isFinite(remaining)) return null if (limit < 0 || remaining < 0) return null const rawUsed = Number(h.get('x-ratelimit-used')) @@ -154,6 +157,7 @@ function readRateLimitHeaders(h) { /** Validate a rate-limit object from the `/rate_limit` body. */ function asValidRateLimit(obj) { if (!obj) return null + if (obj.limit == null || obj.remaining == null) return null const limit = Number(obj.limit) const remaining = Number(obj.remaining) if (!Number.isFinite(limit) || !Number.isFinite(remaining)) return null From c977428d9f411bb4e8bc9e7676fad69d04043287 Mon Sep 17 00:00:00 2001 From: Nasir Yousuf Date: Sat, 3 Oct 2026 17:25:27 +0530 Subject: [PATCH 3/3] fix: clean rate-limit guards for CodeRabbit --- src/context/AppContext.jsx | 17 ++++++--- src/pages/SettingsPage.jsx | 5 ++- src/services/github.js | 75 +++++++++++++++++++++++--------------- 3 files changed, 60 insertions(+), 37 deletions(-) diff --git a/src/context/AppContext.jsx b/src/context/AppContext.jsx index ee9bf87b..e0e1cfcd 100644 --- a/src/context/AppContext.jsx +++ b/src/context/AppContext.jsx @@ -1,5 +1,5 @@ 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' @@ -33,6 +33,7 @@ export function AppProvider({ children }) { 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) @@ -101,11 +102,11 @@ export function AppProvider({ children }) { useEffect(() => { const handler = e => { - const next = e.detail - if (!Number.isFinite(Number(next?.limit)) || !Number.isFinite(Number(next?.remaining))) return - rateLimitRef.current = next - setRateLimit(next) - localStorage.setItem('oe_rate_limit', JSON.stringify(next)) + const normalized = asValidRateLimit(e.detail) + if (!normalized) return + rateLimitRef.current = normalized + setRateLimit(normalized) + localStorage.setItem('oe_rate_limit', JSON.stringify(normalized)) } window.addEventListener('rate-limit-update', handler) @@ -128,8 +129,10 @@ export function AppProvider({ children }) { }, [rateLimit]) const refreshRateLimit = useCallback(async () => { + const generation = patGenerationRef.current const rl = await fetchRateLimit(pat) if (!rl) return false + if (generation !== patGenerationRef.current) return 'superseded' // `GET /rate_limit` does not consume quota and its `resources.core` // body can lag behind the live `x-ratelimit-*` counters (reads full // while search headers show consumed quota, with a different `reset` @@ -151,6 +154,8 @@ export function AppProvider({ children }) { 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 diff --git a/src/pages/SettingsPage.jsx b/src/pages/SettingsPage.jsx index 85ae0ae5..c19c0a4c 100644 --- a/src/pages/SettingsPage.jsx +++ b/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); } diff --git a/src/services/github.js b/src/services/github.js index bb223405..eb142c7a 100644 --- a/src/services/github.js +++ b/src/services/github.js @@ -3,6 +3,13 @@ const DB_NAME = 'orgexplorer_cache' const STORE = 'cache' const TTL_MS = 3_600_000 // 1 hour +// PAT generation: bumped every time the saved PAT changes (even A -> B -> A). +// Requests capture it at start and drop their quota event if it changed, +// so a late response from an old identity never overwrites current state. +let patGeneration = 0 +export function bumpPatGeneration() { patGeneration += 1 } +export function getPatGeneration() { return patGeneration } + function openDB() { return new Promise((resolve, reject) => { const req = indexedDB.open(DB_NAME, 1) @@ -53,6 +60,7 @@ export async function cacheClear() { // Core fetchWithCache async function fetchWithCache(url, pat) { + const generationAtStart = patGeneration // L2 check const cached = await cacheGet(url) if (cached) return cached @@ -64,9 +72,22 @@ async function fetchWithCache(url, pat) { const live = readRateLimitHeaders(res.headers) if (live) { - window.dispatchEvent( - new CustomEvent('rate-limit-update', { detail: live }) - ) + // Drop quota events from a superseded PAT: if the user saved a new + // token while this request was in flight, its counters belong to the + // old identity and must not overwrite the cleared/current state. + // Generation catches even A -> B -> A (same value, new save). + let superseded = generationAtStart !== patGeneration + if (!superseded) { + try { + const currentPat = typeof localStorage !== 'undefined' ? localStorage.getItem('oe_pat') || '' : null + if (currentPat !== null) superseded = (pat || '') !== currentPat + } catch { /* keep generation-check result on storage failure */ } + } + if (!superseded) { + window.dispatchEvent( + new CustomEvent('rate-limit-update', { detail: live }) + ) + } } if (res.status === 403) throw new Error('RATE_LIMIT') @@ -130,6 +151,23 @@ export async function fetchPulls(org, repo, pat) { return all } +/** Shared numeric validation, nonnegative checks and reset fallback. */ +export function normalizeRateLimit(limitRaw, remainingRaw, usedRaw, resetRaw) { + if (limitRaw == null || remainingRaw == null) return null + const limit = Number(limitRaw) + const remaining = Number(remainingRaw) + if (!Number.isFinite(limit) || !Number.isFinite(remaining)) return null + if (limit < 0 || remaining < 0) return null + const used = Number(usedRaw) + const reset = Number(resetRaw) + return { + limit, + remaining, + used: Number.isFinite(used) ? used : limit - remaining, + reset: Number.isFinite(reset) ? reset : 0, + } +} + /** Read live counters from response headers (authoritative per GitHub docs). * Returns null when the headers are absent so callers can fall back. */ function readRateLimitHeaders(h) { @@ -137,39 +175,18 @@ function readRateLimitHeaders(h) { const rawLimit = h.get('x-ratelimit-limit') const rawRemaining = h.get('x-ratelimit-remaining') if (rawLimit == null || rawRemaining == null) return null - const limit = Number(rawLimit) - const remaining = Number(rawRemaining) - if (!Number.isFinite(limit) || !Number.isFinite(remaining)) return null - if (limit < 0 || remaining < 0) return null const rawUsed = Number(h.get('x-ratelimit-used')) - const rawReset = Number(h.get('x-ratelimit-reset')) + const rawReset = h.get('x-ratelimit-reset') // `x-ratelimit-used` is not in Access-Control-Expose-Headers, so browsers // always read it as null -> 0. Derive it instead of showing a false 0. - const used = Number.isFinite(rawUsed) && rawUsed > 0 ? rawUsed : limit - remaining - return { - limit, - remaining, - used, - reset: Number.isFinite(rawReset) ? rawReset : 0, - } + const usedRaw = Number.isFinite(rawUsed) && rawUsed > 0 ? rawUsed : undefined + return normalizeRateLimit(rawLimit, rawRemaining, usedRaw, rawReset) } /** Validate a rate-limit object from the `/rate_limit` body. */ -function asValidRateLimit(obj) { +export function asValidRateLimit(obj) { if (!obj) return null - if (obj.limit == null || obj.remaining == null) return null - const limit = Number(obj.limit) - const remaining = Number(obj.remaining) - if (!Number.isFinite(limit) || !Number.isFinite(remaining)) return null - if (limit < 0 || remaining < 0) return null - const used = Number(obj.used) - const reset = Number(obj.reset) - return { - limit, - remaining, - used: Number.isFinite(used) ? used : limit - remaining, - reset: Number.isFinite(reset) ? reset : 0, - } + return normalizeRateLimit(obj.limit, obj.remaining, obj.used, obj.reset) } export async function fetchRateLimit(pat) {