From 9f720c66af882c361f205fc438060d2d7f4dbf8f Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Sun, 28 Jun 2026 23:41:14 +0300 Subject: [PATCH] fix(github): stop PR-status requests from starving startup connection pool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Watching N worktrees fired N PR-status requests at once (startWatching called refresh() directly, bypassing the batch limiter). Each request can take 20s+ under GitHub secondary-rate-limiting, and N of them saturate the browser's ~6 HTTP/1.1 connections per origin, starving the critical path (bootstrap session.status, diffs, sending messages) until they finish — the UI appeared frozen for ~20s on startup. - Gate all PR-status network calls through a global concurrency semaphore (max 2), so free sockets always remain for critical traffic. - Bound resolveGitHubPrStatus with a 12s timeout so a slow request fails fast instead of holding a socket; the client keeps its last-known status. - Reuse already-fetched repo metadata for the default branch instead of a redundant repos.get, reducing serial GitHub calls (less rate-limiting). --- .../ui/src/stores/useGitHubPrStatusStore.ts | 45 ++++++++++++++++++- packages/web/server/lib/github/pr-status.js | 11 +++++ packages/web/server/lib/github/routes.js | 35 ++++++++++++--- 3 files changed, 84 insertions(+), 7 deletions(-) diff --git a/packages/ui/src/stores/useGitHubPrStatusStore.ts b/packages/ui/src/stores/useGitHubPrStatusStore.ts index 6cf8a2c5..92933750 100644 --- a/packages/ui/src/stores/useGitHubPrStatusStore.ts +++ b/packages/ui/src/stores/useGitHubPrStatusStore.ts @@ -93,6 +93,43 @@ const timers = new Map(); const bootstrapTimers = new Map(); const inFlightBySignature = new Set(); const lastRefreshBySignature = new Map(); + +// Global concurrency gate for PR-status network requests. +// +// PR status is non-critical chrome, but each request can be slow (the server +// makes many serial GitHub API calls and GitHub secondary-rate-limits bursts, +// so a single request can take 20s+). The browser allows only ~6 concurrent +// HTTP/1.1 connections per origin. Without this cap, watching N worktrees fires +// N PR-status requests at once (each startWatching() calls refresh() directly, +// bypassing refreshTargets' batch limiter), which saturates the connection pool +// and starves the critical path (bootstrap session.status, diffs, sending +// messages) for the full duration — the whole UI appears frozen on startup. +// +// Capping concurrency low guarantees free sockets remain for critical traffic. +const PR_STATUS_NETWORK_CONCURRENCY = 2; +let prStatusNetworkActive = 0; +const prStatusNetworkWaiters: Array<() => void> = []; + +const acquirePrStatusNetworkSlot = (): Promise => { + if (prStatusNetworkActive < PR_STATUS_NETWORK_CONCURRENCY) { + prStatusNetworkActive += 1; + return Promise.resolve(); + } + return new Promise((resolve) => { + prStatusNetworkWaiters.push(resolve); + }); +}; + +const releasePrStatusNetworkSlot = (): void => { + const next = prStatusNetworkWaiters.shift(); + if (next) { + // Hand the slot directly to the next waiter — keep the active count steady. + next(); + return; + } + prStatusNetworkActive = Math.max(0, prStatusNetworkActive - 1); +}; + const createEntry = (): PrStatusEntry => ({ status: null, isLoading: false, @@ -488,7 +525,13 @@ export const useGitHubPrStatusStore = create()( activeRequestCount: prev.activeRequestCount + 1, totalRequestCount: prev.totalRequestCount + 1, })); - const next = await params.github.prStatus(params.directory, params.branch, params.remoteName ?? undefined, { force: options?.force }); + await acquirePrStatusNetworkSlot(); + let next: GitHubPullRequestStatus; + try { + next = await params.github.prStatus(params.directory, params.branch, params.remoteName ?? undefined, { force: options?.force }); + } finally { + releasePrStatusNetworkSlot(); + } set((prev) => { const nextEntries = { ...prev.entries }; signatureKeys.forEach((signatureKey) => { diff --git a/packages/web/server/lib/github/pr-status.js b/packages/web/server/lib/github/pr-status.js index 6374827d..4f8e27f8 100644 --- a/packages/web/server/lib/github/pr-status.js +++ b/packages/web/server/lib/github/pr-status.js @@ -160,6 +160,17 @@ const getRepoDefaultBranch = async (octokit, repo) => { return cached.defaultBranch; } + // Reuse the full repo metadata if it was already fetched (expandRepoNetwork + // calls getRepoMetadata for every candidate before the default-branch loop). + // This avoids a redundant repos.get per repo — fewer serial GitHub calls means + // less exposure to secondary-rate-limiting that makes PR status slow. + const metaCached = repoMetadataCache.get(repoKey); + if (metaCached && Date.now() - metaCached.fetchedAt < REPO_DEFAULT_BRANCH_TTL_MS) { + const defaultBranch = normalizeText(metaCached.data?.default_branch) || null; + defaultBranchCache.set(repoKey, { defaultBranch, fetchedAt: Date.now() }); + return defaultBranch; + } + try { const response = await octokit.rest.repos.get({ owner: repo.owner, diff --git a/packages/web/server/lib/github/routes.js b/packages/web/server/lib/github/routes.js index ea9b975f..6307c268 100644 --- a/packages/web/server/lib/github/routes.js +++ b/packages/web/server/lib/github/routes.js @@ -1,7 +1,26 @@ const PR_STATUS_CACHE_TTL_MS = 90_000; const PR_STATUS_CACHE_MAX_ENTRIES = 200; +// Upper bound for resolving a single PR status. resolveGitHubPrStatus makes many +// serial GitHub API calls; under GitHub secondary-rate-limiting a single request +// can otherwise hang 20s+. We bound it so the route fails fast instead of holding +// the response (and a client socket) open — the client keeps its last-known +// status on error, and a later poll fills it in. +const PR_STATUS_RESOLVE_TIMEOUT_MS = 12_000; const prStatusCache = new Map(); +function withTimeout(promise, timeoutMs, label) { + let timer; + const timeout = new Promise((_resolve, reject) => { + timer = setTimeout(() => { + const error = new Error(`${label} timed out after ${timeoutMs}ms`); + error.code = 'ETIMEDOUT'; + reject(error); + }, timeoutMs); + if (typeof timer.unref === 'function') timer.unref(); + }); + return Promise.race([promise, timeout]).finally(() => clearTimeout(timer)); +} + function getRequestedRepo(req) { const owner = typeof req.query?.owner === 'string' ? req.query.owner.trim() : ''; const repo = typeof req.query?.repo === 'string' ? req.query.repo.trim() : ''; @@ -417,12 +436,16 @@ export function registerGitHubRoutes(app) { } const { resolveGitHubPrStatus } = await import('./pr-status.js'); - const resolvedStatus = await resolveGitHubPrStatus({ - octokit, - directory, - branch, - remoteName: remote, - }); + const resolvedStatus = await withTimeout( + resolveGitHubPrStatus({ + octokit, + directory, + branch, + remoteName: remote, + }), + PR_STATUS_RESOLVE_TIMEOUT_MS, + 'resolveGitHubPrStatus', + ); const searchRepo = resolvedStatus.repo; const first = resolvedStatus.pr; if (!searchRepo) {