Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/components/RateLimitBanner.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ export default function RateLimitBanner() {
</span>
)}
</span>
<span style={{ fontSize: 11, color: 'var(--text2)' }}>RESETS HOURLY</span>
<span style={{ fontSize: 11, color: 'var(--text2)' }}>
Used: {rateLimit.used} • Reset at{' '}
{new Date(rateLimit.reset * 1000).toLocaleTimeString()}
</span>
Comment on lines +36 to +39

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add fallback for potentially undefined rate limit properties.

If the rate-limit-update event payload is incomplete (due to the missing validation in AppContext), rateLimit.used or rateLimit.reset could be undefined, displaying "Used: undefined" or an invalid date.

🛡️ Proposed fix with fallbacks
 <span style={{ fontSize: 11, color: 'var(--text2)' }}>
-  Used: {rateLimit.used} • Reset at{' '}
-  {new Date(rateLimit.reset * 1000).toLocaleTimeString()}
+  Used: {rateLimit.used ?? 0} • Reset at{' '}
+  {rateLimit.reset ? new Date(rateLimit.reset * 1000).toLocaleTimeString() : 'Unknown'}
 </span>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/RateLimitBanner.jsx` around lines 36 - 39, The span element in
RateLimitBanner directly accesses rateLimit.used and rateLimit.reset without
checking if these properties exist, which could display "undefined" or create
invalid dates if the event payload is incomplete. Add fallback values for both
rateLimit.used (such as a default value like 0) and rateLimit.reset (such as a
default value like 0) using optional chaining operators or logical OR operators
to ensure valid display content even when these properties are undefined.

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Consider adding timezone and locale control for better internationalization.

The toLocaleTimeString() call uses default browser locale and no timezone information is displayed. Users in different timezones may be confused about when the rate limit actually resets, especially since GitHub's rate limits reset in UTC.

🌍 Optional enhancement for i18n
 <span style={{ fontSize: 11, color: 'var(--text2)' }}>
   Used: {rateLimit.used} • Reset at{' '}
-  {new Date(rateLimit.reset * 1000).toLocaleTimeString()}
+  {new Date(rateLimit.reset * 1000).toLocaleTimeString(undefined, { 
+    hour: '2-digit', 
+    minute: '2-digit',
+    second: '2-digit',
+    timeZoneName: 'short'
+  })}
 </span>

This adds timezone information (e.g., "4:07:57 PM PST") to help users understand when the reset occurs in their local time.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<span style={{ fontSize: 11, color: 'var(--text2)' }}>
Used: {rateLimit.used} Reset at{' '}
{new Date(rateLimit.reset * 1000).toLocaleTimeString()}
</span>
<span style={{ fontSize: 11, color: 'var(--text2)' }}>
Used: {rateLimit.used} Reset at{' '}
{new Date(rateLimit.reset * 1000).toLocaleTimeString(undefined, {
hour: '2-digit',
minute: '2-digit',
second: '2-digit',
timeZoneName: 'short'
})}
</span>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/RateLimitBanner.jsx` around lines 36 - 39, The
toLocaleTimeString() method call on the rateLimit.reset date in the
RateLimitBanner component displays only the time without timezone information,
which can confuse users about when the rate limit actually resets. Modify the
toLocaleTimeString() call to include timezone information by passing an options
object as a second parameter that specifies timeZoneName property, so users see
both the local time and their timezone abbreviation (e.g., "4:07:57 PM PST") for
better clarity across different user timezones.

Source: Coding guidelines

</div>
)
}
70 changes: 56 additions & 14 deletions src/context/AppContext.jsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,63 @@
import { createContext, useContext, useState, useCallback } from 'react'
import { fetchOrg, fetchRepos, fetchContributors, fetchIssues, fetchRateLimit } from '../services/github'
import { createContext, useContext, useState, useCallback, useEffect } from 'react'
import { fetchOrg, fetchRepos, fetchContributors, fetchIssues, } from '../services/github'
import { buildAnalyticalModel } from '../services/analytics'

const Ctx = createContext(null)

function getStoredRateLimit() {
const stored = localStorage.getItem('oe_rate_limit')

if (!stored) return null

try {
const data = JSON.parse(stored)

if (Date.now() > data.reset * 1000) {
localStorage.removeItem('oe_rate_limit')
return null
}

return data
} catch {
localStorage.removeItem('oe_rate_limit')
return null
}
}

export function AppProvider({ children }) {
const [pat, setPat] = useState(() => localStorage.getItem('oe_pat') || '')
const [orgs, setOrgs] = useState([])
const [model, setModel] = useState(null)
const [pat, setPat] = useState(() => localStorage.getItem('oe_pat') || '')
const [orgs, setOrgs] = useState([])
const [model, setModel] = useState(null)
const [issuesData, setIssuesData] = useState({})
const [rateLimit, setRateLimit] = useState(null)
const [loading, setLoading] = useState(false)
const [loadMsg, setLoadMsg] = useState('')
const [rateLimit, setRateLimit] = useState(getStoredRateLimit)
const [loading, setLoading] = useState(false)
const [loadMsg, setLoadMsg] = useState('')
const [govLoading, setGovLoading] = useState(false)
const [error, setError] = useState('')
const [error, setError] = useState('')

useEffect(() => {
const handler = e => {
setRateLimit(e.detail)
localStorage.setItem('oe_rate_limit', JSON.stringify(e.detail))
}

window.addEventListener('rate-limit-update', handler)

return () => {
window.removeEventListener('rate-limit-update', handler)
}
}, [])
Comment on lines +38 to +49

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Validate event payload to prevent malformed rate limit data.

The event handler (lines 39-42) directly uses e.detail without validation. If a malicious script or browser extension dispatches a rate-limit-update event with invalid data, it could corrupt the rate limit state and localStorage.

🔒 Proposed fix with validation
 useEffect(() => {
   const handler = e => {
+    const { limit, remaining, used, reset } = e.detail || {}
+    
+    if (
+      typeof limit !== 'number' || limit < 0 ||
+      typeof remaining !== 'number' || remaining < 0 ||
+      typeof used !== 'number' || used < 0 ||
+      typeof reset !== 'number' || reset <= 0
+    ) {
+      console.warn('Invalid rate-limit-update event payload:', e.detail)
+      return
+    }
+    
     setRateLimit(e.detail)
     localStorage.setItem('oe_rate_limit', JSON.stringify(e.detail))
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
useEffect(() => {
const handler = e => {
setRateLimit(e.detail)
localStorage.setItem('oe_rate_limit', JSON.stringify(e.detail))
}
window.addEventListener('rate-limit-update', handler)
return () => {
window.removeEventListener('rate-limit-update', handler)
}
}, [])
useEffect(() => {
const handler = e => {
const { limit, remaining, used, reset } = e.detail || {}
if (
typeof limit !== 'number' || limit < 0 ||
typeof remaining !== 'number' || remaining < 0 ||
typeof used !== 'number' || used < 0 ||
typeof reset !== 'number' || reset <= 0
) {
console.warn('Invalid rate-limit-update event payload:', e.detail)
return
}
setRateLimit(e.detail)
localStorage.setItem('oe_rate_limit', JSON.stringify(e.detail))
}
window.addEventListener('rate-limit-update', handler)
return () => {
window.removeEventListener('rate-limit-update', handler)
}
}, [])
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/context/AppContext.jsx` around lines 38 - 49, The handler function in the
useEffect for the 'rate-limit-update' event listener directly uses e.detail
without validation before calling setRateLimit and storing it in localStorage
with the key 'oe_rate_limit'. Add validation logic to verify that e.detail is a
valid object with the expected structure and properties (such as checking that
it is an object type and has required fields) before proceeding with
setRateLimit and localStorage.setItem. Only allow the state and localStorage
updates to occur if the validation passes.


useEffect(() => {
if (!rateLimit?.reset) return

const timeout = setTimeout(() => {
localStorage.removeItem('oe_rate_limit')
setRateLimit(null)
}, Math.max(0, rateLimit.reset * 1000 - Date.now()))

return () => clearTimeout(timeout)
}, [rateLimit])

const savePat = useCallback(token => {
setPat(token)
Expand All @@ -25,7 +69,7 @@ export function AppProvider({ children }) {
setLoading(true); setError(''); setModel(null); setOrgs([]); setIssuesData({})
try {
setLoadMsg('Fetching organization metadata...')
const orgRes = await Promise.allSettled(orgNames.map(n => fetchOrg(n, pat)))
const orgRes = await Promise.allSettled(orgNames.map(n => fetchOrg(n, pat)))
const validOrgs = orgRes.filter(r => r.status === 'fulfilled').map(r => r.value)
if (!validOrgs.length) throw new Error('No valid organizations found. Check the names and try again.')
setOrgs(validOrgs)
Expand All @@ -50,11 +94,9 @@ export function AppProvider({ children }) {
setLoadMsg('Building analytical data model...')
setModel(buildAnalyticalModel(validOrgs, reposPerOrg, contribsPerRepo))

const rl = await fetchRateLimit(pat)
if (rl) setRateLimit(rl)

// Save to recent searches
const prev = JSON.parse(localStorage.getItem('oe_recent') || '[]')
const prev = JSON.parse(localStorage.getItem('oe_recent') || '[]')
const entry = orgNames.join(', ')
localStorage.setItem('oe_recent', JSON.stringify([...new Set([entry, ...prev])].slice(0, 6)))
return true
Expand All @@ -72,7 +114,7 @@ export function AppProvider({ children }) {
const runAudit = useCallback(async () => {
if (!model || govLoading) return
setGovLoading(true)
const map = {}
const map = {}
const repos = model.allRepos.slice(0, 15)

// Batches of 5 using Promise.allSettled
Expand Down
28 changes: 20 additions & 8 deletions src/services/github.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
// IndexedDB Cache (L2)
const DB_NAME = 'orgexplorer_cache'
const STORE = 'cache'
const TTL_MS = 3_600_000 // 1 hour
const STORE = 'cache'
const TTL_MS = 3_600_000 // 1 hour

function openDB() {
return new Promise((resolve, reject) => {
const req = indexedDB.open(DB_NAME, 1)
req.onupgradeneeded = e => e.target.result.createObjectStore(STORE, { keyPath: 'k' })
req.onsuccess = e => resolve(e.target.result)
req.onerror = () => reject(req.error)
req.onsuccess = e => resolve(e.target.result)
req.onerror = () => reject(req.error)
})
}

Expand All @@ -34,7 +34,7 @@ export async function cacheSet(key, value) {
const tx = db.transaction(STORE, 'readwrite')
tx.objectStore(STORE).put({ k: key, v: value, ts: Date.now() })
tx.oncomplete = () => res(true)
tx.onerror = () => res(false)
tx.onerror = () => res(false)
})
} catch { return false }
}
Expand All @@ -46,7 +46,7 @@ export async function cacheClear() {
const tx = db.transaction(STORE, 'readwrite')
tx.objectStore(STORE).clear()
tx.oncomplete = () => res(true)
tx.onerror = () => res(false)
tx.onerror = () => res(false)
})
} catch { return false }
}
Expand All @@ -61,6 +61,18 @@ async function fetchWithCache(url, pat) {
if (pat) headers.Authorization = `token ${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'))
}
})
)
Comment on lines +65 to +74

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add defensive null checks for rate limit headers.

The code converts header values using Number() without checking if the headers exist. If GitHub doesn't send rate limit headers (edge case), Number(null) returns 0, which would incorrectly display "0/0" remaining.

🛡️ Proposed fix with validation
-  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 limit = res.headers.get('x-ratelimit-limit')
+  const remaining = res.headers.get('x-ratelimit-remaining')
+  const used = res.headers.get('x-ratelimit-used')
+  const reset = res.headers.get('x-ratelimit-reset')
+  
+  if (limit && remaining && used && reset) {
+    window.dispatchEvent(
+      new CustomEvent('rate-limit-update', {
+        detail: {
+          limit: Number(limit),
+          remaining: Number(remaining),
+          used: Number(used),
+          reset: Number(reset)
+        }
+      })
+    )
+  }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/services/github.js` around lines 65 - 74, Add null checks before
converting rate limit header values to numbers in the custom event dispatch. For
each header retrieval (x-ratelimit-limit, x-ratelimit-remaining,
x-ratelimit-used, x-ratelimit-reset), verify that the header exists before
passing it to Number(). Either check if the header value is null before
conversion or provide sensible default values when headers are missing. This
prevents Number(null) from returning 0 and ensures rate limit information
displays correctly even when headers are absent.


if (res.status === 403) throw new Error('RATE_LIMIT')
if (res.status === 404) throw new Error('NOT_FOUND')
if (!res.ok) throw new Error(`HTTP_${res.status}`)
Expand All @@ -77,7 +89,7 @@ export const fetchOrg = (org, pat) =>
export async function fetchRepos(org, pat) {
const all = []
for (let page = 1; page <= 5; page++) {
const url = `https://api.github.com/orgs/${org}/repos?per_page=100&page=${page}&sort=updated`
const url = `https://api.github.com/orgs/${org}/repos?per_page=100&page=${page}&sort=updated`
const data = await fetchWithCache(url, pat)
all.push(...data)
if (data.length < 100) break
Expand Down Expand Up @@ -105,7 +117,7 @@ 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 res = await fetch('https://api.github.com/rate_limit', { headers })
const data = await res.json()
return data.rate
} catch { return null }
Expand Down
Loading