From fb5bc4fd24ffdf5ca760845feb71228ddc2394d3 Mon Sep 17 00:00:00 2001 From: Mark Xian Date: Thu, 16 Jul 2026 04:39:16 +0800 Subject: [PATCH] fix(gitea): share and cache the /pulls scan so card refreshes can't hammer a self-hosted forge (#8858) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(gitea): share and cache the /pulls scan so card refreshes can't hammer a self-hosted forge Each worktree card resolved its branch by paginating the repo's full /pulls listing (state=all, up to 5 pages) independently, with a 5s timeout. Gitea/Forgejo have no head-branch filter, self-hosted Forgejo takes ~5s per page (it loads reviewer data per PR), and a push event refreshes every card at once — so one repo turned into hundreds of near-simultaneous requests whose responses were aborted right as they completed, and the burst OOM-killed a 512Mi Forgejo pod (#8807). - Share one in-flight /pulls scan per repo across concurrent branch lookups and cache the result for 30s, so a burst costs a single page walk instead of one per worktree. - Raise the list-scan timeout to 15s so slow-but-successful pages are used instead of discarded and retried. - Invalidate the cached scan after Orca itself creates a PR (and before the post-create fallback lookup) so the new PR is seen immediately. Fixes #8807 Co-Authored-By: Claude Fable 5 * fix(gitea): don't let an in-flight scan re-cache a listing from before an invalidation Review feedback: if a PR is created while a card-refresh scan is in flight, the invalidation cleared the cache but the scan then finished and re-cached the pre-create listing, hiding the new PR for a full TTL. Track a per-repo generation: invalidation bumps it (and drops the in-flight promise), and a scan only writes the cache when its generation is still current. Co-Authored-By: Claude Fable 5 * fix(gitea): bound PR scan cache and retry failures Co-authored-by: Orca --------- Co-authored-by: Claude Fable 5 Co-authored-by: Jinwoo-H Co-authored-by: Orca --- src/main/gitea/client.test.ts | 145 +++++++++++++++++++ src/main/gitea/client.ts | 43 ++++-- src/main/gitea/pull-request-creation.ts | 9 +- src/main/gitea/pull-request-scan-cache.ts | 161 ++++++++++++++++++++++ 4 files changed, 342 insertions(+), 16 deletions(-) create mode 100644 src/main/gitea/pull-request-scan-cache.ts diff --git a/src/main/gitea/client.test.ts b/src/main/gitea/client.test.ts index 620ab33225f..bf1fe13ca72 100644 --- a/src/main/gitea/client.test.ts +++ b/src/main/gitea/client.test.ts @@ -14,6 +14,11 @@ import { normalizeGiteaApiBaseUrl } from './client' import { _resetGiteaRepoRefCache } from './repository-ref' +import { + _getGiteaPullRequestScanCacheSize, + _resetGiteaPullRequestScanCache, + scanGiteaPullRequests +} from './pull-request-scan-cache' const OLD_ENV = process.env @@ -44,6 +49,7 @@ describe('Gitea client', () => { stderr: '' }) _resetGiteaRepoRefCache() + _resetGiteaPullRequestScanCache() vi.unstubAllGlobals() }) @@ -92,6 +98,145 @@ describe('Gitea client', () => { expect(listUrl.searchParams.get('limit')).toBe('50') }) + it('shares one /pulls scan across concurrent branch lookups (#8807)', async () => { + let listCalls = 0 + const fetchMock = vi.fn(async (url: string) => { + const parsed = new URL(url) + if (parsed.pathname.endsWith('/status')) { + return Response.json({ state: 'success' }) + } + listCalls++ + return Response.json([giteaPr(7, 'feature/a'), giteaPr(8, 'feature/b')]) + }) + vi.stubGlobal('fetch', fetchMock) + + const [a, b, missing] = await Promise.all([ + getGiteaPullRequestForBranch('/repo', 'feature/a'), + getGiteaPullRequestForBranch('/repo', 'feature/b'), + getGiteaPullRequestForBranch('/repo', 'feature/none') + ]) + + expect(a?.number).toBe(7) + expect(b?.number).toBe(8) + expect(missing).toBeNull() + expect(listCalls).toBe(1) + }) + + it('reuses the cached /pulls scan for lookups inside the TTL', async () => { + let listCalls = 0 + const fetchMock = vi.fn(async (url: string) => { + const parsed = new URL(url) + if (parsed.pathname.endsWith('/status')) { + return Response.json({ state: 'success' }) + } + listCalls++ + return Response.json([giteaPr()]) + }) + vi.stubGlobal('fetch', fetchMock) + + await getGiteaPullRequestForBranch('/repo', 'feature/gitea') + await getGiteaPullRequestForBranch('/repo', 'feature/gitea') + await getGiteaPullRequestForBranch('/repo', 'no-pr-branch') + + expect(listCalls).toBe(1) + }) + + it('retries a failed /pulls scan after only the short failure cooldown', async () => { + vi.useFakeTimers() + try { + let listCalls = 0 + const fetchMock = vi.fn(async (url: string) => { + const parsed = new URL(url) + if (parsed.pathname.endsWith('/status')) { + return Response.json({ state: 'success' }) + } + listCalls++ + return listCalls === 1 + ? Response.json({ message: 'temporary failure' }, { status: 503 }) + : Response.json([giteaPr()]) + }) + vi.stubGlobal('fetch', fetchMock) + + await expect(getGiteaPullRequestForBranch('/repo', 'feature/gitea')).resolves.toBeNull() + await expect(getGiteaPullRequestForBranch('/repo', 'feature/gitea')).resolves.toBeNull() + expect(listCalls).toBe(1) + + await vi.advanceTimersByTimeAsync(3_001) + await expect(getGiteaPullRequestForBranch('/repo', 'feature/gitea')).resolves.toMatchObject({ + number: 7 + }) + expect(listCalls).toBe(2) + } finally { + vi.useRealTimers() + } + }) + + it('expires successful scans and bounds retained repository listings', async () => { + vi.useFakeTimers() + try { + let listCalls = 0 + const fetchMock = vi.fn(async (url: string) => { + const parsed = new URL(url) + if (parsed.pathname.endsWith('/status')) { + return Response.json({ state: 'success' }) + } + listCalls++ + return Response.json([giteaPr()]) + }) + vi.stubGlobal('fetch', fetchMock) + + await getGiteaPullRequestForBranch('/repo', 'feature/gitea') + expect(_getGiteaPullRequestScanCacheSize()).toBe(1) + await vi.advanceTimersByTimeAsync(30_001) + expect(_getGiteaPullRequestScanCacheSize()).toBe(0) + await getGiteaPullRequestForBranch('/repo', 'feature/gitea') + expect(listCalls).toBe(2) + + await Promise.all( + Array.from({ length: 40 }, (_, index) => + scanGiteaPullRequests(`repo-${index}`, async () => [], 50, 5) + ) + ) + expect(_getGiteaPullRequestScanCacheSize()).toBe(32) + } finally { + vi.useRealTimers() + } + }) + + it('does not let an in-flight scan re-cache results from before an invalidation', async () => { + let releaseFirstScan!: () => void + const firstScanGate = new Promise((resolve) => { + releaseFirstScan = resolve + }) + let listCalls = 0 + const fetchMock = vi.fn(async (url: string) => { + const parsed = new URL(url) + if (parsed.pathname.endsWith('/status')) { + return Response.json({ state: 'success' }) + } + listCalls++ + if (listCalls === 1) { + // First scan is in flight (pre-create listing) when the invalidation lands. + await firstScanGate + return Response.json([giteaPr(7, 'feature/old')]) + } + return Response.json([giteaPr(7, 'feature/old'), giteaPr(8, 'feature/new')]) + }) + vi.stubGlobal('fetch', fetchMock) + + const staleScanRead = getGiteaPullRequestForBranch('/repo', 'feature/old') + const { invalidateGiteaPullRequestScanForRepo, getGiteaRepoSlug } = await import('./client') + const repo = await getGiteaRepoSlug('/repo') + invalidateGiteaPullRequestScanForRepo(repo!) + releaseFirstScan() + await staleScanRead + + await expect(getGiteaPullRequestForBranch('/repo', 'feature/new')).resolves.toMatchObject({ + number: 8 + }) + expect(listCalls).toBe(2) + }) + it('uses an API base URL override for subpath or non-standard deployments', async () => { process.env.ORCA_GITEA_API_BASE_URL = 'https://git.example.com/code' const fetchMock = vi.fn(async (url: string | URL) => { diff --git a/src/main/gitea/client.ts b/src/main/gitea/client.ts index 300ecda6e04..30545f83ad7 100644 --- a/src/main/gitea/client.ts +++ b/src/main/gitea/client.ts @@ -6,12 +6,17 @@ import { type RawGiteaPullRequest } from './pull-request-mappers' import { getGiteaRepoRef, type GiteaRepoRef } from './repository-ref' +import { invalidateGiteaPullRequestScan, scanGiteaPullRequests } from './pull-request-scan-cache' import { getHostedReviewLocalGitOptions, type HostedReviewExecutionOptions } from '../source-control/hosted-review-git-options' const REQUEST_TIMEOUT_MS = 5000 +// Why: self-hosted Forgejo can take ~5s to serve one /pulls page (it loads +// reviewer data per PR). The default 5s cap aborted responses right as they +// completed, so the work was discarded and retried on the next refresh (#8807). +const PULL_REQUEST_LIST_TIMEOUT_MS = 15_000 const PULL_REQUEST_PAGE_LIMIT = 50 const MAX_PULL_REQUEST_PAGES = 5 @@ -104,6 +109,16 @@ function encodedRepoPath(repo: GiteaRepoRef): string { return `${encodeURIComponent(repo.owner)}/${encodeURIComponent(repo.repo)}` } +function giteaPullRequestScanKey(repo: GiteaRepoRef): string { + return `${configuredApiBaseUrl(repo)}/${encodedRepoPath(repo)}` +} + +/** Invalidate the shared /pulls scan after Orca itself creates a PR so the + * next worktree-card refresh sees it instead of a cached miss. */ +export function invalidateGiteaPullRequestScanForRepo(repo: GiteaRepoRef): void { + invalidateGiteaPullRequestScan(giteaPullRequestScanKey(repo)) +} + async function getCommitStatus( repo: GiteaRepoRef, headSha: string | undefined @@ -227,26 +242,24 @@ export async function getGiteaPullRequestForBranch( } if (branchName) { - for (let page = 1; page <= MAX_PULL_REQUEST_PAGES; page++) { - const list = await requestJson( - repo, - `/repos/${encodedRepoPath(repo)}/pulls`, - { + const pullRequests = await scanGiteaPullRequests( + giteaPullRequestScanKey(repo), + (page) => + requestJson(repo, `/repos/${encodedRepoPath(repo)}/pulls`, { searchParams: { state: 'all', sort: 'recentupdate', page, limit: PULL_REQUEST_PAGE_LIMIT - } - } - ) - const raw = list?.find((item) => matchesBranch(item, branchName)) - if (raw) { - return normalizePullRequest(repo, raw) - } - if (!list || list.length < PULL_REQUEST_PAGE_LIMIT) { - break - } + }, + timeoutMs: PULL_REQUEST_LIST_TIMEOUT_MS + }), + PULL_REQUEST_PAGE_LIMIT, + MAX_PULL_REQUEST_PAGES + ) + const raw = pullRequests.find((item) => matchesBranch(item, branchName)) + if (raw) { + return normalizePullRequest(repo, raw) } } diff --git a/src/main/gitea/pull-request-creation.ts b/src/main/gitea/pull-request-creation.ts index b113b1783ce..3fdef7daec1 100644 --- a/src/main/gitea/pull-request-creation.ts +++ b/src/main/gitea/pull-request-creation.ts @@ -8,7 +8,7 @@ import { requestHostedReviewJson } from '../source-control/hosted-review-api-request' import { readHostedPullRequestTemplate } from '../source-control/pull-request-template' -import { getGiteaPullRequestForBranch } from './client' +import { getGiteaPullRequestForBranch, invalidateGiteaPullRequestScanForRepo } from './client' import { mapGiteaPullRequest, type RawGiteaPullRequest } from './pull-request-mappers' import { getGiteaRepoRef, type GiteaRepoRef } from './repository-ref' @@ -105,6 +105,12 @@ async function findExistingPullRequest( head: string, connectionId?: string | null ): Promise<{ number: number; url: string } | null> { + // Why: only called after a create attempt, which may have just mutated the + // remote — a cached /pulls scan from before the POST would miss the new PR. + const repo = await getGiteaRepoRef(repoPath, connectionId) + if (repo) { + invalidateGiteaPullRequestScanForRepo(repo) + } const existing = await getGiteaPullRequestForBranch(repoPath, head, null, connectionId) return existing ? { number: existing.number, url: existing.url } : null } @@ -177,6 +183,7 @@ export async function createGiteaPullRequest( ) const created = mapGiteaPullRequest(raw, 'neutral') if (created) { + invalidateGiteaPullRequestScanForRepo(repo) return { ok: true, number: created.number, url: created.url } } const found = await findExistingPullRequest(repoPath, head, connectionId).catch(() => null) diff --git a/src/main/gitea/pull-request-scan-cache.ts b/src/main/gitea/pull-request-scan-cache.ts new file mode 100644 index 00000000000..8864f087d6d --- /dev/null +++ b/src/main/gitea/pull-request-scan-cache.ts @@ -0,0 +1,161 @@ +import type { RawGiteaPullRequest } from './pull-request-mappers' + +export type GiteaPullRequestPageFetcher = (page: number) => Promise + +type GiteaPullRequestScanEntry = { + expiresAt: number + expirationTimer: ReturnType + pullRequests: RawGiteaPullRequest[] +} + +// Why: long enough to absorb a push-event burst that refreshes every worktree +// card at once, short enough that a PR opened outside Orca shows up promptly. +const SCAN_TTL_MS = 30_000 +// Why: a short failure cooldown still coalesces rapid card retries without +// turning a transient outage into a 30-second authoritative "no PR" result. +const FAILED_SCAN_RETRY_MS = 3_000 +// Why: each entry can retain hundreds of full PR payloads, so TTL alone is not +// enough protection when many repositories are opened during one app session. +const MAX_SCAN_CACHE_ENTRIES = 32 + +const scanCache = new Map() +const inFlightScans = new Map>() +// Why: an invalidation (PR just created) must also defeat a scan already in +// flight — otherwise that scan finishes afterwards and re-caches a listing +// from before the mutation, hiding the new PR for a full TTL. +const scanGenerations = new Map() +const activeScanCounts = new Map() + +function removeScanCacheEntry(repoKey: string, expected?: GiteaPullRequestScanEntry): void { + const entry = scanCache.get(repoKey) + if (!entry || (expected && entry !== expected)) { + return + } + clearTimeout(entry.expirationTimer) + scanCache.delete(repoKey) +} + +function rememberScanCacheEntry( + repoKey: string, + pullRequests: RawGiteaPullRequest[], + ttlMs: number +): void { + removeScanCacheEntry(repoKey) + let entry!: GiteaPullRequestScanEntry + const expirationTimer = setTimeout(() => removeScanCacheEntry(repoKey, entry), ttlMs) + expirationTimer.unref() + entry = { + expiresAt: Date.now() + ttlMs, + expirationTimer, + pullRequests + } + scanCache.set(repoKey, entry) + while (scanCache.size > MAX_SCAN_CACHE_ENTRIES) { + const oldestKey = scanCache.keys().next().value + if (oldestKey === undefined) { + break + } + removeScanCacheEntry(oldestKey) + } +} + +function reusableScanCacheEntry(repoKey: string): GiteaPullRequestScanEntry | null { + const entry = scanCache.get(repoKey) + if (!entry) { + return null + } + if (Date.now() >= entry.expiresAt) { + removeScanCacheEntry(repoKey, entry) + return null + } + // Keep the cap useful for users actively switching among several repositories. + scanCache.delete(repoKey) + scanCache.set(repoKey, entry) + return entry +} + +/** + * Why: every worktree card resolves its branch by paginating the same + * /repos/{repo}/pulls listing — Gitea/Forgejo have no head-branch filter. + * Self-hosted forges serve that endpoint slowly, and a push event refreshes + * all cards at once, so per-card scans multiplied one page walk into hundreds + * of requests and OOM-killed a small Forgejo pod (#8807). All concurrent + * callers share one in-flight scan per repo, and the result is cached briefly + * so a burst costs a single page walk. + */ +export async function scanGiteaPullRequests( + repoKey: string, + fetchPage: GiteaPullRequestPageFetcher, + pageLimit: number, + maxPages: number +): Promise { + const cached = reusableScanCacheEntry(repoKey) + if (cached) { + return cached.pullRequests + } + const running = inFlightScans.get(repoKey) + if (running) { + return running + } + const generation = scanGenerations.get(repoKey) ?? 0 + activeScanCounts.set(repoKey, (activeScanCounts.get(repoKey) ?? 0) + 1) + const scan = (async () => { + const pullRequests: RawGiteaPullRequest[] = [] + let completed = true + for (let page = 1; page <= maxPages; page++) { + const list = await fetchPage(page) + if (!list) { + completed = false + break + } + pullRequests.push(...list) + if (list.length < pageLimit) { + break + } + } + if ((scanGenerations.get(repoKey) ?? 0) === generation) { + rememberScanCacheEntry(repoKey, pullRequests, completed ? SCAN_TTL_MS : FAILED_SCAN_RETRY_MS) + } + return pullRequests + })() + inFlightScans.set(repoKey, scan) + try { + return await scan + } finally { + if (inFlightScans.get(repoKey) === scan) { + inFlightScans.delete(repoKey) + } + const activeScans = (activeScanCounts.get(repoKey) ?? 1) - 1 + if (activeScans > 0) { + activeScanCounts.set(repoKey, activeScans) + } else { + activeScanCounts.delete(repoKey) + scanGenerations.delete(repoKey) + } + } +} + +/** Drop the cached scan after a mutation Orca itself performed (PR create), + * so the next card refresh sees the new PR instead of a stale miss. */ +export function invalidateGiteaPullRequestScan(repoKey: string): void { + removeScanCacheEntry(repoKey) + inFlightScans.delete(repoKey) + if ((activeScanCounts.get(repoKey) ?? 0) > 0) { + scanGenerations.set(repoKey, (scanGenerations.get(repoKey) ?? 0) + 1) + } else { + scanGenerations.delete(repoKey) + } +} + +export function _resetGiteaPullRequestScanCache(): void { + for (const repoKey of scanCache.keys()) { + removeScanCacheEntry(repoKey) + } + inFlightScans.clear() + scanGenerations.clear() + activeScanCounts.clear() +} + +export function _getGiteaPullRequestScanCacheSize(): number { + return scanCache.size +}