mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
fix(gitea): share and cache the /pulls scan so card refreshes can't hammer a self-hosted forge (#8858)
* 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * fix(gitea): bound PR scan cache and retry failures Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Jinwoo-H <jinwoo0825@gmail.com> Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
co-authored by
Claude Fable 5
Orca
Jinwoo-H
parent
ed2135d248
commit
fb5bc4fd24
@@ -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<void>((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) => {
|
||||
|
||||
+28
-15
@@ -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<RawGiteaPullRequest[]>(
|
||||
repo,
|
||||
`/repos/${encodedRepoPath(repo)}/pulls`,
|
||||
{
|
||||
const pullRequests = await scanGiteaPullRequests(
|
||||
giteaPullRequestScanKey(repo),
|
||||
(page) =>
|
||||
requestJson<RawGiteaPullRequest[]>(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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -0,0 +1,161 @@
|
||||
import type { RawGiteaPullRequest } from './pull-request-mappers'
|
||||
|
||||
export type GiteaPullRequestPageFetcher = (page: number) => Promise<RawGiteaPullRequest[] | null>
|
||||
|
||||
type GiteaPullRequestScanEntry = {
|
||||
expiresAt: number
|
||||
expirationTimer: ReturnType<typeof setTimeout>
|
||||
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<string, GiteaPullRequestScanEntry>()
|
||||
const inFlightScans = new Map<string, Promise<RawGiteaPullRequest[]>>()
|
||||
// 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<string, number>()
|
||||
const activeScanCounts = new Map<string, number>()
|
||||
|
||||
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<RawGiteaPullRequest[]> {
|
||||
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
|
||||
}
|
||||
Reference in New Issue
Block a user