starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-10-18 08:03:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

Scope remaining GitHub rate-limit accounting

Browse Source
This commit is contained in:
Jinjing
2026-07-20 12:35:26 -07:00
parent 00835e0720
commit 5a75fce1be
5 changed files with 192 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/github/client-pr-checks.test.ts
+17 -10
View File
@@ -8,8 +8,6 @@ const {
gitExecFileAsyncMock,
extractExecErrorMock,
getRateLimitMock,
rateLimitGuardMock,
noteRateLimitSpendMock,
repositoryRateLimitGuardMock,
noteRepositoryRateLimitSpendMock,
spendsSharedGitHubComQuotaMock,
@@ -32,11 +30,11 @@ const {
return { stderr: String(err), stdout: '' }
}),
getRateLimitMock: vi.fn(),
rateLimitGuardMock: vi.fn(() => ({ blocked: false })),
noteRateLimitSpendMock: vi.fn(),
repositoryRateLimitGuardMock: vi.fn(() => ({ blocked: false })),
noteRepositoryRateLimitSpendMock: vi.fn(),
spendsSharedGitHubComQuotaMock: vi.fn(() => true),
spendsSharedGitHubComQuotaMock: vi.fn<
(repository?: { host?: string } | null, options?: { wslDistro?: string }) => boolean
>(() => true),
acquireMock: vi.fn(),
releaseMock: vi.fn()
}))
@@ -71,8 +69,6 @@ vi.mock('../git/runner', () => ({
vi.mock('./rate-limit', () => ({
getRateLimit: getRateLimitMock,
rateLimitGuard: rateLimitGuardMock,
noteRateLimitSpend: noteRateLimitSpendMock,
repositoryRateLimitGuard: repositoryRateLimitGuardMock,
noteRepositoryRateLimitSpend: noteRepositoryRateLimitSpendMock,
spendsSharedGitHubComQuota: spendsSharedGitHubComQuotaMock
@@ -211,9 +207,14 @@ describe('getPRChecks', () => {
extractExecErrorMock.mockClear()
getRateLimitMock.mockReset()
getRateLimitMock.mockResolvedValue({ resources: {} })
rateLimitGuardMock.mockReset()
rateLimitGuardMock.mockReturnValue({ blocked: false })
noteRateLimitSpendMock.mockReset()
repositoryRateLimitGuardMock.mockReset()
repositoryRateLimitGuardMock.mockReturnValue({ blocked: false })
noteRepositoryRateLimitSpendMock.mockReset()
spendsSharedGitHubComQuotaMock.mockReset()
spendsSharedGitHubComQuotaMock.mockImplementation(
(repository?: { host?: string } | null, options?: { wslDistro?: string }) =>
(!repository?.host || repository.host.toLowerCase() === 'github.com') && !options?.wslDistro
)
acquireMock.mockReset()
releaseMock.mockReset()
acquireMock.mockResolvedValue(undefined)
@@ -602,6 +603,12 @@ describe('getPRChecks', () => {
env: expect.objectContaining({ GH_PROMPT_DISABLED: '1' })
})
)
expect(getRateLimitMock).not.toHaveBeenCalled()
expect(repositoryRateLimitGuardMock).toHaveBeenCalledWith(
{ owner: 'acme', repo: 'widgets', host: 'github.com' },
'graphql',
expect.objectContaining({ cwd: '/repo-root', wslDistro: 'Ubuntu', host: 'github.com' })
)
})
it('uses explicit PR repo for rollup and gh pr checks fallback', async () => {
src/main/github/client.ts
+3 -5
View File
@@ -117,7 +117,6 @@ import { mapGraphQLReactionGroups, type GitHubGraphQLReactionGroup } from './com
import {
getRateLimit,
noteRepositoryRateLimitSpend,
rateLimitGuard,
repositoryRateLimitGuard,
spendsSharedGitHubComQuota,
type RateLimitBucketKind
@@ -189,11 +188,10 @@ async function assertRateLimitBudget(
repository?: GitHubApiRepository | null,
executionOptions?: Pick<GhExecOptions, 'wslDistro'>
): Promise<void> {
if (!spendsSharedGitHubComQuota(repository, executionOptions)) {
return
if (spendsSharedGitHubComQuota(repository, executionOptions)) {
await getRateLimit()
}
await getRateLimit()
const guard = rateLimitGuard(bucket)
const guard = repositoryRateLimitGuard(repository, bucket, executionOptions)
if (guard.blocked) {
throw new Error(
`GitHub ${bucket} rate limit is low; retry after ${new Date(guard.resetAt * 1000).toLocaleTimeString()}`
src/main/github/pr-refresh-coordinator.test.ts
+80 -7
View File
@@ -8,15 +8,21 @@ const {
sendToTrustedUIRendererMock,
getAllWebContentsMock,
getPRForBranchOutcomeMock,
getOriginGitHubApiRepositoryMock,
getRateLimitMock,
rateLimitGuardMock
noteRepositoryRateLimitSpendMock,
repositoryRateLimitGuardMock,
spendsSharedGitHubComQuotaMock
} = vi.hoisted(() => ({
sendMock: vi.fn(),
sendToTrustedUIRendererMock: vi.fn(),
getAllWebContentsMock: vi.fn(),
getPRForBranchOutcomeMock: vi.fn(),
getOriginGitHubApiRepositoryMock: vi.fn(),
getRateLimitMock: vi.fn(),
rateLimitGuardMock: vi.fn()
noteRepositoryRateLimitSpendMock: vi.fn(),
repositoryRateLimitGuardMock: vi.fn(),
spendsSharedGitHubComQuotaMock: vi.fn()
}))
vi.mock('electron', () => ({
@@ -29,10 +35,15 @@ vi.mock('./client', () => ({
getPRForBranchOutcome: getPRForBranchOutcomeMock
}))
vi.mock('./github-api-repository', () => ({
getOriginGitHubApiRepository: getOriginGitHubApiRepositoryMock
}))
vi.mock('./rate-limit', () => ({
getRateLimit: getRateLimitMock,
noteRateLimitSpend: vi.fn(),
rateLimitGuard: rateLimitGuardMock
noteRepositoryRateLimitSpend: noteRepositoryRateLimitSpendMock,
repositoryRateLimitGuard: repositoryRateLimitGuardMock,
spendsSharedGitHubComQuota: spendsSharedGitHubComQuotaMock
}))
vi.mock('../ipc/ui', () => ({
@@ -94,9 +105,21 @@ describe('pr-refresh-coordinator', () => {
})
getAllWebContentsMock.mockReset()
getPRForBranchOutcomeMock.mockReset()
getOriginGitHubApiRepositoryMock.mockReset()
getOriginGitHubApiRepositoryMock.mockResolvedValue({
owner: 'acme',
repo: 'widgets',
host: 'github.com'
})
getRateLimitMock.mockReset()
rateLimitGuardMock.mockReset()
rateLimitGuardMock.mockReturnValue({ blocked: false })
noteRepositoryRateLimitSpendMock.mockReset()
repositoryRateLimitGuardMock.mockReset()
repositoryRateLimitGuardMock.mockReturnValue({ blocked: false })
spendsSharedGitHubComQuotaMock.mockReset()
spendsSharedGitHubComQuotaMock.mockImplementation(
(repository: { host?: string } | null, options?: { wslDistro?: string }) =>
(!repository?.host || repository.host.toLowerCase() === 'github.com') && !options?.wslDistro
)
getAllWebContentsMock.mockReturnValue([
{
id: 1,
@@ -569,6 +592,56 @@ describe('pr-refresh-coordinator', () => {
expect(pausedEvents).toHaveLength(0)
})
it.each([
{
scope: 'GitHub Enterprise',
repository: { owner: 'acme', repo: 'widgets', host: 'github.acme-corp.com' },
localGitOptions: undefined
},
{
scope: 'WSL',
repository: { owner: 'acme', repo: 'widgets', host: 'github.com' },
localGitOptions: { wslDistro: 'Ubuntu' }
}
])('bypasses the shared budget for $scope background refreshes', async (testCase) => {
const { enqueuePRRefresh } = await import('./pr-refresh-coordinator')
getOriginGitHubApiRepositoryMock.mockResolvedValue(testCase.repository)
getPRForBranchOutcomeMock.mockResolvedValue({
kind: 'no-pr',
fetchedAt: Date.now()
})
const executionOptions = testCase.localGitOptions ?? {}
enqueuePRRefresh(makeCandidate({ localGitOptions: testCase.localGitOptions }), 'active', 80, 1)
await vi.advanceTimersByTimeAsync(0)
expect(getRateLimitMock).not.toHaveBeenCalled()
expect(repositoryRateLimitGuardMock).toHaveBeenCalledWith(
testCase.repository,
'core',
executionOptions
)
expect(repositoryRateLimitGuardMock).toHaveBeenCalledWith(
testCase.repository,
'graphql',
executionOptions
)
expect(noteRepositoryRateLimitSpendMock).toHaveBeenCalledWith(
testCase.repository,
'core',
1,
executionOptions
)
expect(noteRepositoryRateLimitSpendMock).toHaveBeenCalledWith(
testCase.repository,
'graphql',
1,
executionOptions
)
expect(getPRForBranchOutcomeMock).toHaveBeenCalledTimes(1)
})
it('does not consume active burst slots for rate-limit pauses', async () => {
const { enqueuePRRefresh } = await import('./pr-refresh-coordinator')
// Why: keyed on drained items (one getRateLimit call each) rather than
@@ -579,7 +652,7 @@ describe('pr-refresh-coordinator', () => {
drainedItems += 1
return { ok: true }
})
rateLimitGuardMock.mockImplementation(() =>
repositoryRateLimitGuardMock.mockImplementation(() =>
drainedItems <= 3
? { blocked: true, remaining: 0, limit: 5000, resetAt: 61 }
: { blocked: false }
src/main/github/pr-refresh-coordinator.ts
+30 -6
View File
@@ -9,7 +9,14 @@ import type {
PRRefreshOutcome
} from '../../shared/types'
import { getPRForBranchOutcome, type GitHubPRBranchLookupOptions } from './client'
import { getRateLimit, noteRateLimitSpend, rateLimitGuard } from './rate-limit'
import { getOriginGitHubApiRepository } from './github-api-repository'
import { ghRepoExecOptions, githubRepoContext } from './gh-utils'
import {
getRateLimit,
noteRepositoryRateLimitSpend,
repositoryRateLimitGuard,
spendsSharedGitHubComQuota
} from './rate-limit'
import { recordCoalescedCrashBreadcrumb } from '../crash-reporting/crash-breadcrumb-store'
import { sendToTrustedUIRenderer } from '../ipc/ui'
@@ -728,11 +735,20 @@ async function drainQueue(): Promise<void> {
)
if (isBackground(next.reason)) {
// Why: the probe only warms rateLimitGuard's cache, so it must fail open — GHES with rate limiting disabled 404s every probe (#7553).
await getRateLimit()
const executionOptions = next.candidate.localGitOptions ?? {}
const repository = await getOriginGitHubApiRepository(
next.candidate.repoPath,
next.candidate.connectionId,
executionOptions
)
// Why: only native github.com uses the singleton snapshot; scoped breakers protect GHES and WSL.
if (spendsSharedGitHubComQuota(repository, executionOptions)) {
// Why: the probe only warms the cache, so failures must fail open (#7553).
await getRateLimit()
}
const buckets = backgroundRefreshBuckets()
const blockedGuard = buckets
.map((bucket) => rateLimitGuard(bucket))
.map((bucket) => repositoryRateLimitGuard(repository, bucket, executionOptions))
.find((guard) => guard.blocked)
if (blockedGuard?.blocked) {
const retryAt = blockedGuard.resetAt * 1000
@@ -755,7 +771,7 @@ async function drainQueue(): Promise<void> {
noteActiveStart(next)
}
for (const bucket of buckets) {
noteRateLimitSpend(bucket)
noteRepositoryRateLimitSpend(repository, bucket, 1, executionOptions)
}
}
@@ -922,8 +938,16 @@ export async function refreshPRNow(candidate: GitHubPRRefreshCandidate): Promise
}
// Why: enforce the rate-limit gate so a stale renderer can't bypass it; refuse without spending quota until the later of the two cooldowns.
const manualExecutionOptions = ghRepoExecOptions(
githubRepoContext(candidate.repoPath, candidate.connectionId, candidate.localGitOptions)
)
const manualRepository = await getOriginGitHubApiRepository(
candidate.repoPath,
candidate.connectionId,
manualExecutionOptions
)
const manualBlockedGuard = backgroundRefreshBuckets()
.map((bucket) => rateLimitGuard(bucket))
.map((bucket) => repositoryRateLimitGuard(manualRepository, bucket, manualExecutionOptions))
.find((guard) => guard.blocked)
const secondaryGateUntil = manualRetryGates.get(key)
const gateUntil = Math.max(
src/main/github/work-item-details.test.ts
+62 -26
View File
@@ -14,8 +14,6 @@ const {
getWorkItemByOwnerRepoMock,
getPRChecksMock,
getPRCommentsMock,
rateLimitGuardMock,
noteRateLimitSpendMock,
repositoryRateLimitGuardMock,
noteRepositoryRateLimitSpendMock,
ghRepoExecOptionsMock,
@@ -32,8 +30,6 @@ const {
getWorkItemByOwnerRepoMock: vi.fn(),
getPRChecksMock: vi.fn(),
getPRCommentsMock: vi.fn(),
rateLimitGuardMock: vi.fn<() => RateLimitGuardResult>(() => ({ blocked: false })),
noteRateLimitSpendMock: vi.fn(),
repositoryRateLimitGuardMock: vi.fn<() => RateLimitGuardResult>(() => ({ blocked: false })),
noteRepositoryRateLimitSpendMock: vi.fn(),
ghRepoExecOptionsMock: vi.fn((context) =>
@@ -73,8 +69,6 @@ vi.mock('./github-enterprise-repository', () => ({
}))
vi.mock('./rate-limit', () => ({
rateLimitGuard: rateLimitGuardMock,
noteRateLimitSpend: noteRateLimitSpendMock,
repositoryRateLimitGuard: repositoryRateLimitGuardMock,
noteRepositoryRateLimitSpend: noteRepositoryRateLimitSpendMock
}))
@@ -102,9 +96,6 @@ describe('getWorkItemDetails', () => {
getWorkItemByOwnerRepoMock.mockReset()
getPRChecksMock.mockReset()
getPRCommentsMock.mockReset()
rateLimitGuardMock.mockReset()
rateLimitGuardMock.mockReturnValue({ blocked: false })
noteRateLimitSpendMock.mockReset()
repositoryRateLimitGuardMock.mockReset()
repositoryRateLimitGuardMock.mockReturnValue({ blocked: false })
noteRepositoryRateLimitSpendMock.mockReset()
@@ -248,6 +239,68 @@ describe('getWorkItemDetails', () => {
}
])
expect(details?.participants?.[0]?.login).toBe('issue-author')
expect(repositoryRateLimitGuardMock).toHaveBeenCalledWith(
{ owner: 'acme', repo: 'widgets' },
'graphql',
{}
)
expect(noteRepositoryRateLimitSpendMock).toHaveBeenCalledWith(
{ owner: 'acme', repo: 'widgets' },
'graphql',
1,
{}
)
})
it('scopes collapsed issue GraphQL accounting to the selected WSL runtime', async () => {
const repository = { owner: 'acme', repo: 'widgets' }
const localGitOptions = { wslDistro: 'Ubuntu' }
getWorkItemMock.mockResolvedValueOnce({
id: 'issue:923',
type: 'issue',
number: 923,
title: 'Use upstream issues',
state: 'open',
url: 'https://github.com/acme/widgets/issues/923',
labels: [],
updatedAt: '2026-04-01T00:00:00Z',
author: 'issue-author'
})
getIssueOwnerRepoMock.mockResolvedValue(repository)
ghExecFileAsyncMock
.mockResolvedValueOnce({
stdout: JSON.stringify({
data: {
repository: {
issue: {
body: 'WSL issue body',
assignees: { nodes: [] },
participants: { nodes: [] },
comments: { nodes: [] }
}
}
}
})
})
.mockResolvedValueOnce({ stdout: '' })
const details = await getWorkItemDetails('/repo-root', 923, 'issue', null, localGitOptions)
expect(details?.body).toBe('WSL issue body')
expect(repositoryRateLimitGuardMock).toHaveBeenCalledWith(
repository,
'graphql',
localGitOptions
)
expect(noteRepositoryRateLimitSpendMock).toHaveBeenCalledWith(
repository,
'graphql',
1,
localGitOptions
)
expect(ghExecFileAsyncMock.mock.calls.every((call) => call[1]?.wslDistro === 'Ubuntu')).toBe(
true
)
})
it('enriches a non-participating assignee avatar from the GraphQL assignees connection', async () => {
@@ -432,14 +485,6 @@ describe('getWorkItemDetails', () => {
})
it('skips optional GraphQL issue detail calls when the cached GraphQL budget is low', async () => {
// github.com repos share the singleton quota, so both the raw and
// repository-scoped guards report the same low-budget block.
rateLimitGuardMock.mockReturnValue({
blocked: true,
remaining: 3,
limit: 5000,
resetAt: 1_800_000_000
})
repositoryRateLimitGuardMock.mockReturnValue({
blocked: true,
remaining: 3,
@@ -467,7 +512,6 @@ describe('getWorkItemDetails', () => {
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(3)
expect(ghExecFileAsyncMock.mock.calls.some((call) => call[0][1] === 'graphql')).toBe(false)
expect(noteRateLimitSpendMock).not.toHaveBeenCalled()
expect(noteRepositoryRateLimitSpendMock).not.toHaveBeenCalled()
expect(details?.body).toBe('Issue body')
expect(details?.participants).toEqual([])
@@ -588,12 +632,6 @@ describe('getWorkItemDetails', () => {
it('routes local WSL PR detail fan-out through the selected distro', async () => {
const localGitOptions = { wslDistro: 'Ubuntu' }
rateLimitGuardMock.mockReturnValue({
blocked: true,
remaining: 0,
limit: 5000,
resetAt: 1_800_000_000
})
getWorkItemMock.mockResolvedValueOnce({
id: 'pr:42',
type: 'pr',
@@ -671,8 +709,6 @@ describe('getWorkItemDetails', () => {
expect(ghExecFileAsyncMock.mock.calls.every((call) => call[1]?.wslDistro === 'Ubuntu')).toBe(
true
)
expect(rateLimitGuardMock).not.toHaveBeenCalled()
expect(noteRateLimitSpendMock).not.toHaveBeenCalled()
})
it('uses the GitHub Enterprise host for SSH-backed PR work item details', async () => {