Skip to content
Merged
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
41 changes: 37 additions & 4 deletions apps/server/src/routes/worktree/routes/list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,15 @@ interface GitHubRemoteCacheEntry {
checkedAt: number;
}

interface GitHubPRCacheEntry {
prs: Map<string, WorktreePRInfo>;
fetchedAt: number;
}

const githubRemoteCache = new Map<string, GitHubRemoteCacheEntry>();
const githubPRCache = new Map<string, GitHubPRCacheEntry>();
const GITHUB_REMOTE_CACHE_TTL_MS = 5 * 60 * 1000; // 5 minutes
const GITHUB_PR_CACHE_TTL_MS = 2 * 60 * 1000; // 2 minutes - avoid hitting GitHub on every poll

interface WorktreeInfo {
path: string;
Expand Down Expand Up @@ -180,9 +187,21 @@ async function getGitHubRemoteStatus(projectPath: string): Promise<GitHubRemoteS
* This also allows detecting PRs that were created outside the app.
*
* Uses cached GitHub remote status to avoid repeated warnings when the
* project doesn't have a GitHub remote configured.
* project doesn't have a GitHub remote configured. Results are cached
* briefly to avoid hammering GitHub on frequent worktree polls.
*/
async function fetchGitHubPRs(projectPath: string): Promise<Map<string, WorktreePRInfo>> {
async function fetchGitHubPRs(
projectPath: string,
forceRefresh = false
): Promise<Map<string, WorktreePRInfo>> {
const now = Date.now();
const cached = githubPRCache.get(projectPath);

// Return cached result if valid and not forcing refresh
if (!forceRefresh && cached && now - cached.fetchedAt < GITHUB_PR_CACHE_TTL_MS) {
return cached.prs;
}

const prMap = new Map<string, WorktreePRInfo>();

try {
Expand Down Expand Up @@ -225,8 +244,22 @@ async function fetchGitHubPRs(projectPath: string): Promise<Map<string, Worktree
createdAt: pr.createdAt,
});
}

// Only update cache on successful fetch
githubPRCache.set(projectPath, {
prs: prMap,
fetchedAt: Date.now(),
});
} catch (error) {
// Silently fail - PR detection is optional
// On fetch failure, return stale cached data if available to avoid
// repeated API calls during GitHub API flakiness or temporary outages
if (cached) {
logger.warn(`Failed to fetch GitHub PRs, returning stale cache: ${getErrorMessage(error)}`);
// Extend cache TTL to avoid repeated retries during outages
githubPRCache.set(projectPath, { prs: cached.prs, fetchedAt: Date.now() });
return cached.prs;
}
// No cache available, log warning and return empty map
logger.warn(`Failed to fetch GitHub PRs: ${getErrorMessage(error)}`);
Comment on lines +254 to 263
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid retry storms when there’s no cached PR data.
If the first fetch fails (rate limit/outage) and there’s no cache, every poll will still hit GitHub. Consider negative-caching an empty map for a short window to suppress repeated retries.

🛠️ Suggested fix
     if (cached) {
       logger.warn(`Failed to fetch GitHub PRs, returning stale cache: ${getErrorMessage(error)}`);
       // Extend cache TTL to avoid repeated retries during outages
       githubPRCache.set(projectPath, { prs: cached.prs, fetchedAt: Date.now() });
       return cached.prs;
     }
     // No cache available, log warning and return empty map
     logger.warn(`Failed to fetch GitHub PRs: ${getErrorMessage(error)}`);
+    const empty = new Map<string, WorktreePRInfo>();
+    githubPRCache.set(projectPath, { prs: empty, fetchedAt: Date.now() });
+    return empty;
🤖 Prompt for AI Agents
In `@apps/server/src/routes/worktree/routes/list.ts` around lines 254 - 263, When
a fetch for GitHub PRs fails and there is no existing cached entry, add a short
negative-cache entry to githubPRCache for the given projectPath (e.g., store
prs: new Map() or empty map and fetchedAt: Date.now()) so subsequent polls use
the negative cache instead of hammering GitHub; update the error logging that
uses getErrorMessage(error) as before, and ensure the negative TTL is short
(e.g., seconds-to-minutes) by relying on the same cache expiry mechanism so
normal polling resumes after the window; make this change in the same failure
branch where you currently log "Failed to fetch GitHub PRs" so callers of the
function that reads githubPRCache will see the empty map until expiry.

}

Expand Down Expand Up @@ -364,7 +397,7 @@ export function createListHandler() {
// Only fetch GitHub PRs if includeDetails is requested (performance optimization).
// Uses --state all to detect merged/closed PRs, limited to 1000 recent PRs.
const githubPRs = includeDetails
? await fetchGitHubPRs(projectPath)
? await fetchGitHubPRs(projectPath, forceRefreshGitHub)
: new Map<string, WorktreePRInfo>();

for (const worktree of worktrees) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,20 @@ export function useWorktrees({
);

// fetchWorktrees for backward compatibility - now just triggers a refetch
const fetchWorktrees = useCallback(async () => {
await queryClient.invalidateQueries({
queryKey: queryKeys.worktrees.all(projectPath),
});
return refetch();
}, [projectPath, queryClient, refetch]);
// The silent option is accepted but not used (React Query handles loading states)
// Returns removed worktrees array if any were detected, undefined otherwise
const fetchWorktrees = useCallback(
async (_options?: {
silent?: boolean;
}): Promise<Array<{ path: string; branch: string }> | undefined> => {
await queryClient.invalidateQueries({
queryKey: queryKeys.worktrees.all(projectPath),
});
const result = await refetch();
return result.data?.removedWorktrees;
},
[projectPath, queryClient, refetch]
);

const currentWorktreePath = currentWorktree?.path ?? null;
const selectedWorktree = currentWorktreePath
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -383,13 +383,13 @@ export function WorktreePanel({

const isMobile = useIsMobile();

// Periodic interval check (5 seconds) to detect branch changes on disk
// Reduced from 1s to 5s to minimize GPU/CPU usage from frequent re-renders
// Periodic interval check (30 seconds) to detect branch changes on disk
// Reduced polling to lessen repeated worktree list calls while keeping UI reasonably fresh
const intervalRef = useRef<NodeJS.Timeout | null>(null);
useEffect(() => {
intervalRef.current = setInterval(() => {
fetchWorktrees({ silent: true });
}, 5000);
}, 30000);

return () => {
if (intervalRef.current) {
Expand Down