perf(source-control): cache PR conflict-summary derivation and throttle base fetch (#7606)
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Brennan Benson
GitHub
parent
e33f31689b
commit
46a67cb2eb
|
|
@ -122,7 +122,7 @@ import {
|
|||
_resetMergeQueueCacheForTests,
|
||||
__resetTrackedUpstreamBranchCacheForTests
|
||||
} from './client'
|
||||
import { __resetPRConflictSummaryGitCapabilityCacheForTests } from './conflict-summary'
|
||||
import { __resetPRConflictSummaryCachesForTests } from './conflict-summary'
|
||||
import { resetMergedPRCommitMembershipCacheForTest } from './merged-pr-commit-membership'
|
||||
|
||||
describe('checkOrcaStarred', () => {
|
||||
|
|
@ -194,7 +194,7 @@ describe('getPRForBranch', () => {
|
|||
_resetOwnerRepoCache()
|
||||
_resetMergeQueueCacheForTests()
|
||||
__resetTrackedUpstreamBranchCacheForTests()
|
||||
__resetPRConflictSummaryGitCapabilityCacheForTests()
|
||||
__resetPRConflictSummaryCachesForTests()
|
||||
resetMergedPRCommitMembershipCacheForTest()
|
||||
})
|
||||
|
||||
|
|
@ -2987,11 +2987,16 @@ describe('getPRForBranch', () => {
|
|||
baseRefOid: 'base-oid',
|
||||
headRefOid: 'head-oid'
|
||||
}
|
||||
// Why a second head OID: identical inputs now hit the summary result
|
||||
// cache outright; a pushed head re-derives and must still skip the
|
||||
// unsupported --merge-base retry via the capability cache.
|
||||
const pushedBranchLookup = { ...branchLookup, head: { ref: 'feature/test', sha: 'head-oid-2' } }
|
||||
const pushedExactLookup = { ...exactLookup, headRefOid: 'head-oid-2' }
|
||||
ghExecFileAsyncMock
|
||||
.mockResolvedValueOnce({ stdout: JSON.stringify([branchLookup]) })
|
||||
.mockResolvedValueOnce({ stdout: JSON.stringify(exactLookup) })
|
||||
.mockResolvedValueOnce({ stdout: JSON.stringify([branchLookup]) })
|
||||
.mockResolvedValueOnce({ stdout: JSON.stringify(exactLookup) })
|
||||
.mockResolvedValueOnce({ stdout: JSON.stringify([pushedBranchLookup]) })
|
||||
.mockResolvedValueOnce({ stdout: JSON.stringify(pushedExactLookup) })
|
||||
gitExecFileAsyncMock
|
||||
.mockResolvedValueOnce({ stdout: '' })
|
||||
.mockResolvedValueOnce({ stdout: 'latest-base-oid\n' })
|
||||
|
|
@ -2999,8 +3004,6 @@ describe('getPRForBranch', () => {
|
|||
.mockResolvedValueOnce({ stdout: '2\n' })
|
||||
.mockRejectedValueOnce({ stderr: "error: unknown option `merge-base'" })
|
||||
.mockRejectedValueOnce({ stdout: 'result-tree-oid\u0000src/conflict.ts\u0000' })
|
||||
.mockResolvedValueOnce({ stdout: '' })
|
||||
.mockResolvedValueOnce({ stdout: 'latest-base-oid\n' })
|
||||
.mockResolvedValueOnce({ stdout: 'merge-base-oid\n' })
|
||||
.mockResolvedValueOnce({ stdout: '2\n' })
|
||||
.mockRejectedValueOnce({ stdout: 'result-tree-oid\u0000src/conflict.ts\u0000' })
|
||||
|
|
@ -3567,6 +3570,7 @@ describe('GitHub GraphQL rate-limit guard', () => {
|
|||
acquireMock.mockResolvedValue(undefined)
|
||||
_resetOwnerRepoCache()
|
||||
_resetMergeQueueCacheForTests()
|
||||
__resetPRConflictSummaryCachesForTests()
|
||||
})
|
||||
|
||||
it('skips PR review-thread GraphQL fetch while preserving REST comments', async () => {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,150 @@
|
|||
import type { PRConflictSummary } from '../../shared/types'
|
||||
|
||||
// Why 60s: the hottest coordinator cadences that re-derive a CONFLICTING PR
|
||||
// (10s mergeability-pending, 2.5s manual-pending) previously each ran a
|
||||
// network fetch; one fetch per base branch per minute matches the 60s minimum
|
||||
// background refresh cadence, so the tracked base tip is never staler than the
|
||||
// PR data around it. Manual refresh intentionally shares the window: GitHub's
|
||||
// own mergeability recompute is async too, and the card self-corrects within
|
||||
// a minute.
|
||||
export const CONFLICT_SUMMARY_BASE_FETCH_WINDOW_MS = 60_000
|
||||
|
||||
// Why bounded: the main process is long-lived; cap the maps so repos and PRs
|
||||
// that stop refreshing can't accumulate entries forever.
|
||||
const BASE_OID_CACHE_MAX = 64
|
||||
const SUMMARY_CACHE_MAX = 128
|
||||
|
||||
export type FreshBaseTipResolution =
|
||||
| { kind: 'resolved'; oid: string }
|
||||
| { kind: 'fallback-unresolved' }
|
||||
|
||||
type CachedBaseTipResolution = {
|
||||
oid: string | null
|
||||
resolvedAt: number
|
||||
}
|
||||
|
||||
type CachedSummary = {
|
||||
value: PRConflictSummary | undefined
|
||||
// Why: a successful summary is a pure function of two immutable commit OIDs
|
||||
// and never goes stale; a failed derivation depends on which objects exist
|
||||
// locally, which the next fetch window can change, so it carries an expiry.
|
||||
staleAt: number | null
|
||||
}
|
||||
|
||||
const baseOidCache = new Map<string, CachedBaseTipResolution>()
|
||||
const summaryCache = new Map<string, CachedSummary>()
|
||||
const inFlightBaseOidResolves = new Map<string, Promise<FreshBaseTipResolution>>()
|
||||
const inFlightSummaryDerivations = new Map<string, Promise<PRConflictSummary | undefined>>()
|
||||
|
||||
// Why: WSL distros have their own git binary, filesystem view, and remote
|
||||
// access, so cached state must never leak across the host/distro boundary.
|
||||
export function getConflictSummaryGitRuntimeKey(wslDistro: string | undefined): string {
|
||||
return wslDistro ? `wsl:${wslDistro}` : 'local:host'
|
||||
}
|
||||
|
||||
// Why JSON: repo paths and git ref names may contain any printable joiner
|
||||
// character (git allows `|` in branch names), so a delimiter-joined key could
|
||||
// alias distinct identities onto one cache entry.
|
||||
export function buildConflictSummaryCacheKey(...parts: string[]): string {
|
||||
return JSON.stringify(parts)
|
||||
}
|
||||
|
||||
export function readFreshBaseTipResolution(baseKey: string): FreshBaseTipResolution | null {
|
||||
const entry = baseOidCache.get(baseKey)
|
||||
if (!entry) {
|
||||
return null
|
||||
}
|
||||
if (Date.now() - entry.resolvedAt >= CONFLICT_SUMMARY_BASE_FETCH_WINDOW_MS) {
|
||||
baseOidCache.delete(baseKey)
|
||||
return null
|
||||
}
|
||||
return entry.oid ? { kind: 'resolved', oid: entry.oid } : { kind: 'fallback-unresolved' }
|
||||
}
|
||||
|
||||
export function storeResolvedBaseTip(baseKey: string, oid: string): void {
|
||||
setBoundedMapEntry(baseOidCache, baseKey, { oid, resolvedAt: Date.now() }, BASE_OID_CACHE_MAX)
|
||||
}
|
||||
|
||||
export function rememberUnresolvedBaseTip(baseKey: string): void {
|
||||
setBoundedMapEntry(
|
||||
baseOidCache,
|
||||
baseKey,
|
||||
{ oid: null, resolvedAt: Date.now() },
|
||||
BASE_OID_CACHE_MAX
|
||||
)
|
||||
}
|
||||
|
||||
export function readCachedSummary(summaryKey: string): CachedSummary | null {
|
||||
const entry = summaryCache.get(summaryKey)
|
||||
if (!entry) {
|
||||
return null
|
||||
}
|
||||
if (entry.staleAt !== null && Date.now() >= entry.staleAt) {
|
||||
summaryCache.delete(summaryKey)
|
||||
return null
|
||||
}
|
||||
return entry
|
||||
}
|
||||
|
||||
export function storeCachedSummary(summaryKey: string, value: PRConflictSummary | undefined): void {
|
||||
setBoundedMapEntry(
|
||||
summaryCache,
|
||||
summaryKey,
|
||||
{
|
||||
value,
|
||||
staleAt: value === undefined ? Date.now() + CONFLICT_SUMMARY_BASE_FETCH_WINDOW_MS : null
|
||||
},
|
||||
SUMMARY_CACHE_MAX
|
||||
)
|
||||
}
|
||||
|
||||
export function dedupeBaseOidResolve(
|
||||
key: string,
|
||||
factory: () => Promise<FreshBaseTipResolution>
|
||||
): Promise<FreshBaseTipResolution> {
|
||||
return dedupeInFlight(inFlightBaseOidResolves, key, factory)
|
||||
}
|
||||
|
||||
export function dedupeSummaryDerivation(
|
||||
key: string,
|
||||
factory: () => Promise<PRConflictSummary | undefined>
|
||||
): Promise<PRConflictSummary | undefined> {
|
||||
return dedupeInFlight(inFlightSummaryDerivations, key, factory)
|
||||
}
|
||||
|
||||
function dedupeInFlight<T>(
|
||||
map: Map<string, Promise<T>>,
|
||||
key: string,
|
||||
factory: () => Promise<T>
|
||||
): Promise<T> {
|
||||
const existing = map.get(key)
|
||||
if (existing) {
|
||||
return existing
|
||||
}
|
||||
const promise = factory().finally(() => {
|
||||
map.delete(key)
|
||||
})
|
||||
map.set(key, promise)
|
||||
return promise
|
||||
}
|
||||
|
||||
function setBoundedMapEntry<K, V>(map: Map<K, V>, key: K, value: V, maxEntries: number): void {
|
||||
if (map.has(key)) {
|
||||
map.delete(key)
|
||||
}
|
||||
map.set(key, value)
|
||||
while (map.size > maxEntries) {
|
||||
const oldest = map.keys().next()
|
||||
if (oldest.done) {
|
||||
return
|
||||
}
|
||||
map.delete(oldest.value)
|
||||
}
|
||||
}
|
||||
|
||||
export function __resetPRConflictSummaryDerivationCachesForTests(): void {
|
||||
baseOidCache.clear()
|
||||
summaryCache.clear()
|
||||
inFlightBaseOidResolves.clear()
|
||||
inFlightSummaryDerivations.clear()
|
||||
}
|
||||
|
|
@ -0,0 +1,331 @@
|
|||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
// Why these tests exist: the conflict-summary derivation used to re-run a
|
||||
// network `git fetch` plus a four-subprocess chain on every PR refresh tick
|
||||
// (measured ~490 full derivations in 2.1h on one machine). They pin the
|
||||
// regression contract by counting subprocess spawns: unchanged inputs must
|
||||
// cost zero subprocesses, and the base fetch must run at most once per
|
||||
// throttle window.
|
||||
|
||||
const gitExecFileAsyncMock = vi.hoisted(() => vi.fn())
|
||||
|
||||
vi.mock('../git/runner', () => ({ gitExecFileAsync: gitExecFileAsyncMock }))
|
||||
|
||||
import { CONFLICT_SUMMARY_BASE_FETCH_WINDOW_MS } from './conflict-summary-cache'
|
||||
import { __resetPRConflictSummaryCachesForTests, getPRConflictSummary } from './conflict-summary'
|
||||
|
||||
type GitResult = { stdout: string }
|
||||
type GitHandler = (argv: string[]) => Promise<GitResult>
|
||||
|
||||
const defaultHandlers: Record<string, GitHandler> = {
|
||||
fetch: async () => ({ stdout: '' }),
|
||||
'rev-parse': async () => ({ stdout: 'base-tip-1\n' }),
|
||||
'merge-base': async () => ({ stdout: 'merge-base-1\n' }),
|
||||
'rev-list': async () => ({ stdout: '3\n' }),
|
||||
'merge-tree': async () => ({ stdout: 'tree-oid\u0000src/conflict.ts\u0000' })
|
||||
}
|
||||
|
||||
function mockGitDispatch(overrides: Record<string, GitHandler> = {}): void {
|
||||
gitExecFileAsyncMock.mockImplementation((argv: string[]) => {
|
||||
const handler = overrides[argv[0]] ?? defaultHandlers[argv[0]]
|
||||
if (!handler) {
|
||||
return Promise.reject(new Error(`unexpected git command: ${argv.join(' ')}`))
|
||||
}
|
||||
return handler(argv)
|
||||
})
|
||||
}
|
||||
|
||||
function spawnCount(command?: string): number {
|
||||
const calls = gitExecFileAsyncMock.mock.calls
|
||||
if (!command) {
|
||||
return calls.length
|
||||
}
|
||||
return calls.filter(([argv]) => Array.isArray(argv) && argv[0] === command).length
|
||||
}
|
||||
|
||||
const expectedSummary = {
|
||||
baseRef: 'main',
|
||||
baseCommit: 'base-ti',
|
||||
commitsBehind: 3,
|
||||
files: ['src/conflict.ts']
|
||||
}
|
||||
|
||||
function deriveSummary(headRefOid = 'head-oid-1', wslDistro?: string) {
|
||||
return getPRConflictSummary(
|
||||
'/repo-root',
|
||||
'main',
|
||||
'github-base-oid',
|
||||
headRefOid,
|
||||
wslDistro ? { wslDistro } : {}
|
||||
)
|
||||
}
|
||||
|
||||
describe('getPRConflictSummary caching', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
vi.setSystemTime(1_750_000_000_000)
|
||||
gitExecFileAsyncMock.mockReset()
|
||||
__resetPRConflictSummaryCachesForTests()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('runs exactly one fetch and one derivation chain, then zero subprocesses on repeat calls', async () => {
|
||||
mockGitDispatch()
|
||||
|
||||
const first = await deriveSummary()
|
||||
expect(first).toEqual(expectedSummary)
|
||||
expect(spawnCount('fetch')).toBe(1)
|
||||
expect(spawnCount()).toBe(5)
|
||||
|
||||
const second = await deriveSummary()
|
||||
const third = await deriveSummary()
|
||||
expect(second).toEqual(expectedSummary)
|
||||
expect(third).toEqual(expectedSummary)
|
||||
expect(spawnCount()).toBe(5)
|
||||
})
|
||||
|
||||
it('re-derives when headRefOid changes without re-fetching inside the throttle window', async () => {
|
||||
mockGitDispatch()
|
||||
|
||||
await deriveSummary('head-oid-1')
|
||||
const afterPush = await deriveSummary('head-oid-2')
|
||||
|
||||
expect(afterPush).toEqual(expectedSummary)
|
||||
expect(spawnCount('fetch')).toBe(1)
|
||||
expect(spawnCount('rev-parse')).toBe(1)
|
||||
expect(spawnCount('merge-base')).toBe(2)
|
||||
expect(spawnCount('rev-list')).toBe(2)
|
||||
expect(spawnCount('merge-tree')).toBe(2)
|
||||
})
|
||||
|
||||
it('fetches again after the throttle window and skips derivation when the base tip is unchanged', async () => {
|
||||
mockGitDispatch()
|
||||
|
||||
await deriveSummary()
|
||||
vi.setSystemTime(1_750_000_000_000 + CONFLICT_SUMMARY_BASE_FETCH_WINDOW_MS + 1_000)
|
||||
const afterExpiry = await deriveSummary()
|
||||
|
||||
expect(afterExpiry).toEqual(expectedSummary)
|
||||
expect(spawnCount('fetch')).toBe(2)
|
||||
expect(spawnCount('rev-parse')).toBe(2)
|
||||
// Why: same (headRefOid, latestBaseOid) pair — the summary cache still hits.
|
||||
expect(spawnCount('merge-base')).toBe(1)
|
||||
expect(spawnCount('rev-list')).toBe(1)
|
||||
expect(spawnCount('merge-tree')).toBe(1)
|
||||
})
|
||||
|
||||
it('re-derives after the throttle window when the base tip moved', async () => {
|
||||
mockGitDispatch()
|
||||
await deriveSummary()
|
||||
|
||||
vi.setSystemTime(1_750_000_000_000 + CONFLICT_SUMMARY_BASE_FETCH_WINDOW_MS + 1_000)
|
||||
mockGitDispatch({
|
||||
'rev-parse': async () => ({ stdout: 'base-tip-2\n' }),
|
||||
'rev-list': async () => ({ stdout: '5\n' })
|
||||
})
|
||||
const afterBaseMove = await deriveSummary()
|
||||
|
||||
expect(afterBaseMove).toEqual({ ...expectedSummary, baseCommit: 'base-ti', commitsBehind: 5 })
|
||||
expect(spawnCount('fetch')).toBe(2)
|
||||
expect(spawnCount('merge-base')).toBe(2)
|
||||
expect(spawnCount('rev-list')).toBe(2)
|
||||
expect(spawnCount('merge-tree')).toBe(2)
|
||||
})
|
||||
|
||||
it('dedupes concurrent identical calls onto one in-flight subprocess chain', async () => {
|
||||
let releaseFetch: ((value: GitResult) => void) | undefined
|
||||
mockGitDispatch({
|
||||
fetch: () =>
|
||||
new Promise<GitResult>((resolve) => {
|
||||
releaseFetch = resolve
|
||||
})
|
||||
})
|
||||
|
||||
const firstCall = deriveSummary()
|
||||
const secondCall = deriveSummary()
|
||||
releaseFetch?.({ stdout: '' })
|
||||
const [first, second] = await Promise.all([firstCall, secondCall])
|
||||
|
||||
expect(first).toEqual(expectedSummary)
|
||||
expect(second).toEqual(expectedSummary)
|
||||
expect(spawnCount()).toBe(5)
|
||||
})
|
||||
|
||||
it('dedupes overlapping derivations after resolving the same live base tip', async () => {
|
||||
let releaseFetch: ((value: GitResult) => void) | undefined
|
||||
mockGitDispatch({
|
||||
fetch: () =>
|
||||
new Promise<GitResult>((resolve) => {
|
||||
releaseFetch = resolve
|
||||
})
|
||||
})
|
||||
|
||||
const backgroundRefresh = getPRConflictSummary(
|
||||
'/repo-root',
|
||||
'main',
|
||||
'older-github-base-oid',
|
||||
'head-oid-1',
|
||||
{}
|
||||
)
|
||||
const manualRefresh = getPRConflictSummary(
|
||||
'/repo-root',
|
||||
'main',
|
||||
'newer-github-base-oid',
|
||||
'head-oid-1',
|
||||
{}
|
||||
)
|
||||
releaseFetch?.({ stdout: '' })
|
||||
const [backgroundSummary, manualSummary] = await Promise.all([backgroundRefresh, manualRefresh])
|
||||
|
||||
expect(backgroundSummary).toEqual(expectedSummary)
|
||||
expect(manualSummary).toEqual(expectedSummary)
|
||||
expect(spawnCount('fetch')).toBe(1)
|
||||
expect(spawnCount('rev-parse')).toBe(1)
|
||||
expect(spawnCount('merge-base')).toBe(1)
|
||||
expect(spawnCount('rev-list')).toBe(1)
|
||||
expect(spawnCount('merge-tree')).toBe(1)
|
||||
})
|
||||
|
||||
it('shares one base fetch across concurrent PRs on the same base branch', async () => {
|
||||
let releaseFetch: ((value: GitResult) => void) | undefined
|
||||
mockGitDispatch({
|
||||
fetch: () =>
|
||||
new Promise<GitResult>((resolve) => {
|
||||
releaseFetch = resolve
|
||||
})
|
||||
})
|
||||
|
||||
const prA = deriveSummary('head-oid-a')
|
||||
const prB = deriveSummary('head-oid-b')
|
||||
releaseFetch?.({ stdout: '' })
|
||||
const [summaryA, summaryB] = await Promise.all([prA, prB])
|
||||
|
||||
expect(summaryA).toEqual(expectedSummary)
|
||||
expect(summaryB).toEqual(expectedSummary)
|
||||
expect(spawnCount('fetch')).toBe(1)
|
||||
expect(spawnCount('rev-parse')).toBe(1)
|
||||
expect(spawnCount('merge-base')).toBe(2)
|
||||
})
|
||||
|
||||
it('keeps concurrent fallback base OIDs isolated per PR when the base ref is unavailable', async () => {
|
||||
let releaseFetch: ((value: GitResult) => void) | undefined
|
||||
mockGitDispatch({
|
||||
fetch: () =>
|
||||
new Promise<GitResult>((resolve) => {
|
||||
releaseFetch = resolve
|
||||
}),
|
||||
'rev-parse': () => Promise.reject(new Error('missing remote-tracking ref')),
|
||||
'rev-list': async (argv) => ({
|
||||
stdout: argv[2]?.includes('bbbb2222-base') ? '7\n' : '4\n'
|
||||
}),
|
||||
'merge-tree': async (argv) => ({
|
||||
stdout: argv.includes('bbbb2222-base')
|
||||
? 'tree-oid\u0000src/b.ts\u0000'
|
||||
: 'tree-oid\u0000src/a.ts\u0000'
|
||||
})
|
||||
})
|
||||
|
||||
const prA = getPRConflictSummary('/repo-root', 'main', 'aaaa1111-base', 'head-oid-a', {})
|
||||
const prB = getPRConflictSummary('/repo-root', 'main', 'bbbb2222-base', 'head-oid-b', {})
|
||||
releaseFetch?.({ stdout: '' })
|
||||
const [summaryA, summaryB] = await Promise.all([prA, prB])
|
||||
|
||||
expect(summaryA).toEqual({
|
||||
baseRef: 'main',
|
||||
baseCommit: 'aaaa111',
|
||||
commitsBehind: 4,
|
||||
files: ['src/a.ts']
|
||||
})
|
||||
expect(summaryB).toEqual({
|
||||
baseRef: 'main',
|
||||
baseCommit: 'bbbb222',
|
||||
commitsBehind: 7,
|
||||
files: ['src/b.ts']
|
||||
})
|
||||
expect(spawnCount('fetch')).toBe(1)
|
||||
expect(spawnCount('merge-base')).toBe(2)
|
||||
})
|
||||
|
||||
it('falls back to the local remote-tracking ref when fetch fails', async () => {
|
||||
mockGitDispatch({
|
||||
fetch: () => Promise.reject(new Error('offline'))
|
||||
})
|
||||
|
||||
const summary = await deriveSummary()
|
||||
|
||||
expect(summary).toEqual(expectedSummary)
|
||||
expect(spawnCount('fetch')).toBe(1)
|
||||
|
||||
// Why: a failed fetch attempt must also be throttled — offline machines
|
||||
// were the worst case, paying the 10s fetch timeout on every tick.
|
||||
const repeat = await deriveSummary()
|
||||
expect(repeat).toEqual(expectedSummary)
|
||||
expect(spawnCount('fetch')).toBe(1)
|
||||
expect(spawnCount()).toBe(5)
|
||||
})
|
||||
|
||||
it("falls back to GitHub's baseRefOid when fetch and remote-tracking refs are unavailable", async () => {
|
||||
mockGitDispatch({
|
||||
fetch: () => Promise.reject(new Error('offline')),
|
||||
'rev-parse': () => Promise.reject(new Error('unknown revision'))
|
||||
})
|
||||
|
||||
const summary = await deriveSummary()
|
||||
|
||||
expect(summary).toEqual({ ...expectedSummary, baseCommit: 'github-' })
|
||||
expect(
|
||||
gitExecFileAsyncMock.mock.calls.some(
|
||||
([argv]) => argv[0] === 'merge-base' && argv.includes('github-base-oid')
|
||||
)
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('keeps WSL-distro derivations isolated from host derivations', async () => {
|
||||
mockGitDispatch()
|
||||
|
||||
await deriveSummary()
|
||||
await deriveSummary('head-oid-1', 'Ubuntu')
|
||||
|
||||
expect(spawnCount('fetch')).toBe(2)
|
||||
expect(
|
||||
gitExecFileAsyncMock.mock.calls.filter(
|
||||
([argv, options]) =>
|
||||
argv[0] === 'fetch' && (options as { wslDistro?: string })?.wslDistro === 'Ubuntu'
|
||||
)
|
||||
).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('keeps identities distinct when paths or ref names contain a joiner character', async () => {
|
||||
mockGitDispatch()
|
||||
|
||||
// Why: these two calls alias under naive pipe-joined keys — the segment
|
||||
// boundary shifts between repoPath and baseRefName.
|
||||
await getPRConflictSummary('/repo|x', 'main', 'github-base-oid', 'head-oid-1', {})
|
||||
await getPRConflictSummary('/repo', 'x|main', 'github-base-oid', 'head-oid-1', {})
|
||||
|
||||
expect(spawnCount('fetch')).toBe(2)
|
||||
expect(spawnCount('merge-base')).toBe(2)
|
||||
})
|
||||
|
||||
it('caches failed derivations only until the throttle window expires', async () => {
|
||||
mockGitDispatch({
|
||||
'merge-base': () => Promise.reject(new Error('bad object'))
|
||||
})
|
||||
|
||||
await expect(deriveSummary()).resolves.toBeUndefined()
|
||||
const failureSpawns = spawnCount()
|
||||
|
||||
// Within the window the failure is negative-cached: zero subprocesses.
|
||||
await expect(deriveSummary()).resolves.toBeUndefined()
|
||||
expect(spawnCount()).toBe(failureSpawns)
|
||||
|
||||
// After expiry the derivation retries (a fetch may have brought objects in).
|
||||
vi.setSystemTime(1_750_000_000_000 + CONFLICT_SUMMARY_BASE_FETCH_WINDOW_MS + 1_000)
|
||||
mockGitDispatch()
|
||||
await expect(deriveSummary()).resolves.toEqual(expectedSummary)
|
||||
expect(spawnCount('merge-base')).toBe(2)
|
||||
})
|
||||
})
|
||||
|
|
@ -1,5 +1,17 @@
|
|||
import type { PRConflictSummary } from '../../shared/types'
|
||||
import { gitExecFileAsync } from '../git/runner'
|
||||
import {
|
||||
__resetPRConflictSummaryDerivationCachesForTests,
|
||||
buildConflictSummaryCacheKey,
|
||||
dedupeBaseOidResolve,
|
||||
dedupeSummaryDerivation,
|
||||
getConflictSummaryGitRuntimeKey,
|
||||
readCachedSummary,
|
||||
readFreshBaseTipResolution,
|
||||
rememberUnresolvedBaseTip,
|
||||
storeResolvedBaseTip,
|
||||
storeCachedSummary
|
||||
} from './conflict-summary-cache'
|
||||
|
||||
type LocalGitExecOptions = {
|
||||
wslDistro?: string
|
||||
|
|
@ -7,8 +19,9 @@ type LocalGitExecOptions = {
|
|||
|
||||
const mergeTreeMergeBaseUnsupportedRuntimes = new Set<string>()
|
||||
|
||||
export function __resetPRConflictSummaryGitCapabilityCacheForTests(): void {
|
||||
export function __resetPRConflictSummaryCachesForTests(): void {
|
||||
mergeTreeMergeBaseUnsupportedRuntimes.clear()
|
||||
__resetPRConflictSummaryDerivationCachesForTests()
|
||||
}
|
||||
|
||||
export async function getPRConflictSummary(
|
||||
|
|
@ -18,45 +31,122 @@ export async function getPRConflictSummary(
|
|||
headRefOid: string,
|
||||
localGitOptions: LocalGitExecOptions = {}
|
||||
): Promise<PRConflictSummary | undefined> {
|
||||
try {
|
||||
// Why: the renderer only needs a read-only merge-conflict snapshot. We
|
||||
// derive it from local git state so the PR card can show GitHub-style
|
||||
// detail without spending additional gh API calls on every refresh. We use
|
||||
// GitHub's head OID directly because the registered repo path may not have
|
||||
// a matching local branch name for the PR head. For the base side, prefer a
|
||||
// freshly-fetched remote-tracking ref so Orca matches GitHub's portal,
|
||||
// which compares against the latest base branch tip rather than the PR's
|
||||
// older pinned baseRefOid snapshot.
|
||||
const latestBaseOid = await resolveLatestBaseOid(
|
||||
// Why: the renderer only needs a read-only merge-conflict snapshot. We
|
||||
// derive it from local git state so the PR card can show GitHub-style
|
||||
// detail without spending additional gh API calls on every refresh. We use
|
||||
// GitHub's head OID directly because the registered repo path may not have
|
||||
// a matching local branch name for the PR head. For the base side, prefer a
|
||||
// freshly-fetched remote-tracking ref so Orca matches GitHub's portal,
|
||||
// which compares against the latest base branch tip rather than the PR's
|
||||
// older pinned baseRefOid snapshot.
|
||||
const latestBaseOid = await resolveLatestBaseOidThrottled(
|
||||
repoPath,
|
||||
baseRefName,
|
||||
baseRefOid,
|
||||
localGitOptions
|
||||
)
|
||||
// Why: the summary is a pure function of the two commit OIDs, so a key hit
|
||||
// can skip the whole merge-base/rev-list/merge-tree subprocess chain.
|
||||
const runtimeKey = getConflictSummaryGitRuntimeKey(localGitOptions.wslDistro)
|
||||
const summaryKey = buildConflictSummaryCacheKey(
|
||||
runtimeKey,
|
||||
repoPath,
|
||||
baseRefName,
|
||||
headRefOid,
|
||||
latestBaseOid
|
||||
)
|
||||
const cached = readCachedSummary(summaryKey)
|
||||
if (cached) {
|
||||
return cached.value
|
||||
}
|
||||
|
||||
// Why: different GitHub reads can report different pinned baseRefOid values
|
||||
// while still resolving to the same live base tip; dedupe the expensive
|
||||
// local derivation on the actual summary identity.
|
||||
return dedupeSummaryDerivation(summaryKey, () =>
|
||||
derivePRConflictSummary(
|
||||
repoPath,
|
||||
baseRefName,
|
||||
baseRefOid,
|
||||
headRefOid,
|
||||
latestBaseOid,
|
||||
summaryKey,
|
||||
localGitOptions
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
async function derivePRConflictSummary(
|
||||
repoPath: string,
|
||||
baseRefName: string,
|
||||
headRefOid: string,
|
||||
latestBaseOid: string,
|
||||
summaryKey: string,
|
||||
localGitOptions: LocalGitExecOptions
|
||||
): Promise<PRConflictSummary | undefined> {
|
||||
const cached = readCachedSummary(summaryKey)
|
||||
if (cached) {
|
||||
return cached.value
|
||||
}
|
||||
|
||||
try {
|
||||
const mergeBase = await resolveMergeBase(repoPath, headRefOid, latestBaseOid, localGitOptions)
|
||||
const [commitsBehind, files] = await Promise.all([
|
||||
countCommits(repoPath, `${headRefOid}..${latestBaseOid}`, localGitOptions),
|
||||
loadConflictingFiles(repoPath, mergeBase, headRefOid, latestBaseOid, localGitOptions)
|
||||
])
|
||||
|
||||
return {
|
||||
const summary = {
|
||||
baseRef: baseRefName,
|
||||
baseCommit: latestBaseOid.slice(0, 7),
|
||||
commitsBehind,
|
||||
files,
|
||||
...(files.length === 0 ? { localMergeState: 'clean' as const } : {})
|
||||
}
|
||||
storeCachedSummary(summaryKey, summary)
|
||||
return summary
|
||||
} catch {
|
||||
storeCachedSummary(summaryKey, undefined)
|
||||
return undefined
|
||||
}
|
||||
}
|
||||
|
||||
async function resolveLatestBaseOid(
|
||||
async function resolveLatestBaseOidThrottled(
|
||||
repoPath: string,
|
||||
baseRefName: string,
|
||||
fallbackBaseOid: string,
|
||||
localGitOptions: LocalGitExecOptions
|
||||
): Promise<string> {
|
||||
const runtimeKey = getConflictSummaryGitRuntimeKey(localGitOptions.wslDistro)
|
||||
const baseKey = buildConflictSummaryCacheKey(runtimeKey, repoPath, baseRefName)
|
||||
const cachedResolution = readFreshBaseTipResolution(baseKey)
|
||||
if (cachedResolution) {
|
||||
return cachedResolution.kind === 'resolved' ? cachedResolution.oid : fallbackBaseOid
|
||||
}
|
||||
return dedupeBaseOidResolve(baseKey, async () => {
|
||||
// Why re-check inside the dedupe slot: a sibling caller may have finished
|
||||
// resolving between our cache read and this factory starting.
|
||||
const freshResolution = readFreshBaseTipResolution(baseKey)
|
||||
if (freshResolution) {
|
||||
return freshResolution
|
||||
}
|
||||
const oid = await resolveLatestBaseOid(repoPath, baseRefName, localGitOptions)
|
||||
if (oid) {
|
||||
storeResolvedBaseTip(baseKey, oid)
|
||||
return { kind: 'resolved', oid }
|
||||
}
|
||||
// Why cache the unresolved probe, not the caller fallback: the fetch
|
||||
// attempt is branch-wide expensive work, but GitHub's baseRefOid is
|
||||
// PR-specific and must not leak to sibling PRs on the same base branch.
|
||||
rememberUnresolvedBaseTip(baseKey)
|
||||
return { kind: 'fallback-unresolved' }
|
||||
}).then((resolution) => (resolution.kind === 'resolved' ? resolution.oid : fallbackBaseOid))
|
||||
}
|
||||
|
||||
async function resolveLatestBaseOid(
|
||||
repoPath: string,
|
||||
baseRefName: string,
|
||||
localGitOptions: LocalGitExecOptions
|
||||
): Promise<string | null> {
|
||||
const remoteName = 'origin'
|
||||
|
||||
try {
|
||||
|
|
@ -88,7 +178,7 @@ async function resolveLatestBaseOid(
|
|||
}
|
||||
}
|
||||
|
||||
return fallbackBaseOid
|
||||
return null
|
||||
}
|
||||
|
||||
async function resolveMergeBase(
|
||||
|
|
@ -123,7 +213,7 @@ async function loadConflictingFiles(
|
|||
baseOid: string,
|
||||
localGitOptions: LocalGitExecOptions
|
||||
): Promise<string[]> {
|
||||
const capabilityKey = getMergeTreeCapabilityKey(localGitOptions)
|
||||
const capabilityKey = getConflictSummaryGitRuntimeKey(localGitOptions.wslDistro)
|
||||
const modernArgs = [
|
||||
'merge-tree',
|
||||
'--write-tree',
|
||||
|
|
@ -193,10 +283,6 @@ async function loadConflictingFilesWithLegacyMergeTree(
|
|||
}
|
||||
}
|
||||
|
||||
function getMergeTreeCapabilityKey(localGitOptions: LocalGitExecOptions): string {
|
||||
return localGitOptions.wslDistro ? `wsl:${localGitOptions.wslDistro}` : 'local:host'
|
||||
}
|
||||
|
||||
function parseMergeTreeNameOnlyOutput(stdout: string): string[] {
|
||||
const entries = stdout.split('\0').filter(Boolean)
|
||||
if (entries.length === 0) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue