mirror of
https://github.com/stablyai/orca.git
synced 2026-10-16 16:03:26 +00:00
Cache definitive non-membership answers as long as confirmations
A merged PR's commit set is frozen, so a 200 response that lacks the PR is as immutable as one that contains it — only errors (unpushed commits, network) warrant the short retry TTL. Keeps a reused-branch worktree with the checks panel open from re-probing GitHub every five minutes. Also adds the review-requested why comments on the fallback gate and the preservation seam. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -821,6 +821,15 @@ describe('getPRForBranch', () => {
|
||||
const pr = await getPRForBranch('/repo-root', 'fix-hibernation-wake')
|
||||
|
||||
expect(pr).toBeNull()
|
||||
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(3)
|
||||
|
||||
// A definitive "not part of this PR" answer is immutable for a merged PR:
|
||||
// repeated polls must not re-probe GitHub.
|
||||
mockMergedBranchPRLookupBehindHead()
|
||||
const second = await getPRForBranch('/repo-root', 'fix-hibernation-wake')
|
||||
|
||||
expect(second).toBeNull()
|
||||
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(5)
|
||||
})
|
||||
|
||||
it('keeps hiding a merged branch PR when the commit membership probe fails', async () => {
|
||||
|
||||
@@ -2950,6 +2950,10 @@ export async function getPRForBranchOutcome(
|
||||
explicitCurrentHeadOid !== null &&
|
||||
shouldHideMergedImplicitPR(data, linkedPRNumber, explicitCurrentHeadOid) &&
|
||||
!(await mergedPRContainsHead(data, dataRepo, explicitCurrentHeadOid))
|
||||
// Why no lazy-HEAD re-check on preservation: fallback numbers come from
|
||||
// callers that already gated them on head equality or confirmed
|
||||
// containment; re-hiding against the main-repo HEAD would blank
|
||||
// deleted-head merged PRs that are deliberately kept visible.
|
||||
const shouldPreserveMergedFallback =
|
||||
!explicitHeadHidesMergedImplicitPR &&
|
||||
(fallbackConfirmedMergedBranch || options.acceptMergedFallbackPR === true)
|
||||
|
||||
@@ -4,12 +4,13 @@ import type { OwnerRepo } from './github-repository-identity'
|
||||
|
||||
type GhExecOptions = Parameters<typeof ghExecFileAsync>[1]
|
||||
|
||||
// Why: a merged PR's commit set is immutable, so confirmations never expire in
|
||||
// practice; the TTLs only bound memory and let "commit not on GitHub yet"
|
||||
// answers retry after a future push.
|
||||
// Why: a merged PR's commit set is immutable, so definitive answers — member
|
||||
// or not — never change; that TTL only bounds memory. Errors (a commit not on
|
||||
// GitHub yet, network) get a short TTL so a future push can flip the answer
|
||||
// without the checks-panel poll re-probing every cycle.
|
||||
const MEMBERSHIP_CACHE_MAX_ENTRIES = 200
|
||||
const MEMBERSHIP_CONFIRMED_TTL_MS = 6 * 60 * 60 * 1000
|
||||
const MEMBERSHIP_NEGATIVE_TTL_MS = 5 * 60 * 1000
|
||||
const MEMBERSHIP_DEFINITIVE_TTL_MS = 6 * 60 * 60 * 1000
|
||||
const MEMBERSHIP_ERROR_TTL_MS = 5 * 60 * 1000
|
||||
|
||||
const membershipCache = new Map<string, { value: boolean; expiresAt: number }>()
|
||||
|
||||
@@ -80,7 +81,7 @@ export async function isCommitPartOfMergedPR(args: {
|
||||
)
|
||||
membershipCache.set(cacheKey, {
|
||||
value,
|
||||
expiresAt: now + (value ? MEMBERSHIP_CONFIRMED_TTL_MS : MEMBERSHIP_NEGATIVE_TTL_MS)
|
||||
expiresAt: now + MEMBERSHIP_DEFINITIVE_TTL_MS
|
||||
})
|
||||
return value
|
||||
} catch {
|
||||
@@ -88,7 +89,7 @@ export async function isCommitPartOfMergedPR(args: {
|
||||
// definitive "new local work" today, but a push can change it; retry later.
|
||||
membershipCache.set(cacheKey, {
|
||||
value: false,
|
||||
expiresAt: now + MEMBERSHIP_NEGATIVE_TTL_MS
|
||||
expiresAt: now + MEMBERSHIP_ERROR_TTL_MS
|
||||
})
|
||||
return false
|
||||
}
|
||||
|
||||
@@ -1038,6 +1038,9 @@ function buildPRRefreshCandidate(
|
||||
true
|
||||
)
|
||||
const cachedFallbackPRNumber = cachedPR?.number ?? null
|
||||
// Why: a merged PR stays a valid fallback when the worktree sits on its head
|
||||
// or on a commit main confirmed as part of the PR; anything else means the
|
||||
// branch moved on and the number must not resurrect the old merged PR.
|
||||
const cachedMergedPRMovedPastHead =
|
||||
worktree.linkedPR == null &&
|
||||
cachedPR?.state === 'merged' &&
|
||||
|
||||
Reference in New Issue
Block a user