Tier GitHub PR lookup polling to prevent quota exhaustion (#12013)
* Tier GitHub PR lookup polling to prevent quota exhaustion The selected worktree (O(1)) checks per-minute; card list (O(N)) per-15-minutes. Introduce process-wide cache to collapse concurrent polling and gate lookups on available rate-limit budget with exponential backoff on failure. - Preserve last-known review during backoff - Invalidate cache when Orca opens a PR - Stop coordinator from double-charging * Tier GitHub PR lookup polling to prevent quota exhaustion - Return the latest reset time when both GitHub API buckets are rate-limited, preventing premature retries against still-blocked buckets. - Serve the last known review on transient lookup failures, preventing reviews from blinking out on temporary errors. - Discard in-flight lookups that predate an invalidation so stale answers cannot overwrite newly opened reviews. * fix: give rate-limit reset tests unique titles oxlint vitest/no-identical-title was failing static analysis because two cases shared the same describe title.
This commit is contained in:
Jinjing
GitHub
parent
a7d769e13d
commit
2f104d8713
|
|
@ -141,7 +141,10 @@ export async function fetchHostedReviewForBranch(
|
|||
{
|
||||
repo: mobileRepoSelectorFromWorktreeId(worktreeId),
|
||||
branch: args.branch,
|
||||
linkedGitHubPR: args.linkedGitHubPR ?? null
|
||||
linkedGitHubPR: args.linkedGitHubPR ?? null,
|
||||
// Why: the mobile PR sidebar is only ever open on the selected worktree,
|
||||
// so it belongs in the host's fast re-check tier (#11532).
|
||||
active: true
|
||||
},
|
||||
readForBranch
|
||||
)
|
||||
|
|
|
|||
|
|
@ -170,6 +170,7 @@ import {
|
|||
getPRComments,
|
||||
getPRForBranch,
|
||||
getPRForBranchOutcome,
|
||||
getGitHubPRLookupRateLimitBlock,
|
||||
getRepoSlug,
|
||||
getRepoUpstream,
|
||||
getWorkItem,
|
||||
|
|
@ -4609,3 +4610,70 @@ describe('GitHub GraphQL rate-limit guard', () => {
|
|||
expect(noteRateLimitSpendMock).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('getGitHubPRLookupRateLimitBlock', () => {
|
||||
beforeEach(() => {
|
||||
execFileAsyncMock.mockReset()
|
||||
ghExecFileAsyncMock.mockReset()
|
||||
getOwnerRepoMock.mockReset()
|
||||
getOwnerRepoMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' })
|
||||
getRemoteUrlForRepoMock.mockReset()
|
||||
gitExecFileAsyncMock.mockReset()
|
||||
getRateLimitMock.mockReset()
|
||||
getRateLimitMock.mockResolvedValue(undefined)
|
||||
rateLimitGuardMock.mockReset()
|
||||
rateLimitGuardMock.mockReturnValue({ blocked: false })
|
||||
noteRateLimitSpendMock.mockReset()
|
||||
_resetOwnerRepoCache()
|
||||
})
|
||||
|
||||
it('reports no block while every lookup bucket has budget', async () => {
|
||||
await expect(getGitHubPRLookupRateLimitBlock('/repo-root')).resolves.toBeNull()
|
||||
expect(getRateLimitMock).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('reports a block when either lookup bucket is exhausted', async () => {
|
||||
rateLimitGuardMock.mockImplementation(((bucket: string) =>
|
||||
bucket === 'graphql'
|
||||
? { blocked: true, remaining: 4, limit: 5000, resetAt: 1_800_000_000 }
|
||||
: { blocked: false }) as () => RateLimitGuardResult)
|
||||
|
||||
await expect(getGitHubPRLookupRateLimitBlock('/repo-root')).resolves.toEqual({
|
||||
resetAt: 1_800_000_000
|
||||
})
|
||||
})
|
||||
|
||||
it('reports the latest reset when both lookup buckets are exhausted', async () => {
|
||||
rateLimitGuardMock.mockImplementation(((bucket: string) => ({
|
||||
blocked: true,
|
||||
remaining: 4,
|
||||
limit: 5000,
|
||||
// Why: core resets first, so returning it would retry into graphql's block.
|
||||
resetAt: bucket === 'core' ? 1_800_000_000 : 1_800_003_600
|
||||
})) as () => RateLimitGuardResult)
|
||||
|
||||
await expect(getGitHubPRLookupRateLimitBlock('/repo-root')).resolves.toEqual({
|
||||
resetAt: 1_800_003_600
|
||||
})
|
||||
})
|
||||
|
||||
it('reports the later reset when graphql outlasts core', async () => {
|
||||
// Retrying at the earlier reset would fail again on the bucket still blocked.
|
||||
rateLimitGuardMock.mockImplementation(((bucket: string) => ({
|
||||
blocked: true,
|
||||
remaining: 0,
|
||||
limit: 5000,
|
||||
resetAt: bucket === 'graphql' ? 1_800_000_600 : 1_800_000_000
|
||||
})) as () => RateLimitGuardResult)
|
||||
|
||||
await expect(getGitHubPRLookupRateLimitBlock('/repo-root')).resolves.toEqual({
|
||||
resetAt: 1_800_000_600
|
||||
})
|
||||
})
|
||||
|
||||
it('fails open when the exempt rate-limit probe itself fails', async () => {
|
||||
getRateLimitMock.mockRejectedValue(new Error('probe offline'))
|
||||
|
||||
await expect(getGitHubPRLookupRateLimitBlock('/repo-root')).resolves.toBeNull()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -195,6 +195,52 @@ async function assertRateLimitBudget(
|
|||
}
|
||||
}
|
||||
|
||||
// Why: a branch lookup prefers REST but can fall back to `gh pr list` and
|
||||
// `gh pr view`, so both buckets are guarded and charged. Mirrors the PR refresh
|
||||
// coordinator's own estimate.
|
||||
const PR_BRANCH_LOOKUP_BUCKETS = ['core', 'graphql'] as const
|
||||
|
||||
/**
|
||||
* Rate-limit floor for GitHub PR lookups that do not run through the PR refresh
|
||||
* coordinator's queue (#11532).
|
||||
*
|
||||
* The coordinator guards and paces its own background refreshes, but
|
||||
* `hostedReview:forBranch` polls the same lookup straight from the renderer.
|
||||
* Ungated, the two paths together could spend the user's entire hourly quota —
|
||||
* which is per user and shared with their own `gh` and CLI agents.
|
||||
* Returns the reset time when the caller must not spend, else `null`.
|
||||
*/
|
||||
export async function getGitHubPRLookupRateLimitBlock(
|
||||
repoPath: string,
|
||||
connectionId?: string | null,
|
||||
localGitOptions: LocalGitExecOptions = {}
|
||||
): Promise<{ resetAt: number } | null> {
|
||||
const executionOptions = ghRepoExecOptions(
|
||||
githubRepoContext(repoPath, connectionId, localGitOptions)
|
||||
)
|
||||
// Why: identity resolution runs local git, which can fail for reasons that
|
||||
// have nothing to do with the budget; let the lookup itself classify those.
|
||||
const repository = await getOriginGitHubApiRepository(
|
||||
repoPath,
|
||||
connectionId,
|
||||
executionOptions
|
||||
).catch(() => null)
|
||||
if (repository === null) {
|
||||
return null
|
||||
}
|
||||
if (spendsSharedGitHubComQuota(repository, executionOptions)) {
|
||||
// Why: the probe only warms the snapshot and is exempt from limits, so a
|
||||
// failure must fail open rather than block the lookup (#7553).
|
||||
await getRateLimit().catch(() => undefined)
|
||||
}
|
||||
// Why: retrying at the earlier reset would fail again on the bucket that has
|
||||
// not reset yet, so the latest blocked reset is the only honest retry time.
|
||||
const resets = PR_BRANCH_LOOKUP_BUCKETS.map((bucket) =>
|
||||
repositoryRateLimitGuard(repository, bucket, executionOptions)
|
||||
).flatMap((guard) => (guard.blocked ? [guard.resetAt] : []))
|
||||
return resets.length > 0 ? { resetAt: Math.max(...resets) } : null
|
||||
}
|
||||
|
||||
function prRefreshUpstreamError(
|
||||
err: unknown
|
||||
): Extract<PRRefreshOutcome, { kind: 'upstream-error' }> {
|
||||
|
|
@ -2956,6 +3002,13 @@ export async function getPRForBranchOutcome(
|
|||
if (connectionId && candidates.length === 0) {
|
||||
return { kind: 'no-pr', fetchedAt: Date.now() }
|
||||
}
|
||||
// Why (#11532): account every lookup, not just the coordinator's queue —
|
||||
// `hostedReview:forBranch` reaches this directly from renderer polling and
|
||||
// was spending the shared quota invisibly. headRepo is `origin`, the same
|
||||
// identity the coordinator guards on.
|
||||
for (const bucket of PR_BRANCH_LOOKUP_BUCKETS) {
|
||||
noteRepositoryRateLimitSpend(headRepo ?? candidates[0], bucket, 1, ghOptions)
|
||||
}
|
||||
let data: PullRequestLookupData | null = null
|
||||
let dataRepo: OwnerRepo | null = null
|
||||
let dataHeadRepo: OwnerRepo | null = headRepo
|
||||
|
|
|
|||
|
|
@ -32,6 +32,7 @@ const {
|
|||
getRateLimitMock,
|
||||
rateLimitGuardMock,
|
||||
noteRateLimitSpendMock,
|
||||
noteRepositoryRateLimitSpendMock,
|
||||
ghRepoExecOptionsMock,
|
||||
githubRepoContextMock,
|
||||
getSshGitProviderMock,
|
||||
|
|
@ -52,6 +53,7 @@ const {
|
|||
blocked: false
|
||||
})),
|
||||
noteRateLimitSpendMock: vi.fn(),
|
||||
noteRepositoryRateLimitSpendMock: vi.fn(),
|
||||
ghRepoExecOptionsMock: vi.fn((context) =>
|
||||
context.connectionId
|
||||
? {}
|
||||
|
|
@ -116,7 +118,8 @@ vi.mock('./local-git-config-signature', () => ({
|
|||
vi.mock('./rate-limit', () => ({
|
||||
getRateLimit: getRateLimitMock,
|
||||
rateLimitGuard: rateLimitGuardMock,
|
||||
noteRateLimitSpend: noteRateLimitSpendMock
|
||||
noteRateLimitSpend: noteRateLimitSpendMock,
|
||||
noteRepositoryRateLimitSpend: noteRepositoryRateLimitSpendMock
|
||||
}))
|
||||
|
||||
import {
|
||||
|
|
|
|||
|
|
@ -649,18 +649,9 @@ describe('pr-refresh-coordinator', () => {
|
|||
'graphql',
|
||||
executionOptions
|
||||
)
|
||||
expect(noteRepositoryRateLimitSpendMock).toHaveBeenCalledWith(
|
||||
testCase.repository,
|
||||
'core',
|
||||
1,
|
||||
executionOptions
|
||||
)
|
||||
expect(noteRepositoryRateLimitSpendMock).toHaveBeenCalledWith(
|
||||
testCase.repository,
|
||||
'graphql',
|
||||
1,
|
||||
executionOptions
|
||||
)
|
||||
// Why (#11532): the lookup itself debits the snapshot now, so every caller
|
||||
// is accounted for; the coordinator must not double-charge on top.
|
||||
expect(noteRepositoryRateLimitSpendMock).not.toHaveBeenCalled()
|
||||
expect(getPRForBranchOutcomeMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -11,12 +11,11 @@ import type {
|
|||
import { getPRForBranchOutcome, type GitHubPRBranchLookupOptions } from './client'
|
||||
import { getOriginGitHubApiRepository } from './github-api-repository'
|
||||
import { ghRepoExecOptions, githubRepoContext } from './gh-utils'
|
||||
import { getRateLimit, repositoryRateLimitGuard, spendsSharedGitHubComQuota } from './rate-limit'
|
||||
import {
|
||||
getRateLimit,
|
||||
noteRepositoryRateLimitSpend,
|
||||
repositoryRateLimitGuard,
|
||||
spendsSharedGitHubComQuota
|
||||
} from './rate-limit'
|
||||
lookupBackoffDelayMs,
|
||||
NO_REVIEW_REFRESH_INTERVAL_MS
|
||||
} from '../source-control/hosted-review-refresh-pacing'
|
||||
import { recordCoalescedCrashBreadcrumb } from '../crash-reporting/crash-breadcrumb-store'
|
||||
import { sendToTrustedUIRenderer } from '../ipc/ui'
|
||||
|
||||
|
|
@ -74,8 +73,6 @@ const BACKGROUND_BUDGET_WINDOW_MS = 5 * 60_000
|
|||
const MIN_BACKGROUND_SPACING_MS = 10_000
|
||||
const BACKGROUND_BUDGET_MAX = 20
|
||||
const POST_PUSH_DELAY_MS = 2_500
|
||||
const BACKOFF_BASE_MS = 60_000
|
||||
const BACKOFF_MAX_MS = 15 * 60_000
|
||||
const DIAGNOSTIC_BREADCRUMB_MIN_INTERVAL_MS = 30_000
|
||||
const ACTIVE_BURST_WINDOW_MS = 30_000
|
||||
const ACTIVE_BURST_MAX = 3
|
||||
|
|
@ -424,8 +421,7 @@ function removeQueuedAliasForInvalidCandidate(key: string, alias: GitHubPRRefres
|
|||
*/
|
||||
function nextVisibleErrorRetryAt(key: string): number {
|
||||
const failures = (errorBackoff.get(key)?.failures ?? 0) + 1
|
||||
const retryAt =
|
||||
Date.now() + Math.min(BACKOFF_MAX_MS, BACKOFF_BASE_MS * 2 ** Math.min(failures - 1, 4))
|
||||
const retryAt = Date.now() + lookupBackoffDelayMs(failures)
|
||||
errorBackoff.set(key, { failures, retryAt })
|
||||
return retryAt
|
||||
}
|
||||
|
|
@ -509,7 +505,7 @@ function refreshIntervalForCandidate(candidate: GitHubPRRefreshCandidate): numbe
|
|||
return 30 * 60_000
|
||||
}
|
||||
if (candidate.cachedHasPR === false) {
|
||||
return 15 * 60_000
|
||||
return NO_REVIEW_REFRESH_INTERVAL_MS
|
||||
}
|
||||
if (
|
||||
candidate.cachedHasPR === true &&
|
||||
|
|
@ -776,9 +772,8 @@ async function drainQueue(): Promise<void> {
|
|||
// Why: tab/worktree churn can enqueue many distinct active refreshes that each probe local Git.
|
||||
noteActiveStart(next)
|
||||
}
|
||||
for (const bucket of buckets) {
|
||||
noteRepositoryRateLimitSpend(repository, bucket, 1, executionOptions)
|
||||
}
|
||||
// Why (#11532): the lookup itself now debits the snapshot, so every
|
||||
// caller is accounted for; debiting here too would double-count.
|
||||
}
|
||||
|
||||
const outcome = await getPRForBranchOutcome(
|
||||
|
|
|
|||
|
|
@ -285,6 +285,26 @@ describe('registerHostedReviewHandlers', () => {
|
|||
localGitExecOptions: { wslDistro: 'Ubuntu' }
|
||||
})
|
||||
)
|
||||
// Card-list polling is the O(N) tier and must not claim the fast one.
|
||||
expect(getHostedReviewForBranchMock.mock.calls[0][0]).not.toHaveProperty('active')
|
||||
})
|
||||
|
||||
it('carries a selected-worktree claim through to the branch lookup', async () => {
|
||||
getHostedReviewForBranchMock.mockResolvedValueOnce(null)
|
||||
registerHostedReviewHandlers(store as never, stats as never)
|
||||
|
||||
await handlers['hostedReview:forBranch'](null, {
|
||||
repoPath,
|
||||
repoId: repo.id,
|
||||
branch: 'feature/selected',
|
||||
active: true
|
||||
})
|
||||
|
||||
// Why: the right sidebar renders only the selected worktree, so its lookup
|
||||
// earns the per-minute tier instead of the card-list interval (#11532).
|
||||
expect(getHostedReviewForBranchMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ branch: 'feature/selected', active: true })
|
||||
)
|
||||
})
|
||||
|
||||
it('passes SSH connectionId through create eligibility instead of blocking the worktree', async () => {
|
||||
|
|
|
|||
|
|
@ -91,6 +91,7 @@ export function registerHostedReviewHandlers(store: Store, stats: StatsCollector
|
|||
linkedAzureDevOpsPR: args.linkedAzureDevOpsPR ?? null,
|
||||
linkedGiteaPR: args.linkedGiteaPR ?? null,
|
||||
currentHeadOid: args.currentHeadOid ?? null,
|
||||
...(args.active === true ? { active: true } : {}),
|
||||
...(Object.keys(localGitOptions).length > 0 ? { localGitExecOptions: localGitOptions } : {})
|
||||
})
|
||||
if (review?.provider === 'github' && !stats.hasCountedPR(review.url)) {
|
||||
|
|
|
|||
|
|
@ -18623,6 +18623,7 @@ export class OrcaRuntimeService {
|
|||
repoSelector: string
|
||||
branch: string
|
||||
currentHeadOid?: string | null
|
||||
active?: boolean
|
||||
linkedGitHubPR?: number | null
|
||||
fallbackGitHubPR?: number | null
|
||||
linkedGitLabMR?: number | null
|
||||
|
|
@ -18637,6 +18638,7 @@ export class OrcaRuntimeService {
|
|||
connectionId: repo.connectionId ?? null,
|
||||
branch: args.branch,
|
||||
currentHeadOid: args.currentHeadOid ?? null,
|
||||
...(args.active === true ? { active: true } : {}),
|
||||
linkedGitHubPR: args.linkedGitHubPR ?? null,
|
||||
fallbackGitHubPR: args.linkedGitHubPR == null ? (args.fallbackGitHubPR ?? null) : null,
|
||||
linkedGitLabMR: args.linkedGitLabMR ?? null,
|
||||
|
|
|
|||
|
|
@ -49,6 +49,28 @@ describe('hosted review RPC methods', () => {
|
|||
})
|
||||
})
|
||||
|
||||
it('carries a selected-worktree claim through to the runtime', async () => {
|
||||
const runtime = {
|
||||
getRuntimeId: () => 'test-runtime',
|
||||
getHostedReviewForBranch: vi.fn().mockResolvedValue(null)
|
||||
} as unknown as OrcaRuntimeService
|
||||
const dispatcher = new RpcDispatcher({ runtime, methods: HOSTED_REVIEW_METHODS })
|
||||
|
||||
await dispatcher.dispatch(
|
||||
makeRequest('hostedReview.forBranch', {
|
||||
repo: '/repo',
|
||||
branch: 'feature/selected',
|
||||
active: true
|
||||
})
|
||||
)
|
||||
|
||||
// Why: without this the mobile PR sidebar would sit on the card-list pacing
|
||||
// and take a no-review interval to notice a PR opened elsewhere (#11532).
|
||||
expect(runtime.getHostedReviewForBranch).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ active: true })
|
||||
)
|
||||
})
|
||||
|
||||
it('dispatches creation eligibility requests to the runtime', async () => {
|
||||
const runtime = {
|
||||
getRuntimeId: () => 'test-runtime',
|
||||
|
|
|
|||
|
|
@ -6,6 +6,8 @@ const HostedReviewForBranch = z.object({
|
|||
repo: requiredString('Missing repo selector'),
|
||||
branch: requiredString('Missing branch'),
|
||||
currentHeadOid: z.string().nullable().optional(),
|
||||
// Only the caller's selected worktree; the host caps how many earn the fast tier.
|
||||
active: z.boolean().optional(),
|
||||
linkedGitHubPR: z.number().int().positive().nullable().optional(),
|
||||
fallbackGitHubPR: z.number().int().positive().nullable().optional(),
|
||||
linkedGitLabMR: z.number().int().positive().nullable().optional(),
|
||||
|
|
@ -54,6 +56,7 @@ export const HOSTED_REVIEW_METHODS: RpcMethod[] = [
|
|||
repoSelector: params.repo,
|
||||
branch: params.branch,
|
||||
currentHeadOid: params.currentHeadOid ?? null,
|
||||
...(params.active === true ? { active: true } : {}),
|
||||
linkedGitHubPR: params.linkedGitHubPR ?? null,
|
||||
...(fallbackGitHubPR !== null ? { fallbackGitHubPR } : {}),
|
||||
linkedGitLabMR: params.linkedGitLabMR ?? null,
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ const {
|
|||
getProjectSlugMock,
|
||||
getPRForBranchOutcomeMock,
|
||||
getRepoSlugMock,
|
||||
getGitHubPRLookupRateLimitBlockMock,
|
||||
getEnterpriseGitHubRepoSlugMock
|
||||
} = vi.hoisted(() => ({
|
||||
createGitHubPullRequestMock: vi.fn(),
|
||||
|
|
@ -25,12 +26,16 @@ const {
|
|||
getProjectSlugMock: vi.fn(),
|
||||
getPRForBranchOutcomeMock: vi.fn(),
|
||||
getRepoSlugMock: vi.fn(),
|
||||
getGitHubPRLookupRateLimitBlockMock: vi.fn(async () => null),
|
||||
getEnterpriseGitHubRepoSlugMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('../gitlab/client', () => ({
|
||||
getProjectSlug: getProjectSlugMock,
|
||||
getMergeRequestForBranch: getMergeRequestForBranchMock,
|
||||
// Why: forge-provider resolves branch reviews via the OrThrow variant so
|
||||
// lookup failures surface as unavailable instead of "no MR found".
|
||||
getMergeRequestForBranchOrThrow: getMergeRequestForBranchMock,
|
||||
getMergeRequest: vi.fn()
|
||||
}))
|
||||
|
||||
|
|
@ -41,7 +46,8 @@ vi.mock('../gitlab/merge-request-creation', () => ({
|
|||
vi.mock('../github/client', () => ({
|
||||
createGitHubPullRequest: createGitHubPullRequestMock,
|
||||
getRepoSlug: getRepoSlugMock,
|
||||
getPRForBranchOutcome: getPRForBranchOutcomeMock
|
||||
getPRForBranchOutcome: getPRForBranchOutcomeMock,
|
||||
getGitHubPRLookupRateLimitBlock: getGitHubPRLookupRateLimitBlockMock
|
||||
}))
|
||||
|
||||
vi.mock('../github/github-enterprise-repository', () => ({
|
||||
|
|
@ -103,6 +109,8 @@ describe('forge provider interface', () => {
|
|||
getPRForBranchOutcomeMock.mockReset()
|
||||
getRepoSlugMock.mockReset()
|
||||
getEnterpriseGitHubRepoSlugMock.mockReset()
|
||||
getGitHubPRLookupRateLimitBlockMock.mockReset()
|
||||
getGitHubPRLookupRateLimitBlockMock.mockResolvedValue(null)
|
||||
})
|
||||
|
||||
it('preserves the existing hosted provider detection order', async () => {
|
||||
|
|
@ -377,4 +385,48 @@ describe('forge provider interface', () => {
|
|||
})
|
||||
).rejects.toThrow(/network/)
|
||||
})
|
||||
|
||||
it('refuses a GitHub branch lookup while the rate-limit budget is exhausted (#11532)', async () => {
|
||||
getGitHubPRLookupRateLimitBlockMock.mockResolvedValueOnce({
|
||||
resetAt: 1_800_000_000
|
||||
} as never)
|
||||
|
||||
await expect(
|
||||
getForgeProviderById('github').getReviewForBranch({
|
||||
repoPath: '/repo',
|
||||
connectionId: null,
|
||||
branch: 'feature/x'
|
||||
})
|
||||
// Throwing (not null) keeps a low budget from reading as "no pull request".
|
||||
).rejects.toThrow(/rate_limited/)
|
||||
expect(getPRForBranchOutcomeMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('refuses a GitHub lookup by number while the rate-limit budget is exhausted (#11532)', async () => {
|
||||
getGitHubPRLookupRateLimitBlockMock.mockResolvedValueOnce({
|
||||
resetAt: 1_800_000_000
|
||||
} as never)
|
||||
|
||||
await expect(
|
||||
getForgeProviderById('github').getReviewByNumber({
|
||||
repoPath: '/repo',
|
||||
connectionId: null,
|
||||
number: 42
|
||||
})
|
||||
).rejects.toThrow(/rate_limited/)
|
||||
expect(getPRForBranchOutcomeMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not gate non-GitHub providers on the GitHub rate limit', async () => {
|
||||
getGitHubPRLookupRateLimitBlockMock.mockResolvedValue({ resetAt: 1_800_000_000 } as never)
|
||||
getMergeRequestForBranchMock.mockResolvedValue(null)
|
||||
|
||||
await expect(
|
||||
getForgeProviderById('gitlab').getReviewForBranch({
|
||||
repoPath: '/repo',
|
||||
connectionId: null,
|
||||
branch: 'feature/x'
|
||||
})
|
||||
).resolves.toBeNull()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -21,7 +21,12 @@ import {
|
|||
getGiteaRepoSlug
|
||||
} from '../gitea/client'
|
||||
import { createGiteaPullRequest } from '../gitea/pull-request-creation'
|
||||
import { createGitHubPullRequest, getPRForBranchOutcome, getRepoSlug } from '../github/client'
|
||||
import {
|
||||
createGitHubPullRequest,
|
||||
getGitHubPRLookupRateLimitBlock,
|
||||
getPRForBranchOutcome,
|
||||
getRepoSlug
|
||||
} from '../github/client'
|
||||
import { getMergeRequest, getMergeRequestForBranchOrThrow, getProjectSlug } from '../gitlab/client'
|
||||
import { createGitLabMergeRequest } from '../gitlab/merge-request-creation'
|
||||
import {
|
||||
|
|
@ -122,6 +127,29 @@ function unwrapGitHubPRForBranchOutcome(
|
|||
return outcome.kind === 'found' ? mapGitHubReview(outcome.pr) : null
|
||||
}
|
||||
|
||||
/**
|
||||
* Why (#11532): hosted-review lookups reach GitHub outside the PR refresh
|
||||
* coordinator's paced queue, so they need the same rate-limit floor. Throwing
|
||||
* (rather than returning null) keeps a low budget from reading as "no pull
|
||||
* request" — callers preserve the last known review and back off.
|
||||
*/
|
||||
async function assertGitHubReviewRateLimitBudget(
|
||||
input: ForgeProviderRepositoryContext
|
||||
): Promise<void> {
|
||||
const block = await getGitHubPRLookupRateLimitBlock(
|
||||
input.repoPath,
|
||||
input.connectionId,
|
||||
getHostedReviewLocalGitOptions(input)
|
||||
)
|
||||
if (block) {
|
||||
throw new Error(
|
||||
`GitHub PR lookup failed (rate_limited): GitHub rate limit is low. Try again after ${new Date(
|
||||
block.resetAt * 1000
|
||||
).toLocaleTimeString()}.`
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
const gitHubForgeProvider = {
|
||||
id: 'github',
|
||||
supportsReviewCreation: true,
|
||||
|
|
@ -131,6 +159,7 @@ const gitHubForgeProvider = {
|
|||
resolveRepository: async (context) =>
|
||||
getRepoSlug(context.repoPath, context.connectionId, ...hostedReviewExecutionArgs(context)),
|
||||
async getReviewForBranch(input) {
|
||||
await assertGitHubReviewRateLimitBudget(input)
|
||||
const fallbackReviewNumber =
|
||||
input.linkedReviewNumber == null ? (input.fallbackReviewNumber ?? null) : null
|
||||
const executionArgs = hostedReviewExecutionArgs(input)
|
||||
|
|
@ -149,6 +178,7 @@ const gitHubForgeProvider = {
|
|||
return unwrapGitHubPRForBranchOutcome(outcome)
|
||||
},
|
||||
async getReviewByNumber(input) {
|
||||
await assertGitHubReviewRateLimitBudget(input)
|
||||
const executionArgs = hostedReviewExecutionArgs(input)
|
||||
const outcome =
|
||||
executionArgs.length > 0
|
||||
|
|
|
|||
|
|
@ -0,0 +1,374 @@
|
|||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { HostedReviewInfo } from '../../shared/hosted-review'
|
||||
import {
|
||||
__resetHostedReviewBranchCacheForTests,
|
||||
invalidateHostedReviewBranchCache,
|
||||
withHostedReviewBranchCache
|
||||
} from './hosted-review-branch-cache'
|
||||
|
||||
const identity = { repoPath: '/repo', connectionId: null, branch: 'feature/x' }
|
||||
|
||||
const openReview: HostedReviewInfo = {
|
||||
provider: 'github',
|
||||
number: 7,
|
||||
title: 'Open PR',
|
||||
state: 'open',
|
||||
url: 'https://github.com/acme/orca/pull/7',
|
||||
status: 'success',
|
||||
updatedAt: '2026-07-31T00:00:00.000Z',
|
||||
mergeable: 'MERGEABLE'
|
||||
}
|
||||
|
||||
const mergedReview: HostedReviewInfo = { ...openReview, state: 'merged' }
|
||||
|
||||
describe('hosted review branch cache (#11532)', () => {
|
||||
beforeEach(() => {
|
||||
__resetHostedReviewBranchCacheForTests()
|
||||
vi.useFakeTimers()
|
||||
vi.setSystemTime(1_000_000)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('holds a no-review answer far longer than a poll interval', async () => {
|
||||
const lookup = vi.fn(async () => null)
|
||||
|
||||
await withHostedReviewBranchCache(identity, { headOid: null }, lookup)
|
||||
vi.setSystemTime(1_000_000 + 5 * 60_000)
|
||||
await withHostedReviewBranchCache(identity, { headOid: null }, lookup)
|
||||
|
||||
expect(lookup).toHaveBeenCalledTimes(1)
|
||||
|
||||
vi.setSystemTime(1_000_000 + 15 * 60_000 + 1)
|
||||
await withHostedReviewBranchCache(identity, { headOid: null }, lookup)
|
||||
|
||||
expect(lookup).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('keeps the no-review answer while the branch head moves', async () => {
|
||||
const lookup = vi.fn(async () => null)
|
||||
|
||||
await withHostedReviewBranchCache(identity, { headOid: 'aaa' }, lookup)
|
||||
vi.setSystemTime(1_000_000 + 60_000)
|
||||
await withHostedReviewBranchCache(identity, { headOid: 'bbb' }, lookup)
|
||||
|
||||
// A commit is not evidence that a review was opened, so it must not defeat
|
||||
// the long interval — that is what kept busy worktrees polling every minute.
|
||||
expect(lookup).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('refreshes a found review at the caller cadence', async () => {
|
||||
const lookup = vi.fn(async () => openReview)
|
||||
|
||||
await withHostedReviewBranchCache(identity, { headOid: null }, lookup)
|
||||
vi.setSystemTime(1_000_000 + 30_000)
|
||||
await withHostedReviewBranchCache(identity, { headOid: null }, lookup)
|
||||
expect(lookup).toHaveBeenCalledTimes(1)
|
||||
|
||||
vi.setSystemTime(1_000_000 + 60_001)
|
||||
await withHostedReviewBranchCache(identity, { headOid: null }, lookup)
|
||||
expect(lookup).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('drops a merged review once the inspected head moves off it', async () => {
|
||||
const lookup = vi.fn(async () => mergedReview)
|
||||
|
||||
await withHostedReviewBranchCache(identity, { headOid: 'aaa' }, lookup)
|
||||
await withHostedReviewBranchCache(identity, { headOid: 'aaa' }, lookup)
|
||||
expect(lookup).toHaveBeenCalledTimes(1)
|
||||
|
||||
await withHostedReviewBranchCache(identity, { headOid: 'bbb' }, lookup)
|
||||
expect(lookup).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('collapses concurrent callers onto one lookup', async () => {
|
||||
let resolveLookup: (value: HostedReviewInfo | null) => void = () => {}
|
||||
const lookup = vi.fn(
|
||||
() =>
|
||||
new Promise<HostedReviewInfo | null>((resolve) => {
|
||||
resolveLookup = resolve
|
||||
})
|
||||
)
|
||||
|
||||
const first = withHostedReviewBranchCache(identity, { headOid: null }, lookup)
|
||||
const second = withHostedReviewBranchCache(identity, { headOid: null }, lookup)
|
||||
resolveLookup(openReview)
|
||||
|
||||
await expect(first).resolves.toEqual(openReview)
|
||||
await expect(second).resolves.toEqual(openReview)
|
||||
expect(lookup).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('separates lookups that differ only by linked review number', async () => {
|
||||
const lookup = vi.fn(async () => null)
|
||||
|
||||
await withHostedReviewBranchCache({ ...identity, linkedGitHubPR: 1 }, { headOid: null }, lookup)
|
||||
await withHostedReviewBranchCache({ ...identity, linkedGitHubPR: 2 }, { headOid: null }, lookup)
|
||||
|
||||
expect(lookup).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('backs a failing branch off instead of re-asking every poll', async () => {
|
||||
const lookup = vi.fn(async () => {
|
||||
throw new Error('rate limited')
|
||||
})
|
||||
|
||||
await expect(withHostedReviewBranchCache(identity, { headOid: null }, lookup)).rejects.toThrow(
|
||||
'rate limited'
|
||||
)
|
||||
vi.setSystemTime(1_000_000 + 30_000)
|
||||
// Nothing cached, so the caller must still hear a failure — but no API call.
|
||||
await expect(withHostedReviewBranchCache(identity, { headOid: null }, lookup)).rejects.toThrow(
|
||||
/backing off/
|
||||
)
|
||||
expect(lookup).toHaveBeenCalledTimes(1)
|
||||
|
||||
vi.setSystemTime(1_000_000 + 60_001)
|
||||
await expect(withHostedReviewBranchCache(identity, { headOid: null }, lookup)).rejects.toThrow(
|
||||
'rate limited'
|
||||
)
|
||||
expect(lookup).toHaveBeenCalledTimes(2)
|
||||
|
||||
// The second failure doubles the window.
|
||||
vi.setSystemTime(1_000_000 + 60_001 + 60_000)
|
||||
await expect(withHostedReviewBranchCache(identity, { headOid: null }, lookup)).rejects.toThrow(
|
||||
/backing off/
|
||||
)
|
||||
expect(lookup).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('serves the last known review from the failure itself, not only the backoff', async () => {
|
||||
const lookup = vi
|
||||
.fn<() => Promise<HostedReviewInfo | null>>()
|
||||
.mockResolvedValueOnce(openReview)
|
||||
.mockRejectedValueOnce(new Error('transient'))
|
||||
|
||||
await withHostedReviewBranchCache(identity, { headOid: null }, lookup)
|
||||
vi.setSystemTime(1_000_000 + 60_001)
|
||||
// The review must not blink out on the first failure and reappear on the next
|
||||
// poll once the backoff window is what serves it.
|
||||
await expect(withHostedReviewBranchCache(identity, { headOid: null }, lookup)).resolves.toEqual(
|
||||
openReview
|
||||
)
|
||||
|
||||
vi.setSystemTime(1_000_000 + 60_001 + 1_000)
|
||||
await expect(withHostedReviewBranchCache(identity, { headOid: null }, lookup)).resolves.toEqual(
|
||||
openReview
|
||||
)
|
||||
expect(lookup).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('resets the escalation once a lookup succeeds', async () => {
|
||||
const lookup = vi
|
||||
.fn<() => Promise<HostedReviewInfo | null>>()
|
||||
.mockRejectedValueOnce(new Error('first'))
|
||||
.mockResolvedValueOnce(openReview)
|
||||
.mockRejectedValueOnce(new Error('second'))
|
||||
.mockResolvedValue(mergedReview)
|
||||
|
||||
await expect(withHostedReviewBranchCache(identity, { headOid: null }, lookup)).rejects.toThrow(
|
||||
'first'
|
||||
)
|
||||
vi.setSystemTime(1_000_000 + 60_001)
|
||||
await expect(withHostedReviewBranchCache(identity, { headOid: null }, lookup)).resolves.toEqual(
|
||||
openReview
|
||||
)
|
||||
|
||||
// The success clears the counter, so the next failure starts at the base
|
||||
// window again rather than resuming a doubled one. That failure is served
|
||||
// from the stale entry, but it still counts.
|
||||
vi.setSystemTime(1_000_000 + 2 * 60_001)
|
||||
await expect(withHostedReviewBranchCache(identity, { headOid: null }, lookup)).resolves.toEqual(
|
||||
openReview
|
||||
)
|
||||
vi.setSystemTime(1_000_000 + 3 * 60_001)
|
||||
await expect(withHostedReviewBranchCache(identity, { headOid: null }, lookup)).resolves.toEqual(
|
||||
mergedReview
|
||||
)
|
||||
expect(lookup).toHaveBeenCalledTimes(4)
|
||||
})
|
||||
|
||||
it('retires a cached no-review answer when Orca opens a review', async () => {
|
||||
const lookup = vi
|
||||
.fn<() => Promise<HostedReviewInfo | null>>()
|
||||
.mockResolvedValueOnce(null)
|
||||
.mockResolvedValueOnce(openReview)
|
||||
|
||||
await withHostedReviewBranchCache(identity, { headOid: null }, lookup)
|
||||
invalidateHostedReviewBranchCache('/repo', null)
|
||||
|
||||
await expect(withHostedReviewBranchCache(identity, { headOid: null }, lookup)).resolves.toEqual(
|
||||
openReview
|
||||
)
|
||||
expect(lookup).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('discards a lookup that was already in flight when Orca opened a review', async () => {
|
||||
let resolveLookup: (value: HostedReviewInfo | null) => void = () => {}
|
||||
const lookup = vi
|
||||
.fn<() => Promise<HostedReviewInfo | null>>()
|
||||
.mockImplementationOnce(
|
||||
() =>
|
||||
new Promise<HostedReviewInfo | null>((resolve) => {
|
||||
resolveLookup = resolve
|
||||
})
|
||||
)
|
||||
.mockResolvedValue(openReview)
|
||||
|
||||
const inflight = withHostedReviewBranchCache(identity, { headOid: null }, lookup)
|
||||
invalidateHostedReviewBranchCache('/repo', null)
|
||||
// The poll started before the review existed, so its "no review" answer is
|
||||
// older than the invalidation and must not be cached back over it.
|
||||
resolveLookup(null)
|
||||
await expect(inflight).resolves.toBeNull()
|
||||
|
||||
await expect(withHostedReviewBranchCache(identity, { headOid: null }, lookup)).resolves.toEqual(
|
||||
openReview
|
||||
)
|
||||
expect(lookup).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('leaves another repo in-flight lookup cacheable across an invalidation', async () => {
|
||||
let resolveLookup: (value: HostedReviewInfo | null) => void = () => {}
|
||||
const other = { ...identity, repoPath: '/other' }
|
||||
const lookup = vi.fn<() => Promise<HostedReviewInfo | null>>().mockImplementationOnce(
|
||||
() =>
|
||||
new Promise<HostedReviewInfo | null>((resolve) => {
|
||||
resolveLookup = resolve
|
||||
})
|
||||
)
|
||||
|
||||
const inflight = withHostedReviewBranchCache(other, { headOid: null }, lookup)
|
||||
invalidateHostedReviewBranchCache('/repo', null)
|
||||
resolveLookup(null)
|
||||
await inflight
|
||||
|
||||
await withHostedReviewBranchCache(other, { headOid: null }, lookup)
|
||||
expect(lookup).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('scopes invalidation to one repo', async () => {
|
||||
const lookup = vi.fn(async () => null)
|
||||
|
||||
await withHostedReviewBranchCache(identity, { headOid: null }, lookup)
|
||||
await withHostedReviewBranchCache(
|
||||
{ ...identity, repoPath: '/other' },
|
||||
{ headOid: null },
|
||||
lookup
|
||||
)
|
||||
expect(lookup).toHaveBeenCalledTimes(2)
|
||||
|
||||
invalidateHostedReviewBranchCache('/other', null)
|
||||
|
||||
await withHostedReviewBranchCache(identity, { headOid: null }, lookup)
|
||||
expect(lookup).toHaveBeenCalledTimes(2)
|
||||
await withHostedReviewBranchCache(
|
||||
{ ...identity, repoPath: '/other' },
|
||||
{ headOid: null },
|
||||
lookup
|
||||
)
|
||||
expect(lookup).toHaveBeenCalledTimes(3)
|
||||
})
|
||||
|
||||
it('keeps SSH and local repos with the same path apart', async () => {
|
||||
const lookup = vi.fn(async () => null)
|
||||
|
||||
await withHostedReviewBranchCache(identity, { headOid: null }, lookup)
|
||||
await withHostedReviewBranchCache(
|
||||
{ ...identity, connectionId: 'ssh-1' },
|
||||
{ headOid: null },
|
||||
lookup
|
||||
)
|
||||
|
||||
expect(lookup).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
describe('selected-worktree tier', () => {
|
||||
it('re-checks the selected branch every minute while the card list waits', async () => {
|
||||
const selected = vi.fn(async () => null)
|
||||
const listed = vi.fn(async () => null)
|
||||
const other = { ...identity, branch: 'feature/y' }
|
||||
|
||||
await withHostedReviewBranchCache(identity, { headOid: null, active: true }, selected)
|
||||
await withHostedReviewBranchCache(other, { headOid: null }, listed)
|
||||
|
||||
vi.setSystemTime(1_000_000 + 60_001)
|
||||
await withHostedReviewBranchCache(identity, { headOid: null, active: true }, selected)
|
||||
await withHostedReviewBranchCache(other, { headOid: null }, listed)
|
||||
|
||||
expect(selected).toHaveBeenCalledTimes(2)
|
||||
expect(listed).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('retires a cached no-review answer when a branch becomes the selection', async () => {
|
||||
const lookup = vi.fn(async () => null)
|
||||
|
||||
await withHostedReviewBranchCache(identity, { headOid: null }, lookup)
|
||||
vi.setSystemTime(1_000_000 + 1_000)
|
||||
|
||||
// Selecting the worktree is the user asking whether a review exists yet.
|
||||
await withHostedReviewBranchCache(identity, { headOid: null, active: true }, lookup)
|
||||
expect(lookup).toHaveBeenCalledTimes(2)
|
||||
|
||||
// Staying on it does not re-ask; the minute interval takes over.
|
||||
await withHostedReviewBranchCache(identity, { headOid: null, active: true }, lookup)
|
||||
expect(lookup).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('keeps a found review when a branch becomes the selection', async () => {
|
||||
const lookup = vi.fn(async () => openReview)
|
||||
|
||||
await withHostedReviewBranchCache(identity, { headOid: null }, lookup)
|
||||
vi.setSystemTime(1_000_000 + 1_000)
|
||||
await withHostedReviewBranchCache(identity, { headOid: null, active: true }, lookup)
|
||||
|
||||
// Only the long no-review answer is worth spending a call to retire.
|
||||
expect(lookup).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('caps the fast tier so a caller cannot promote a whole list', async () => {
|
||||
const lookup = vi.fn(async () => null)
|
||||
const branchAt = (index: number) => ({ ...identity, branch: `feature/${index}` })
|
||||
|
||||
// One more claim than the cap allows, so the first claim is evicted.
|
||||
for (let index = 0; index <= 8; index += 1) {
|
||||
await withHostedReviewBranchCache(branchAt(index), { headOid: null, active: true }, lookup)
|
||||
}
|
||||
expect(lookup).toHaveBeenCalledTimes(9)
|
||||
|
||||
vi.setSystemTime(1_000_000 + 60_001)
|
||||
// The evicted branch is back on card pacing; the newest claim is not.
|
||||
await withHostedReviewBranchCache(branchAt(0), { headOid: null }, lookup)
|
||||
expect(lookup).toHaveBeenCalledTimes(9)
|
||||
await withHostedReviewBranchCache(branchAt(8), { headOid: null }, lookup)
|
||||
expect(lookup).toHaveBeenCalledTimes(10)
|
||||
})
|
||||
|
||||
it('paces a card poll of the selected branch at the selection interval', async () => {
|
||||
const lookup = vi.fn(async () => null)
|
||||
|
||||
await withHostedReviewBranchCache(identity, { headOid: null, active: true }, lookup)
|
||||
vi.setSystemTime(1_000_000 + 60_001)
|
||||
|
||||
// Freshness is a property of the branch, not of which surface asked.
|
||||
await withHostedReviewBranchCache(identity, { headOid: null }, lookup)
|
||||
expect(lookup).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('returns a lapsed selection to card pacing', async () => {
|
||||
const lookup = vi.fn(async () => null)
|
||||
|
||||
await withHostedReviewBranchCache(identity, { headOid: null, active: true }, lookup)
|
||||
// Nothing re-asserted the selection for a full no-review interval.
|
||||
vi.setSystemTime(1_000_000 + 15 * 60_000 + 1)
|
||||
await withHostedReviewBranchCache(identity, { headOid: null }, lookup)
|
||||
expect(lookup).toHaveBeenCalledTimes(2)
|
||||
|
||||
vi.setSystemTime(1_000_000 + 15 * 60_000 + 1 + 60_001)
|
||||
await withHostedReviewBranchCache(identity, { headOid: null }, lookup)
|
||||
expect(lookup).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,306 @@
|
|||
import type { HostedReviewInfo } from '../../shared/hosted-review'
|
||||
import {
|
||||
ACTIVE_CLAIM_TTL_MS,
|
||||
ACTIVE_REFRESH_INTERVAL_MS,
|
||||
lookupBackoffDelayMs,
|
||||
LOOKUP_BACKOFF_MAX_MS,
|
||||
MAX_ACTIVE_BRANCHES,
|
||||
NO_REVIEW_REFRESH_INTERVAL_MS
|
||||
} from './hosted-review-refresh-pacing'
|
||||
|
||||
/**
|
||||
* Process-wide cache for branch review lookups (#11532).
|
||||
*
|
||||
* `hostedReview:forBranch` is polled by every desktop window, the mobile client
|
||||
* and `orca serve` alike, and each one used to reach the provider directly. The
|
||||
* host's API quota is per user, so the only place that can pace them together is
|
||||
* here — the single funnel they all pass through.
|
||||
*
|
||||
* Pacing is tiered by what the user is looking at rather than applied flat: the
|
||||
* selected worktree is O(1) and can afford a per-minute re-check, while the
|
||||
* worktree list is O(N) and is what exhausts the budget.
|
||||
*/
|
||||
|
||||
// Why: a found review still refreshes at the callers' poll cadence; the cache
|
||||
// exists to collapse concurrent clients, not to make review state go stale.
|
||||
const FOUND_REVIEW_TTL_MS = 60_000
|
||||
const MAX_ENTRIES = 500
|
||||
|
||||
type CacheEntry = {
|
||||
review: HostedReviewInfo | null
|
||||
fetchedAt: number
|
||||
headOid: string | null
|
||||
}
|
||||
|
||||
const entries = new Map<string, CacheEntry>()
|
||||
const inflight = new Map<string, Promise<HostedReviewInfo | null>>()
|
||||
const failureBackoff = new Map<string, { until: number; failures: number }>()
|
||||
/** Branches a caller reported as its current selection, least recent first. */
|
||||
const activeClaims = new Map<string, number>()
|
||||
/** Bumped per repo on invalidation so a lookup that predates it cannot store. */
|
||||
const scopeGenerations = new Map<string, number>()
|
||||
|
||||
// Why: NUL is the one byte a repo path or branch name cannot contain, so a
|
||||
// scope prefix cannot straddle a component boundary — invalidating `/a/b` must
|
||||
// not also flush the unrelated repo at `/a/b c`.
|
||||
const KEY_SEPARATOR = '\0'
|
||||
|
||||
export type HostedReviewBranchCacheIdentity = {
|
||||
repoPath: string
|
||||
connectionId?: string | null
|
||||
branch: string
|
||||
linkedGitHubPR?: number | null
|
||||
fallbackGitHubPR?: number | null
|
||||
linkedGitLabMR?: number | null
|
||||
linkedBitbucketPR?: number | null
|
||||
linkedAzureDevOpsPR?: number | null
|
||||
linkedGiteaPR?: number | null
|
||||
localGitExecOptions?: unknown
|
||||
}
|
||||
|
||||
export type HostedReviewBranchCacheOptions = {
|
||||
/** The worktree's checked-out HEAD oid, for merged-at-head visibility. */
|
||||
headOid: string | null
|
||||
/** Set by surfaces that only ever render the selected worktree. */
|
||||
active?: boolean
|
||||
}
|
||||
|
||||
/** Repo-scoped prefix so a single repo's entries can be dropped without a full flush. */
|
||||
function repoScope(repoPath: string, connectionId?: string | null): string {
|
||||
return `${connectionId ?? ''}${KEY_SEPARATOR}${repoPath}`
|
||||
}
|
||||
|
||||
export function hostedReviewBranchCacheKey(identity: HostedReviewBranchCacheIdentity): string {
|
||||
return [
|
||||
repoScope(identity.repoPath, identity.connectionId),
|
||||
identity.branch,
|
||||
// Each linked id selects a different lookup, so it belongs in the identity.
|
||||
identity.linkedGitHubPR ?? '',
|
||||
identity.fallbackGitHubPR ?? '',
|
||||
identity.linkedGitLabMR ?? '',
|
||||
identity.linkedBitbucketPR ?? '',
|
||||
identity.linkedAzureDevOpsPR ?? '',
|
||||
identity.linkedGiteaPR ?? '',
|
||||
identity.localGitExecOptions ? JSON.stringify(identity.localGitExecOptions) : ''
|
||||
].join(KEY_SEPARATOR)
|
||||
}
|
||||
|
||||
/**
|
||||
* Records the caller's current selection, reporting whether the branch was not
|
||||
* already active. Claims are least-recently-used so the fast tier stays bounded
|
||||
* no matter how many a client asserts.
|
||||
*/
|
||||
function noteActiveClaim(key: string): boolean {
|
||||
const now = Date.now()
|
||||
for (const [candidate, claimedAt] of activeClaims) {
|
||||
if (now - claimedAt > ACTIVE_CLAIM_TTL_MS) {
|
||||
activeClaims.delete(candidate)
|
||||
}
|
||||
}
|
||||
const wasActive = activeClaims.has(key)
|
||||
activeClaims.delete(key)
|
||||
activeClaims.set(key, now)
|
||||
while (activeClaims.size > MAX_ACTIVE_BRANCHES) {
|
||||
const oldest = activeClaims.keys().next().value
|
||||
if (oldest === undefined) {
|
||||
break
|
||||
}
|
||||
activeClaims.delete(oldest)
|
||||
}
|
||||
return !wasActive
|
||||
}
|
||||
|
||||
function isActiveBranch(key: string): boolean {
|
||||
const claimedAt = activeClaims.get(key)
|
||||
return claimedAt !== undefined && Date.now() - claimedAt <= ACTIVE_CLAIM_TTL_MS
|
||||
}
|
||||
|
||||
// Why: a merged review is the one answer that depends on the inspected head —
|
||||
// the merged-at-head carve-out keeps it visible only while the head matches.
|
||||
// Negative answers are deliberately head-insensitive, so a branch under active
|
||||
// commits cannot defeat the long no-review interval.
|
||||
function isHeadSensitive(entry: CacheEntry): boolean {
|
||||
return entry.review?.state === 'merged'
|
||||
}
|
||||
|
||||
function refreshIntervalMs(entry: CacheEntry, active: boolean): number {
|
||||
if (entry.review !== null) {
|
||||
return FOUND_REVIEW_TTL_MS
|
||||
}
|
||||
return active ? ACTIVE_REFRESH_INTERVAL_MS : NO_REVIEW_REFRESH_INTERVAL_MS
|
||||
}
|
||||
|
||||
function isFresh(entry: CacheEntry, headOid: string | null, active: boolean): boolean {
|
||||
if (isHeadSensitive(entry) && headOid !== null && entry.headOid !== null) {
|
||||
if (headOid !== entry.headOid) {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return Date.now() - entry.fetchedAt < refreshIntervalMs(entry, active)
|
||||
}
|
||||
|
||||
function storeEntry(key: string, entry: CacheEntry): void {
|
||||
entries.delete(key)
|
||||
entries.set(key, entry)
|
||||
while (entries.size > MAX_ENTRIES) {
|
||||
const oldest = entries.keys().next().value
|
||||
if (oldest === undefined) {
|
||||
break
|
||||
}
|
||||
entries.delete(oldest)
|
||||
}
|
||||
}
|
||||
|
||||
function backoffUntil(key: string): number | null {
|
||||
// Why: a lapsed window keeps its failure count, otherwise the very act of
|
||||
// retrying resets the escalation and the backoff never grows past the base.
|
||||
const entry = failureBackoff.get(key)
|
||||
return entry !== undefined && entry.until > Date.now() ? entry.until : null
|
||||
}
|
||||
|
||||
function noteFailure(key: string): void {
|
||||
const now = Date.now()
|
||||
for (const [candidate, entry] of failureBackoff) {
|
||||
// Why: only counts that lapsed a full max window ago are stale enough to
|
||||
// forget; anything more eager would undo the escalation above.
|
||||
if (now - entry.until > LOOKUP_BACKOFF_MAX_MS) {
|
||||
failureBackoff.delete(candidate)
|
||||
}
|
||||
}
|
||||
const failures = (failureBackoff.get(key)?.failures ?? 0) + 1
|
||||
failureBackoff.delete(key)
|
||||
failureBackoff.set(key, { until: now + lookupBackoffDelayMs(failures), failures })
|
||||
while (failureBackoff.size > MAX_ENTRIES) {
|
||||
const oldest = failureBackoff.keys().next().value
|
||||
if (oldest === undefined) {
|
||||
break
|
||||
}
|
||||
failureBackoff.delete(oldest)
|
||||
}
|
||||
}
|
||||
|
||||
function scopeGeneration(scope: string): number {
|
||||
return scopeGenerations.get(scope) ?? 0
|
||||
}
|
||||
|
||||
function bumpScopeGeneration(scope: string): void {
|
||||
const next = scopeGeneration(scope) + 1
|
||||
scopeGenerations.delete(scope)
|
||||
scopeGenerations.set(scope, next)
|
||||
// Why: an evicted scope reads as generation 0, which only makes a lookup in
|
||||
// flight at eviction discard its result — a wasted call, never a stale one.
|
||||
while (scopeGenerations.size > MAX_ENTRIES) {
|
||||
const oldest = scopeGenerations.keys().next().value
|
||||
if (oldest === undefined) {
|
||||
break
|
||||
}
|
||||
scopeGenerations.delete(oldest)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Drops every cached answer for a repo. Called when Orca itself opens a review,
|
||||
* so the new one is visible immediately instead of after the no-review interval.
|
||||
*/
|
||||
export function invalidateHostedReviewBranchCache(
|
||||
repoPath: string,
|
||||
connectionId?: string | null
|
||||
): void {
|
||||
const scope = repoScope(repoPath, connectionId)
|
||||
bumpScopeGeneration(scope)
|
||||
const prefix = `${scope}${KEY_SEPARATOR}`
|
||||
for (const key of entries.keys()) {
|
||||
if (key.startsWith(prefix)) {
|
||||
entries.delete(key)
|
||||
}
|
||||
}
|
||||
for (const key of failureBackoff.keys()) {
|
||||
if (key.startsWith(prefix)) {
|
||||
failureBackoff.delete(key)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** @internal - exposed for tests only */
|
||||
export function __resetHostedReviewBranchCacheForTests(): void {
|
||||
entries.clear()
|
||||
inflight.clear()
|
||||
failureBackoff.clear()
|
||||
activeClaims.clear()
|
||||
scopeGenerations.clear()
|
||||
}
|
||||
|
||||
/**
|
||||
* Serves `lookup` through the shared cache: a fresh answer is reused, concurrent
|
||||
* callers share one in-flight lookup, and a failing branch backs off instead of
|
||||
* being re-asked at every caller's poll cadence.
|
||||
*/
|
||||
export async function withHostedReviewBranchCache(
|
||||
identity: HostedReviewBranchCacheIdentity,
|
||||
options: HostedReviewBranchCacheOptions,
|
||||
lookup: () => Promise<HostedReviewInfo | null>
|
||||
): Promise<HostedReviewInfo | null> {
|
||||
const key = hostedReviewBranchCacheKey(identity)
|
||||
const headOid = options.headOid
|
||||
if (options.active === true && noteActiveClaim(key) && entries.get(key)?.review === null) {
|
||||
// Why: switching to a worktree is the user asking whether a review exists
|
||||
// yet, so the long no-review interval must not answer on their behalf. This
|
||||
// is the cheap half of the fast tier — it costs one lookup per selection
|
||||
// rather than one per minute.
|
||||
entries.delete(key)
|
||||
}
|
||||
const active = isActiveBranch(key)
|
||||
|
||||
const cached = entries.get(key)
|
||||
if (cached && isFresh(cached, headOid, active)) {
|
||||
return cached.review
|
||||
}
|
||||
|
||||
const pending = inflight.get(key)
|
||||
if (pending) {
|
||||
return pending
|
||||
}
|
||||
|
||||
const until = backoffUntil(key)
|
||||
if (until !== null) {
|
||||
// Why: a stale answer beats an error card, but with nothing cached the
|
||||
// caller must hear the failure rather than read it as "no review".
|
||||
if (cached) {
|
||||
return cached.review
|
||||
}
|
||||
throw new Error(
|
||||
`Hosted review lookup is backing off after repeated failures. Retrying after ${new Date(
|
||||
until
|
||||
).toLocaleTimeString()}.`
|
||||
)
|
||||
}
|
||||
|
||||
const scope = repoScope(identity.repoPath, identity.connectionId)
|
||||
const request = (async () => {
|
||||
const generation = scopeGeneration(scope)
|
||||
try {
|
||||
const review = await lookup()
|
||||
// Why: a review created while this lookup was out makes its answer older
|
||||
// than the invalidation; storing it would re-pin the stale "no review".
|
||||
if (generation === scopeGeneration(scope)) {
|
||||
storeEntry(key, { review, fetchedAt: Date.now(), headOid })
|
||||
failureBackoff.delete(key)
|
||||
}
|
||||
return review
|
||||
} catch (error) {
|
||||
noteFailure(key)
|
||||
// Why: the last good review beats an error card here just as it does on
|
||||
// the backed-off path — otherwise it blinks out on the first failure.
|
||||
// An invalidation drops the entry, so this cannot revive a retired answer.
|
||||
const stale = entries.get(key)
|
||||
if (stale) {
|
||||
return stale.review
|
||||
}
|
||||
throw error
|
||||
} finally {
|
||||
inflight.delete(key)
|
||||
}
|
||||
})()
|
||||
inflight.set(key, request)
|
||||
return request
|
||||
}
|
||||
|
|
@ -403,7 +403,10 @@ describe('createHostedReview', () => {
|
|||
expect.objectContaining({
|
||||
repoPath: '/repo',
|
||||
branch: 'feature',
|
||||
localGitExecOptions: { wslDistro: 'Ubuntu' }
|
||||
localGitExecOptions: { wslDistro: 'Ubuntu' },
|
||||
// Why: a stale no-review answer here would leave Create enabled after a
|
||||
// review was opened outside Orca, so eligibility takes the fast tier.
|
||||
active: true
|
||||
})
|
||||
)
|
||||
expect(ghExecFileAsyncMock).toHaveBeenCalledWith(
|
||||
|
|
|
|||
|
|
@ -36,6 +36,7 @@ import {
|
|||
} from '../gitlab/gl-utils'
|
||||
import { getSshGitProvider } from '../providers/ssh-git-dispatch'
|
||||
import { detectHostedReviewProvider, getForgeProviderForRepository } from './forge-provider'
|
||||
import { invalidateHostedReviewBranchCache } from './hosted-review-branch-cache'
|
||||
import { getHostedReviewForBranch } from './hosted-review'
|
||||
import {
|
||||
getHostedReviewLocalGitOptions,
|
||||
|
|
@ -503,6 +504,10 @@ export async function getHostedReviewCreationEligibility(
|
|||
linkedAzureDevOpsPR: args.linkedAzureDevOpsPR ?? null,
|
||||
linkedGiteaPR: args.linkedGiteaPR ?? null,
|
||||
connectionId: args.connectionId ?? null,
|
||||
// Why: eligibility is only ever asked for the worktree the user is acting
|
||||
// on, so it earns the fast tier. Without it a review opened outside Orca
|
||||
// in the last no-review interval would leave Create enabled (#11532).
|
||||
active: true,
|
||||
...hostedReviewExecutionContext(args)
|
||||
})
|
||||
} catch (error) {
|
||||
|
|
@ -624,7 +629,14 @@ export async function createHostedReview(
|
|||
return blocked
|
||||
}
|
||||
const localGitOptions = getHostedReviewLocalGitOptions(options)
|
||||
return Object.keys(localGitOptions).length > 0
|
||||
? provider.createReview(repoPath, input, connectionId, options)
|
||||
: provider.createReview(repoPath, input, connectionId)
|
||||
const result =
|
||||
Object.keys(localGitOptions).length > 0
|
||||
? await provider.createReview(repoPath, input, connectionId, options)
|
||||
: await provider.createReview(repoPath, input, connectionId)
|
||||
if (result.ok) {
|
||||
// Why (#11532): the branch cache holds a "no review" answer for far longer
|
||||
// than a poll interval, so Orca's own creation must retire it at once.
|
||||
invalidateHostedReviewBranchCache(repoPath, connectionId)
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,42 @@
|
|||
/**
|
||||
* Shared pacing policy for hosted-review lookups (#11532).
|
||||
*
|
||||
* The PR refresh coordinator's queue and the `hostedReview:forBranch` entry
|
||||
* point spend the same per-user API quota, so they must agree on how long an
|
||||
* answer stays good and how hard to back off after a failure. Keeping the
|
||||
* numbers here stops the two paths from drifting apart.
|
||||
*/
|
||||
|
||||
/** A branch with no review only gains one when a review is opened, which is not a per-minute event. */
|
||||
export const NO_REVIEW_REFRESH_INTERVAL_MS = 15 * 60_000
|
||||
|
||||
/**
|
||||
* The worktree the user has selected is re-checked at the old cadence: a review
|
||||
* opened in a browser should show up while they are still looking at the tab.
|
||||
*/
|
||||
export const ACTIVE_REFRESH_INTERVAL_MS = 60_000
|
||||
|
||||
/**
|
||||
* Why: the fast tier is only affordable because it is O(1) — one selected
|
||||
* worktree per client. Capping it here means a caller that wrongly marks a whole
|
||||
* list active costs stale cards, not the 5,000/hr budget.
|
||||
*/
|
||||
export const MAX_ACTIVE_BRANCHES = 8
|
||||
|
||||
/** A selection nobody has re-asserted for a whole no-review interval is not current. */
|
||||
export const ACTIVE_CLAIM_TTL_MS = NO_REVIEW_REFRESH_INTERVAL_MS
|
||||
|
||||
export const LOOKUP_BACKOFF_BASE_MS = 60_000
|
||||
export const LOOKUP_BACKOFF_MAX_MS = 15 * 60_000
|
||||
|
||||
// Why: capped so a long-lived failure settles at LOOKUP_BACKOFF_MAX_MS rather
|
||||
// than overflowing the exponent.
|
||||
const MAX_BACKOFF_DOUBLINGS = 4
|
||||
|
||||
/** Exponential backoff delay for the nth consecutive lookup failure (1-based). */
|
||||
export function lookupBackoffDelayMs(failures: number): number {
|
||||
return Math.min(
|
||||
LOOKUP_BACKOFF_MAX_MS,
|
||||
LOOKUP_BACKOFF_BASE_MS * 2 ** Math.min(Math.max(failures, 1) - 1, MAX_BACKOFF_DOUBLINGS)
|
||||
)
|
||||
}
|
||||
|
|
@ -1,4 +1,6 @@
|
|||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { getHostedReviewForBranch } from './hosted-review'
|
||||
import { __resetHostedReviewBranchCacheForTests } from './hosted-review-branch-cache'
|
||||
|
||||
const {
|
||||
getProjectSlugMock,
|
||||
|
|
@ -36,6 +38,7 @@ vi.mock('../gitlab/client', () => ({
|
|||
vi.mock('../github/client', () => ({
|
||||
getRepoSlug: getRepoSlugMock,
|
||||
getPRForBranchOutcome: getPRForBranchOutcomeMock,
|
||||
getGitHubPRLookupRateLimitBlock: vi.fn(async () => null),
|
||||
createGitHubPullRequest: vi.fn()
|
||||
}))
|
||||
|
||||
|
|
@ -66,8 +69,6 @@ vi.mock('../gitea/client', () => ({
|
|||
getGiteaPullRequest: vi.fn()
|
||||
}))
|
||||
|
||||
import { getHostedReviewForBranch } from './hosted-review'
|
||||
|
||||
describe('getHostedReviewForBranch', () => {
|
||||
beforeEach(() => {
|
||||
getProjectSlugMock.mockReset()
|
||||
|
|
@ -80,6 +81,9 @@ describe('getHostedReviewForBranch', () => {
|
|||
getAzureDevOpsPullRequestForBranchMock.mockReset()
|
||||
getGiteaRepoSlugMock.mockReset()
|
||||
getGiteaPullRequestForBranchMock.mockReset()
|
||||
// The branch cache is process-wide, so one test's answer would otherwise
|
||||
// satisfy the next one's lookup.
|
||||
__resetHostedReviewBranchCacheForTests()
|
||||
})
|
||||
|
||||
it('maps GitLab merge requests into the hosted review surface', async () => {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import type { HostedReviewInfo } from '../../shared/hosted-review'
|
||||
import { getForgeProviderForRepository, type ForgeProviderId } from './forge-provider'
|
||||
import { withHostedReviewBranchCache } from './hosted-review-branch-cache'
|
||||
import type { HostedReviewExecutionOptions } from './hosted-review-git-options'
|
||||
|
||||
function reviewLinkForProvider(
|
||||
|
|
@ -35,6 +36,11 @@ export async function getHostedReviewForBranch(
|
|||
linkedAzureDevOpsPR?: number | null
|
||||
linkedGiteaPR?: number | null
|
||||
currentHeadOid?: string | null
|
||||
/**
|
||||
* Set by surfaces that only ever render the selected worktree, which is the
|
||||
* one branch cheap enough to re-check per minute (#11532).
|
||||
*/
|
||||
active?: boolean
|
||||
} & HostedReviewExecutionOptions
|
||||
): Promise<HostedReviewInfo | null> {
|
||||
const branchName = input.branch.replace(/^refs\/heads\//, '')
|
||||
|
|
@ -52,20 +58,29 @@ export async function getHostedReviewForBranch(
|
|||
return null
|
||||
}
|
||||
|
||||
const provider = await getForgeProviderForRepository({
|
||||
repoPath: input.repoPath,
|
||||
connectionId: input.connectionId,
|
||||
...(input.localGitExecOptions ? { localGitExecOptions: input.localGitExecOptions } : {})
|
||||
})
|
||||
if (!provider) {
|
||||
return null
|
||||
}
|
||||
return provider.getReviewForBranch({
|
||||
repoPath: input.repoPath,
|
||||
connectionId: input.connectionId,
|
||||
branch: branchName,
|
||||
...(input.localGitExecOptions ? { localGitExecOptions: input.localGitExecOptions } : {}),
|
||||
githubCurrentHeadOid: input.currentHeadOid ?? null,
|
||||
...reviewLinkForProvider(input, provider.id)
|
||||
})
|
||||
const headOid = input.currentHeadOid?.trim() || null
|
||||
// Why (#11532): every client polls this one entry point, and they share the
|
||||
// host's per-user API quota, so the cache has to sit above the provider call.
|
||||
return withHostedReviewBranchCache(
|
||||
{ ...input, branch: branchName },
|
||||
{ headOid, ...(input.active === true ? { active: true } : {}) },
|
||||
async () => {
|
||||
const provider = await getForgeProviderForRepository({
|
||||
repoPath: input.repoPath,
|
||||
connectionId: input.connectionId,
|
||||
...(input.localGitExecOptions ? { localGitExecOptions: input.localGitExecOptions } : {})
|
||||
})
|
||||
if (!provider) {
|
||||
return null
|
||||
}
|
||||
return provider.getReviewForBranch({
|
||||
repoPath: input.repoPath,
|
||||
connectionId: input.connectionId,
|
||||
branch: branchName,
|
||||
...(input.localGitExecOptions ? { localGitExecOptions: input.localGitExecOptions } : {}),
|
||||
githubCurrentHeadOid: headOid,
|
||||
...reviewLinkForProvider(input, provider.id)
|
||||
})
|
||||
}
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1400,7 +1400,10 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
linkedBitbucketPR,
|
||||
linkedAzureDevOpsPR,
|
||||
linkedGiteaPR,
|
||||
staleWhileRevalidate: true
|
||||
staleWhileRevalidate: true,
|
||||
// Why: this panel only ever renders the selected worktree, so it earns
|
||||
// the host's fast re-check tier (#11532).
|
||||
active: true
|
||||
})
|
||||
// Why: the gh-based refresh coordinator is GitHub-only; running it elsewhere gave a spurious gh_unavailable error hiding a valid composer.
|
||||
if (activeWorktreeId && isGitHubReviewContext) {
|
||||
|
|
|
|||
|
|
@ -1721,7 +1721,10 @@ function SourceControlInner(): React.JSX.Element {
|
|||
linkedBitbucketPR,
|
||||
linkedAzureDevOpsPR,
|
||||
linkedGiteaPR,
|
||||
staleWhileRevalidate: true
|
||||
staleWhileRevalidate: true,
|
||||
// Why: scoped to the active worktree, so it earns the host's fast
|
||||
// re-check tier instead of the O(N) card pacing (#11532).
|
||||
active: true
|
||||
})
|
||||
// Why: keep the GitHub cache refresh behind the coordinator so Source Control doesn't bypass pacing.
|
||||
enqueueGitHubPRRefresh(activeWorktreeId, 'swr', 30)
|
||||
|
|
|
|||
|
|
@ -32,6 +32,11 @@ type FetchOptions = {
|
|||
repoId?: string
|
||||
staleWhileRevalidate?: boolean
|
||||
currentHeadOid?: string | null
|
||||
/**
|
||||
* Pass from surfaces that only render the selected worktree. The host re-checks
|
||||
* that branch per minute and paces the O(N) card list far slower (#11532).
|
||||
*/
|
||||
active?: boolean
|
||||
}
|
||||
type CreateHostedReviewStoreInput = CreateHostedReviewInput & { repoId?: string | null }
|
||||
|
||||
|
|
@ -387,6 +392,7 @@ export const createHostedReviewSlice: StateCreator<AppState, [], [], HostedRevie
|
|||
branch,
|
||||
...(options?.repoId !== undefined ? { repoId: options.repoId } : {}),
|
||||
currentHeadOid: options?.currentHeadOid ?? null,
|
||||
...(options?.active === true ? { active: true } : {}),
|
||||
linkedGitHubPR: options?.linkedGitHubPR ?? null,
|
||||
...(fallbackGitHubPR !== null ? { fallbackGitHubPR } : {}),
|
||||
linkedGitLabMR: options?.linkedGitLabMR ?? null,
|
||||
|
|
|
|||
|
|
@ -50,6 +50,12 @@ export type HostedReviewForBranchArgs = {
|
|||
linkedGiteaPR?: number | null
|
||||
// The worktree's checked-out HEAD oid (GitHub merged-at-head visibility).
|
||||
currentHeadOid?: string | null
|
||||
/**
|
||||
* Set only by surfaces scoped to the selected worktree. That tier is O(1), so
|
||||
* the host re-checks it per minute; the worktree list is O(N) and is paced far
|
||||
* more slowly to stay inside the shared API budget (#11532).
|
||||
*/
|
||||
active?: boolean
|
||||
}
|
||||
|
||||
export type HostedReviewSummary = {
|
||||
|
|
|
|||
Loading…
Reference in New Issue