fix: load watchable repos in the background and cache them - #24
Merged
Merged
Conversation
The Repos page rebuilt the full available-repo list (personal repos plus every org's repos, across each connected platform) from scratch on every open, inside the popup. That made it slow every time — and because the work ran in the popup, closing it aborted the fetch and the next open started over. On slower connections (e.g. Firefox) this looked like an endless spinner. Move the fetch into the service worker (FETCH_AVAILABLE_REPOS, dedup'd via an in-flight promise) and cache the result in chrome.storage. The popup now renders cache-first, shows an "Updating…" indicator, and re-reads the cache once the refresh resolves. Because the fetch runs in the SW it keeps going after the popup closes, and subsequent opens are instant. A refresh that loads zero repos across all accounts caches an error the page surfaces instead of spinning forever. Fixes #23 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
WalkthroughAdds a background service-worker-driven cache for available watchable repos across GitHub/GitLab/Bitbucket, with deduped in-flight refreshes persisted to ChangesAvailable Repos Caching and Refresh Flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Repos as Repos.tsx
participant Worker as service-worker.ts
participant Storage as storage.ts (chrome.storage.local)
Repos->>Storage: getCachedAvailableRepos()
Storage-->>Repos: cached repos (or null)
Repos->>Worker: sendMessage(FETCH_AVAILABLE_REPOS)
Worker->>Worker: refreshAvailableRepos() (dedup in-flight)
Worker->>Worker: fetch repos per platform/account
Worker->>Storage: saveCachedAvailableRepos({repos, updatedAt, error})
Worker-->>Repos: {done: true}
Repos->>Storage: getCachedAvailableRepos()
Storage-->>Repos: updated repos
Repos->>Repos: buildRepoList() + render (Updating.../error)
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/popup/pages/Repos.tsx (1)
199-202: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winError panel lacks an announcing role for screen readers.
The loading state uses
role="status"(Line 196) so assistive tech announces it, but this new error panel has norole/aria-live, so screen-reader users may not be notified when the fetch fails.♿ Suggested fix
- ) : error && repos.length === 0 ? ( - <div className="px-4 py-8 text-center text-xs text-red-500 dark:text-red-400"> + ) : error && repos.length === 0 ? ( + <div role="alert" className="px-4 py-8 text-center text-xs text-red-500 dark:text-red-400"> {error} </div>As per path instructions, "preserve accessibility requirements such as ARIA labels/roles,
aria-live... " — this new status panel should follow the same pattern as the adjacent loading state.🤖 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/popup/pages/Repos.tsx` around lines 199 - 202, The error panel in Repos should be announced to screen readers just like the adjacent loading state. Update the conditional error render in Repos.tsx to include an appropriate accessibility announcement pattern, such as a status/alert role and/or aria-live on the error container, so assistive tech is notified when fetch fails.Source: Path instructions
src/background/service-worker.ts (1)
95-111: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAccounts are fetched sequentially — consider parallelizing.
Each account's repo fetch is
awaited inside theforloop, so total refresh time is the sum of all accounts' latencies rather than the max. With multiple connected accounts this noticeably delays the "Updating…" resolution.♻️ Suggested parallelization
- for (const account of accounts) { - try { - if (account.platform === 'github') { - const ghRepos = await github.getUserRepos(account.token); - for (const r of ghRepos) repos.push({ platform: 'github', fullName: r.full_name }); - } else if (account.platform === 'gitlab') { - const glRepos = await gitlab.getUserProjects(account.token); - for (const r of glRepos) repos.push({ platform: 'gitlab', fullName: r.path_with_namespace }); - } else if (account.platform === 'bitbucket') { - const bbRepos = await bitbucket.getUserRepositories(account.token); - for (const r of bbRepos) repos.push({ platform: 'bitbucket', fullName: r.full_name }); - } - } catch (err) { - console.error(`[PR Radar] Failed to fetch repos for ${account.platform}:`, err); - failed.push(account.platform); - } - } + await Promise.all(accounts.map(async (account) => { + try { + if (account.platform === 'github') { + const ghRepos = await github.getUserRepos(account.token); + for (const r of ghRepos) repos.push({ platform: 'github', fullName: r.full_name }); + } else if (account.platform === 'gitlab') { + const glRepos = await gitlab.getUserProjects(account.token); + for (const r of glRepos) repos.push({ platform: 'gitlab', fullName: r.path_with_namespace }); + } else if (account.platform === 'bitbucket') { + const bbRepos = await bitbucket.getUserRepositories(account.token); + for (const r of bbRepos) repos.push({ platform: 'bitbucket', fullName: r.full_name }); + } + } catch (err) { + console.error(`[PR Radar] Failed to fetch repos for ${account.platform}:`, err); + failed.push(account.platform); + } + }));🤖 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/background/service-worker.ts` around lines 95 - 111, The repo refresh in service-worker.ts is processing each account sequentially inside the for-loop, which makes total latency add up across accounts. Refactor the account fetch logic in the repo-loading flow to run the per-account GitHub/GitLab/Bitbucket requests in parallel (for example by building a set of promises and awaiting them together), while preserving the existing platform-specific mapping, error handling, and failed-account tracking in the try/catch around each account’s fetch.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/background/service-worker.ts`:
- Around line 122-139: The FETCH_AVAILABLE_REPOS path in
refreshAvailableRepos/doRefreshAvailableRepos can hang indefinitely because the
underlying API calls still rely on plain fetch() without any abort or timeout.
Add a timeout-based AbortController around the available-repos refresh flow
(either inside the shared GitHub/GitLab/Bitbucket request helpers or at the
service-worker message handling boundary) so stalled requests are cancelled and
the popup can recover instead of մն staying in refreshing. Keep the change
localized to the refreshAvailableRepos, doRefreshAvailableRepos, and relevant
API helper methods.
In `@src/shared/storage.ts`:
- Around line 105-136: Move the domain types `AvailableRepo` and
`AvailableReposCache` out of `storage.ts` and into `types.ts`, matching how
`WatchedRepo` is defined and consumed elsewhere. Update
`getCachedAvailableRepos` and `saveCachedAvailableRepos` in `storage.ts` to
import the types from `@/shared/types`, and change any imports such as in
`Repos.tsx` to reference `AvailableRepo` from `@/shared/types` instead of
`@/shared/storage`. Keep `AVAILABLE_REPOS_CACHE_KEY` and the storage helpers in
`storage.ts`; only the type definitions should move.
---
Nitpick comments:
In `@src/background/service-worker.ts`:
- Around line 95-111: The repo refresh in service-worker.ts is processing each
account sequentially inside the for-loop, which makes total latency add up
across accounts. Refactor the account fetch logic in the repo-loading flow to
run the per-account GitHub/GitLab/Bitbucket requests in parallel (for example by
building a set of promises and awaiting them together), while preserving the
existing platform-specific mapping, error handling, and failed-account tracking
in the try/catch around each account’s fetch.
In `@src/popup/pages/Repos.tsx`:
- Around line 199-202: The error panel in Repos should be announced to screen
readers just like the adjacent loading state. Update the conditional error
render in Repos.tsx to include an appropriate accessibility announcement
pattern, such as a status/alert role and/or aria-live on the error container, so
assistive tech is notified when fetch fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b382a491-2e54-4413-a6d1-6d9f05c0f85d
📒 Files selected for processing (5)
CLAUDE.mdsrc/background/service-worker.tssrc/popup/pages/Repos.tsxsrc/shared/storage.tssrc/shared/types.ts
Comment on lines
+122
to
+139
| // Deduplicate concurrent refreshes: a popup reopened mid-fetch awaits the same | ||
| // in-flight run rather than kicking off a second one. | ||
| function refreshAvailableRepos(): Promise<void> { | ||
| if (!availableReposRefresh) { | ||
| availableReposRefresh = doRefreshAvailableRepos().finally(() => { | ||
| availableReposRefresh = null; | ||
| }); | ||
| } | ||
| return availableReposRefresh; | ||
| } | ||
|
|
||
| chrome.runtime.onMessage.addListener((message: Message, _sender, sendResponse) => { | ||
| if (message.type === 'POLL_NOW') { | ||
| pollPRs().then(() => sendResponse({ done: true })); | ||
| return true; // keep channel open for async sendResponse | ||
| } else if (message.type === 'FETCH_AVAILABLE_REPOS') { | ||
| refreshAvailableRepos().then(() => sendResponse({ done: true })); | ||
| return true; // keep channel open for async sendResponse |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -A8 'export (async )?function getUserRepos|export (async )?function getUserProjects|export (async )?function getUserRepositories' src/shared/api
rg -n 'AbortController|signal:|timeout' src/shared/apiRepository: deployhq/pr-radar
Length of output: 2286
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- service worker outline ---'
ast-grep outline src/background/service-worker.ts --view expanded || true
echo '--- popup Repos outline ---'
ast-grep outline src/popup/Repos.tsx --view expanded || true
echo '--- shared api outlines ---'
ast-grep outline src/shared/api/github.ts --view expanded || true
ast-grep outline src/shared/api/gitlab.ts --view expanded || true
ast-grep outline src/shared/api/bitbucket.ts --view expanded || true
echo '--- relevant ranges: service worker ---'
sed -n '1,220p' src/background/service-worker.ts | cat -n
echo '--- relevant ranges: Repos popup ---'
sed -n '1,260p' src/popup/Repos.tsx | cat -n
echo '--- relevant ranges: github api ---'
sed -n '1,280p' src/shared/api/github.ts | cat -n
echo '--- relevant ranges: gitlab api ---'
sed -n '1,220p' src/shared/api/gitlab.ts | cat -n
echo '--- relevant ranges: bitbucket api ---'
sed -n '1,220p' src/shared/api/bitbucket.ts | cat -nRepository: deployhq/pr-radar
Length of output: 21325
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- locate popup repo component ---'
fd -HI 'Repos.tsx|Repos.ts|repos.tsx|repos.ts' src
echo '--- locate FETCH_AVAILABLE_REPOS sendMessage ---'
rg -n -A4 -B4 "FETCH_AVAILABLE_REPOS|refreshAvailableRepos|sendMessage\\(" src
echo '--- github fetch wrappers ---'
sed -n '1,120p' src/shared/api/github.ts | cat -n
echo '--- gitlab fetch wrappers ---'
sed -n '1,80p' src/shared/api/gitlab.ts | cat -n
echo '--- bitbucket fetch wrappers ---'
sed -n '1,60p' src/shared/api/bitbucket.ts | cat -nRepository: deployhq/pr-radar
Length of output: 20501
Add a timeout to the available-repos refresh
doRefreshAvailableRepos() already handles per-account API failures, but src/shared/api/github.ts, src/shared/api/gitlab.ts, and src/shared/api/bitbucket.ts still use plain fetch() with no timeout/abort path. One stalled request can leave FETCH_AVAILABLE_REPOS pending and the popup stuck in refreshing; wrap those calls (or the popup sendMessage) with an AbortController/timeout so the refresh fails closed.
🤖 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/background/service-worker.ts` around lines 122 - 139, The
FETCH_AVAILABLE_REPOS path in refreshAvailableRepos/doRefreshAvailableRepos can
hang indefinitely because the underlying API calls still rely on plain fetch()
without any abort or timeout. Add a timeout-based AbortController around the
available-repos refresh flow (either inside the shared GitHub/GitLab/Bitbucket
request helpers or at the service-worker message handling boundary) so stalled
requests are cancelled and the popup can recover instead of մն staying in
refreshing. Keep the change localized to the refreshAvailableRepos,
doRefreshAvailableRepos, and relevant API helper methods.
Comment on lines
+105
to
+136
| // === Available repos cache === | ||
| // The full list of repos the user *could* watch. Fetching it is slow (personal | ||
| // repos + every org's repos, across each connected platform), so the background | ||
| // service worker fetches it and caches the result here. The Repos page renders | ||
| // from this cache instantly and lets the refresh run in the background — which | ||
| // means it survives the popup being closed. See issue #23. | ||
|
|
||
| export const AVAILABLE_REPOS_CACHE_KEY = 'pr_radar_available_repos'; | ||
|
|
||
| export interface AvailableRepo { | ||
| platform: Platform; | ||
| fullName: string; | ||
| } | ||
|
|
||
| export interface AvailableReposCache { | ||
| repos: AvailableRepo[]; | ||
| updatedAt: number; | ||
| // Set only when nothing could be loaded (every connected account errored), so | ||
| // the popup can show an error instead of an endless spinner. A partial failure | ||
| // still caches the repos that did load and leaves this undefined. | ||
| error?: string; | ||
| } | ||
|
|
||
| export async function getCachedAvailableRepos(): Promise<AvailableReposCache | null> { | ||
| const result = await chrome.storage.local.get(AVAILABLE_REPOS_CACHE_KEY); | ||
| return result[AVAILABLE_REPOS_CACHE_KEY] ?? null; | ||
| } | ||
|
|
||
| export async function saveCachedAvailableRepos(cache: AvailableReposCache): Promise<void> { | ||
| await chrome.storage.local.set({ [AVAILABLE_REPOS_CACHE_KEY]: cache }); | ||
| } | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Move AvailableRepo/AvailableReposCache domain types into types.ts.
These are domain types (analogous to WatchedRepo, which already lives in types.ts), but they're defined here in storage.ts instead. Repos.tsx even imports AvailableRepo from @/shared/storage rather than @/shared/types, which is inconsistent with how other domain types are sourced.
♻️ Suggested move
--- a/src/shared/types.ts
+++ b/src/shared/types.ts
+export interface AvailableRepo {
+ platform: Platform;
+ fullName: string;
+}
+
+export interface AvailableReposCache {
+ repos: AvailableRepo[];
+ updatedAt: number;
+ error?: string;
+}--- a/src/shared/storage.ts
+++ b/src/shared/storage.ts
-export interface AvailableRepo {
- platform: Platform;
- fullName: string;
-}
-
-export interface AvailableReposCache {
- repos: AvailableRepo[];
- updatedAt: number;
- // Set only when nothing could be loaded ...
- error?: string;
-}
+import type { AvailableRepo, AvailableReposCache } from './types';As per path instructions, "In TypeScript source files, use the shared types.ts and constants.ts definitions for domain types, status colors, platform labels, and sound options."
🤖 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/shared/storage.ts` around lines 105 - 136, Move the domain types
`AvailableRepo` and `AvailableReposCache` out of `storage.ts` and into
`types.ts`, matching how `WatchedRepo` is defined and consumed elsewhere. Update
`getCachedAvailableRepos` and `saveCachedAvailableRepos` in `storage.ts` to
import the types from `@/shared/types`, and change any imports such as in
`Repos.tsx` to reference `AvailableRepo` from `@/shared/types` instead of
`@/shared/storage`. Keep `AVAILABLE_REPOS_CACHE_KEY` and the storage helpers in
`storage.ts`; only the type definitions should move.
Source: Path instructions
This was referenced Jul 1, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Reported in #23: on Firefox (Zen) the repo picker showed a spinner "forever." The reporter later clarified it does load — it's just slow, and it doesn't continue if you close the popup.
Root cause:
Repos.tsxrebuilt the entire available-repo list (your personal repos plus every org's repos, for each connected platform) from scratch on every open, and did it inside the popup. So:Fix (background + cache-first)
FETCH_AVAILABLE_REPOSmessage triggersrefreshAvailableRepos(), which is deduplicated via an in-flight promise so a reopened popup awaits the same run instead of starting a second. Because it runs in the SW, the fetch survives the popup closing — exactly the behaviour the reporter expected.chrome.storage. NewAvailableReposCache(getCachedAvailableRepos/saveCachedAvailableRepos). The popup renders cache-first (instant on repeat opens) and shows an "Updating…" indicator while refreshing.Verification
npm run typecheck✅npm run lint✅npm run build✅ (Chrome)Same code path runs on all platforms, so this is verifiable on Chrome as well as the Firefox build. I couldn't reproduce Zen locally, but the change is deterministic (moves work off the popup lifecycle + adds caching) and directly addresses the reported behaviour.
Fixes #23
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes