Revert "fix(memory): bound OOM-prone accumulators (#10179)" (#10255)

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Neil
2026-07-23 18:35:31 -07:00
committed by GitHub
co-authored by Orca
parent 6eb70d8370
commit aab112933e
1577 changed files with 15166 additions and 91654 deletions
+59 -145
View File
@@ -34,7 +34,7 @@ import {
GITHUB_WORK_ITEMS_SSH_REMOTE_REQUIRED_MESSAGE,
sortWorkItemsByNumber
} from '../../shared/work-items'
import { mkdtemp, rm, writeFile } from 'node:fs/promises'
import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'
import { join } from 'node:path'
import { tmpdir } from 'node:os'
import { sliceCheckLogTail } from './check-job-log-tail-slice'
@@ -43,6 +43,8 @@ import {
safePRRefreshErrorMessage
} from './pr-refresh-error-classification'
import { getPRConflictSummary } from './conflict-summary'
import { getSshFilesystemProvider } from '../providers/ssh-filesystem-dispatch'
import { joinWorktreeRelativePath } from '../runtime/runtime-relative-paths'
import { splitRemoteBranchName } from '../../shared/git-effective-upstream'
import {
execFileAsync,
@@ -72,9 +74,6 @@ import {
} from '../source-control/hosted-review-git-options'
import { shouldHideNonOpenReviewOnDefaultBranch } from '../source-control/repo-default-branch'
import { readLocalGitConfigSignature } from './local-git-config-signature'
import { readHostedReviewTemplate } from '../source-control/pull-request-template'
import { cacheIdentityDigest } from '../cache-identity-digest'
import { measureUtf8ByteLength } from '../../shared/utf8-byte-limits'
import {
getGitHubApiRepositoryForRemote,
getIssueGitHubApiRepository,
@@ -1774,6 +1773,41 @@ async function findOpenPRByHeadBase(args: {
return { number: list[0].number, url: list[0].url }
}
async function readPullRequestTemplate(
repoPath: string,
connectionId?: string | null
): Promise<string> {
const relativeCandidates = [
'.github/pull_request_template.md',
'.github/PULL_REQUEST_TEMPLATE.md',
'pull_request_template.md',
'PULL_REQUEST_TEMPLATE.md',
'docs/pull_request_template.md',
'docs/PULL_REQUEST_TEMPLATE.md'
]
const remoteProvider = connectionId ? getSshFilesystemProvider(connectionId) : undefined
if (connectionId && !remoteProvider) {
return ''
}
for (const relativeCandidate of relativeCandidates) {
try {
if (remoteProvider) {
const result = await remoteProvider.readFile(
joinWorktreeRelativePath(repoPath, relativeCandidate)
)
if (result.isBinary) {
continue
}
return result.content
}
return await readFile(join(repoPath, relativeCandidate), 'utf8')
} catch {
// Try the next conventional PR template path.
}
}
return ''
}
export async function createGitHubPullRequest(
repoPath: string,
input: CreateHostedReviewInput,
@@ -1828,7 +1862,7 @@ export async function createGitHubPullRequest(
try {
const body =
input.useTemplate && !input.body?.trim()
? await readHostedReviewTemplate(repoPath, connectionId, 'github')
? await readPullRequestTemplate(repoPath, connectionId)
: (input.body ?? '')
await writeFile(bodyPath, body, 'utf8')
const createArgs = [
@@ -2182,7 +2216,7 @@ async function detectRepositoryMergeMetadata(
branchName: string | undefined,
ghOptions: GhExecOptions
): Promise<GitHubRepositoryMergeMetadata> {
const cacheKey = cacheIdentityDigest([githubRepoIdentityKey(ownerRepo), branchName ?? '__repo__'])
const cacheKey = `${githubRepoIdentityKey(ownerRepo)}:${branchName ?? '__repo__'}`
pruneRepositoryMergeMetadataCache()
const cached = repositoryMergeMetadataCache.get(cacheKey)
if (cached) {
@@ -2356,16 +2390,11 @@ type TrackedUpstreamBranch = {
const TRACKED_UPSTREAM_SNAPSHOT_CACHE_TTL_MS = 30_000
const TRACKED_UPSTREAM_SNAPSHOT_CACHE_MAX_ENTRIES = 512
export const TRACKED_UPSTREAM_SNAPSHOT_MAX_IN_FLIGHT = 32
export const TRACKED_UPSTREAM_SNAPSHOT_MAX_BRANCHES = 4096
export const TRACKED_UPSTREAM_SNAPSHOT_MAX_BYTES = 2 * 1024 * 1024
export const TRACKED_UPSTREAM_SNAPSHOT_CACHE_MAX_BYTES = 32 * 1024 * 1024
type TrackedUpstreamSnapshotCacheEntry = {
expiresAt: number
gitConfigSignature?: string
upstreamsByBranchName: Map<string, TrackedUpstreamBranch | null>
retainedBytes: number
}
type TrackedUpstreamSnapshotProbeResult = {
@@ -2373,7 +2402,6 @@ type TrackedUpstreamSnapshotProbeResult = {
gitConfigSignature?: string
probeFailed: boolean
upstreamsByBranchName: Map<string, TrackedUpstreamBranch | null>
retainedBytes: number
}
const trackedUpstreamSnapshotCache = new Map<string, TrackedUpstreamSnapshotCacheEntry>()
@@ -2382,16 +2410,6 @@ const trackedUpstreamSnapshotInFlight = new Map<
Promise<TrackedUpstreamSnapshotProbeResult>
>()
const trackedUpstreamSnapshotGenerations = new Map<string, symbol>()
let trackedUpstreamSnapshotCacheBytes = 0
function deleteTrackedUpstreamSnapshot(cacheKey: string): void {
const cached = trackedUpstreamSnapshotCache.get(cacheKey)
if (!cached) {
return
}
trackedUpstreamSnapshotCacheBytes -= cached.retainedBytes
trackedUpstreamSnapshotCache.delete(cacheKey)
}
function beginTrackedUpstreamSnapshotProbe(cacheKey: string): symbol {
const generation = Symbol()
@@ -2409,19 +2427,16 @@ function finishTrackedUpstreamSnapshotProbe(cacheKey: string, generation: symbol
function pruneTrackedUpstreamSnapshotCache(now: number): void {
for (const [cacheKey, cached] of trackedUpstreamSnapshotCache) {
if (cached.expiresAt <= now) {
deleteTrackedUpstreamSnapshot(cacheKey)
trackedUpstreamSnapshotCache.delete(cacheKey)
}
}
// Why: workspace/runtime churn can create unbounded unique keys within one TTL window, so expiry sweeping alone isn't a memory bound.
while (
trackedUpstreamSnapshotCache.size > TRACKED_UPSTREAM_SNAPSHOT_CACHE_MAX_ENTRIES ||
trackedUpstreamSnapshotCacheBytes > TRACKED_UPSTREAM_SNAPSHOT_CACHE_MAX_BYTES
) {
while (trackedUpstreamSnapshotCache.size > TRACKED_UPSTREAM_SNAPSHOT_CACHE_MAX_ENTRIES) {
const oldestKey = trackedUpstreamSnapshotCache.keys().next().value
if (oldestKey === undefined) {
break
}
deleteTrackedUpstreamSnapshot(oldestKey)
trackedUpstreamSnapshotCache.delete(oldestKey)
}
}
@@ -2441,7 +2456,6 @@ export function __resetTrackedUpstreamBranchCacheForTests(): void {
trackedUpstreamSnapshotCache.clear()
trackedUpstreamSnapshotInFlight.clear()
trackedUpstreamSnapshotGenerations.clear()
trackedUpstreamSnapshotCacheBytes = 0
}
function parseTrackedUpstreamBranch(upstreamRef: string): TrackedUpstreamBranch | null {
@@ -2449,10 +2463,7 @@ function parseTrackedUpstreamBranch(upstreamRef: string): TrackedUpstreamBranch
if (!parsed) {
return null
}
return {
remoteName: parsed.remoteName.replace(/$/u, ''),
branchName: parsed.branchName.replace(/$/u, '')
}
return parsed
}
function shouldRetryTrackedUpstreamBranch(
@@ -2493,10 +2504,10 @@ async function getTrackedUpstreamBranch(
) {
return cached.upstreamsByBranchName.get(branchName) ?? null
}
deleteTrackedUpstreamSnapshot(cacheKey)
trackedUpstreamSnapshotCache.delete(cacheKey)
}
if (cached) {
deleteTrackedUpstreamSnapshot(cacheKey)
trackedUpstreamSnapshotCache.delete(cacheKey)
}
const inFlight = trackedUpstreamSnapshotInFlight.get(cacheKey)
@@ -2513,31 +2524,18 @@ async function getTrackedUpstreamBranch(
}
}
if (trackedUpstreamSnapshotInFlight.size >= TRACKED_UPSTREAM_SNAPSHOT_MAX_IN_FLIGHT) {
const result = await probeTrackedUpstreamSnapshot(
repoPath,
connectionId,
localGitOptions,
branchName
)
return result.upstreamsByBranchName.get(branchName) ?? null
}
// Why: PR polling asks about hundreds of branches at once; read all upstreams in one git process per repo/runtime, not one probe per branch.
const probeGeneration = beginTrackedUpstreamSnapshotProbe(cacheKey)
const probe = probeTrackedUpstreamSnapshot(repoPath, connectionId, localGitOptions, branchName)
const probe = probeTrackedUpstreamSnapshot(repoPath, connectionId, localGitOptions)
trackedUpstreamSnapshotInFlight.set(cacheKey, probe)
try {
const result = await probe
if (result.cacheable && trackedUpstreamSnapshotGenerations.get(cacheKey) === probeGeneration) {
deleteTrackedUpstreamSnapshot(cacheKey)
trackedUpstreamSnapshotCache.set(cacheKey, {
...(result.gitConfigSignature ? { gitConfigSignature: result.gitConfigSignature } : {}),
upstreamsByBranchName: getCacheableTrackedUpstreamSnapshot(result.upstreamsByBranchName),
retainedBytes: result.retainedBytes,
expiresAt: Date.now() + TRACKED_UPSTREAM_SNAPSHOT_CACHE_TTL_MS
})
trackedUpstreamSnapshotCacheBytes += result.retainedBytes
pruneTrackedUpstreamSnapshotCache(Date.now())
}
if (trackedUpstreamSnapshotGenerations.get(cacheKey) !== probeGeneration) {
@@ -2558,8 +2556,7 @@ async function getTrackedUpstreamBranch(
async function probeTrackedUpstreamSnapshot(
repoPath: string,
connectionId?: string | null,
localGitOptions: { wslDistro?: string } = {},
requestedBranchName?: string
localGitOptions: { wslDistro?: string } = {}
): Promise<TrackedUpstreamSnapshotProbeResult> {
const startingGitConfigSignature = await readLocalGitConfigSignature({
repoPath,
@@ -2569,10 +2566,8 @@ async function probeTrackedUpstreamSnapshot(
const { probeFailed, upstreamsByBranchName } = await probeTrackedUpstreamBranches(
repoPath,
connectionId,
localGitOptions,
requestedBranchName
localGitOptions
)
const retainedBytes = measureTrackedUpstreamSnapshotBytes(upstreamsByBranchName)
const endingGitConfigSignature = await readLocalGitConfigSignature({
repoPath,
connectionId: connectionId ?? null,
@@ -2588,8 +2583,7 @@ async function probeTrackedUpstreamSnapshot(
cacheable: !configSignatureChanged && !probeFailed,
probeFailed,
...(gitConfigSignature ? { gitConfigSignature } : {}),
upstreamsByBranchName,
retainedBytes
upstreamsByBranchName
}
}
@@ -2632,14 +2626,13 @@ function getTrackedUpstreamBranchCacheKey(
const runtimeKey = connectionId
? `ssh:${connectionId}`
: `local:${localGitOptions.wslDistro ?? 'host'}`
return cacheIdentityDigest([runtimeKey, repoPath])
return [runtimeKey, repoPath].join('\0')
}
async function probeTrackedUpstreamBranches(
repoPath: string,
connectionId?: string | null,
localGitOptions: { wslDistro?: string } = {},
requestedBranchName?: string
localGitOptions: { wslDistro?: string } = {}
): Promise<{
probeFailed: boolean
upstreamsByBranchName: Map<string, TrackedUpstreamBranch | null>
@@ -2655,106 +2648,29 @@ async function probeTrackedUpstreamBranches(
})
return {
probeFailed: false,
upstreamsByBranchName: parseTrackedUpstreamBranches(result.stdout, requestedBranchName)
upstreamsByBranchName: parseTrackedUpstreamBranches(result.stdout)
}
} catch {
return { probeFailed: true, upstreamsByBranchName: new Map() }
}
}
function parseTrackedUpstreamBranches(
stdout: string,
requestedBranchName?: string
): Map<string, TrackedUpstreamBranch | null> {
function parseTrackedUpstreamBranches(stdout: string): Map<string, TrackedUpstreamBranch | null> {
const upstreamsByBranchName = new Map<string, TrackedUpstreamBranch | null>()
let retainedBytes = 0
let lineStart = 0
while (lineStart <= stdout.length) {
const newline = stdout.indexOf('\n', lineStart)
const lineEnd = newline === -1 ? stdout.length : newline
const line = stdout.slice(
lineStart,
lineEnd > lineStart && stdout[lineEnd - 1] === '\r' ? lineEnd - 1 : lineEnd
)
lineStart = newline === -1 ? stdout.length + 1 : newline + 1
for (const line of stdout.split(/\r?\n/)) {
if (!line) {
continue
}
const separator = line.indexOf('\0')
const branchName = separator === -1 ? line : line.slice(0, separator)
const upstreamRef = separator === -1 ? '' : line.slice(separator + 1)
const localBranchName = branchName.startsWith('refs/heads/')
? branchName.slice('refs/heads/'.length)
: branchName
const [branchName, upstreamRef] = line.split('\0')
const localBranchName = branchName?.replace(/^refs\/heads\//, '')
if (!localBranchName) {
continue
}
const parsedUpstream = parseTrackedUpstreamRef(upstreamRef)
const entryBytes = measureTrackedUpstreamEntryBytes(localBranchName, parsedUpstream)
if (entryBytes === null) {
continue
}
const isRequested = localBranchName === requestedBranchName
while (
isRequested &&
upstreamsByBranchName.size > 0 &&
(upstreamsByBranchName.size >= TRACKED_UPSTREAM_SNAPSHOT_MAX_BRANCHES ||
retainedBytes + entryBytes > TRACKED_UPSTREAM_SNAPSHOT_MAX_BYTES)
) {
const oldest = upstreamsByBranchName.keys().next().value
if (oldest === undefined) {
break
}
retainedBytes -=
measureTrackedUpstreamEntryBytes(oldest, upstreamsByBranchName.get(oldest) ?? null) ?? 0
upstreamsByBranchName.delete(oldest)
}
if (
upstreamsByBranchName.size >= TRACKED_UPSTREAM_SNAPSHOT_MAX_BRANCHES ||
retainedBytes + entryBytes > TRACKED_UPSTREAM_SNAPSHOT_MAX_BYTES
) {
continue
}
upstreamsByBranchName.set(localBranchName.replace(/$/u, ''), parsedUpstream)
retainedBytes += entryBytes
upstreamsByBranchName.set(localBranchName, parseTrackedUpstreamRef(upstreamRef ?? ''))
}
return upstreamsByBranchName
}
export function _parseTrackedUpstreamBranchesForTests(
stdout: string,
requestedBranchName?: string
): Map<string, TrackedUpstreamBranch | null> {
return parseTrackedUpstreamBranches(stdout, requestedBranchName)
}
function measureTrackedUpstreamEntryBytes(
branchName: string,
upstream: TrackedUpstreamBranch | null
): number | null {
let remainingBytes = TRACKED_UPSTREAM_SNAPSHOT_MAX_BYTES - 64
let bytes = 64
for (const value of [branchName, upstream?.remoteName ?? '', upstream?.branchName ?? '']) {
const measured = measureUtf8ByteLength(value, { stopAfterBytes: remainingBytes })
if (measured.exceededLimit) {
return null
}
remainingBytes -= measured.byteLength
bytes += measured.byteLength
}
return bytes <= TRACKED_UPSTREAM_SNAPSHOT_MAX_BYTES ? bytes : null
}
function measureTrackedUpstreamSnapshotBytes(
upstreamsByBranchName: Map<string, TrackedUpstreamBranch | null>
): number {
let bytes = 0
for (const [branchName, upstream] of upstreamsByBranchName) {
bytes += measureTrackedUpstreamEntryBytes(branchName, upstream) ?? 0
}
return bytes
}
function parseTrackedUpstreamRef(upstreamRef: string): TrackedUpstreamBranch | null {
const remoteRefPrefix = 'refs/remotes/'
const normalizedRef = upstreamRef.trim()
@@ -3871,9 +3787,7 @@ async function attachFailedJobLogTails(
// Why: cap log fetches so failed-job details stay a bounded follow-up, not a burst of hosted log downloads.
for (const job of failedJobs) {
const jobCacheKey = getCheckJobLogTailCacheKey(job)
const cacheKey = jobCacheKey
? cacheIdentityDigest([githubRepoIdentityKey(ownerRepo), jobCacheKey])
: null
const cacheKey = jobCacheKey ? `${githubRepoIdentityKey(ownerRepo)}:${jobCacheKey}` : null
if (!cacheKey) {
continue
}
@@ -1,25 +0,0 @@
import { describe, expect, it } from 'vitest'
import {
parseMergeTreeNameOnlyOutput,
PR_CONFLICT_FILES_MAX_BYTES,
PR_CONFLICT_FILES_MAX_ENTRIES
} from './conflict-summary'
describe('PR conflict summary retention bounds', () => {
it('caps retained conflict file count', () => {
const stdout = [
'tree-oid',
...Array.from({ length: PR_CONFLICT_FILES_MAX_ENTRIES + 1 }, (_, index) => `file-${index}`)
].join('\0')
const files = parseMergeTreeNameOnlyOutput(stdout)
expect(files).toHaveLength(PR_CONFLICT_FILES_MAX_ENTRIES)
expect(files.at(-1)).toBe(`file-${PR_CONFLICT_FILES_MAX_ENTRIES - 1}`)
})
it('stops before retaining an oversized path', () => {
const files = parseMergeTreeNameOnlyOutput(
`tree-oid\0${'x'.repeat(PR_CONFLICT_FILES_MAX_BYTES + 1)}\0later`
)
expect(files).toEqual([])
})
})
+6 -13
View File
@@ -1,5 +1,4 @@
import type { PRConflictSummary } from '../../shared/types'
import { cacheIdentityDigest } from '../cache-identity-digest'
// Why 60s: the hottest coordinator cadences that re-derive a CONFLICTING PR
// (10s mergeability-pending, 2.5s manual-pending) previously each ran a
@@ -14,7 +13,6 @@ export const CONFLICT_SUMMARY_BASE_FETCH_WINDOW_MS = 60_000
// that stop refreshing can't accumulate entries forever.
const BASE_OID_CACHE_MAX = 64
const SUMMARY_CACHE_MAX = 128
const CONFLICT_SUMMARY_MAX_IN_FLIGHT = 32
export type FreshBaseTipResolution =
| { kind: 'resolved'; oid: string }
@@ -44,10 +42,11 @@ export function getConflictSummaryGitRuntimeKey(wslDistro: string | undefined):
return wslDistro ? `wsl:${wslDistro}` : 'local:host'
}
// Why digest: keys include arbitrary repo paths and refs; a length-framed
// digest prevents delimiter aliases without retaining those strings.
// 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 cacheIdentityDigest(parts)
return JSON.stringify(parts)
}
export function readFreshBaseTipResolution(baseKey: string): FreshBaseTipResolution | null {
@@ -122,14 +121,8 @@ function dedupeInFlight<T>(
if (existing) {
return existing
}
if (map.size >= CONFLICT_SUMMARY_MAX_IN_FLIGHT) {
return factory()
}
let promise!: Promise<T>
promise = factory().finally(() => {
if (map.get(key) === promise) {
map.delete(key)
}
const promise = factory().finally(() => {
map.delete(key)
})
map.set(key, promise)
return promise
+6 -25
View File
@@ -4,7 +4,6 @@ import {
isUnsupportedMergeTreeWriteTreeError
} from '../../shared/git-merge-tree-capability'
import { gitExecFileAsync } from '../git/runner'
import { iterateNulDelimitedFields } from '../../shared/nul-delimited-fields'
import {
clearGitCapabilityStateForTests,
getLocalGitCapabilityCache
@@ -21,15 +20,11 @@ import {
storeResolvedBaseTip,
storeCachedSummary
} from './conflict-summary-cache'
import { measureUtf8ByteLength } from '../../shared/utf8-byte-limits'
type LocalGitExecOptions = {
wslDistro?: string
}
export const PR_CONFLICT_FILES_MAX_ENTRIES = 512
export const PR_CONFLICT_FILES_MAX_BYTES = 256 * 1024
export function __resetPRConflictSummaryCachesForTests(): void {
clearGitCapabilityStateForTests()
__resetPRConflictSummaryDerivationCachesForTests()
@@ -306,27 +301,13 @@ async function loadConflictingFilesWithLegacyMergeTree(
}
}
export function parseMergeTreeNameOnlyOutput(stdout: string): string[] {
const files: string[] = []
let skippedTreeId = false
let retainedBytes = 0
for (const entry of iterateNulDelimitedFields(stdout)) {
if (!entry) {
continue
}
if (!skippedTreeId) {
skippedTreeId = true
continue
}
const measured = measureUtf8ByteLength(entry, {
stopAfterBytes: PR_CONFLICT_FILES_MAX_BYTES - retainedBytes
})
if (measured.exceededLimit || files.length >= PR_CONFLICT_FILES_MAX_ENTRIES) {
break
}
files.push(entry.replace(/$/u, ''))
retainedBytes += measured.byteLength
function parseMergeTreeNameOnlyOutput(stdout: string): string[] {
const entries = stdout.split('\0').filter(Boolean)
if (entries.length === 0) {
return []
}
const [, ...files] = entries
return files
}
-29
View File
@@ -678,35 +678,6 @@ describe('github owner/repo resolution', () => {
await rm(repoPath, { recursive: true, force: true })
}
})
it('parses exact-limit config files and safely skips oversized include graphs', async () => {
const repoPath = await mkdtemp(join(tmpdir(), 'orca-gh-utils-'))
const gitDir = join(repoPath, '.git')
const includedConfigPath = join(repoPath, 'included.gitconfig')
const configPath = join(gitDir, 'config')
const maxConfigBytes = 4 * 1024 * 1024
await mkdir(gitDir)
await writeFile(includedConfigPath, '[user]\n\tname = first\n')
const includePrefix = `[include]\n\tpath = "${includedConfigPath}"\n#`
await writeFile(configPath, includePrefix + 'x'.repeat(maxConfigBytes - includePrefix.length))
try {
const exactFirst = await readLocalGitConfigSignature({ repoPath, connectionId: null })
await writeFile(includedConfigPath, '[user]\n\tname = exact-limit-change\n')
const exactSecond = await readLocalGitConfigSignature({ repoPath, connectionId: null })
expect(exactSecond).not.toEqual(exactFirst)
await writeFile(
configPath,
includePrefix + 'x'.repeat(maxConfigBytes + 1 - includePrefix.length)
)
const oversizedFirst = await readLocalGitConfigSignature({ repoPath, connectionId: null })
await writeFile(includedConfigPath, '[user]\n\tname = ignored-oversized-change\n')
const oversizedSecond = await readLocalGitConfigSignature({ repoPath, connectionId: null })
expect(oversizedSecond).toEqual(oversizedFirst)
} finally {
await rm(repoPath, { recursive: true, force: true })
}
})
})
describe('resolveIssueSource', () => {
+17 -4
View File
@@ -4,7 +4,6 @@ import { gitExecFileAsync, ghExecFileAsync } from '../git/runner'
// Pure error-parsing helpers come from the lightweight module (not `runner`) so
// tests that mock `../git/runner` still resolve the real implementations.
import { extractExecError, parseRetryAfterMs } from '../git/exec-error'
import { IntegrationApiConcurrencyGate } from '../integration-api-concurrency'
// Why: legacy generic execFile wrapper - only used by callers that don't need
// WSL-aware routing. Repo-scoped callers should use the runner exports below.
@@ -35,12 +34,26 @@ export type {
} from './github-repository-identity'
const MAX_CONCURRENT = 4
const concurrencyGate = new IntegrationApiConcurrencyGate(MAX_CONCURRENT)
let running = 0
const queue: (() => void)[] = []
export function acquire(): Promise<void> {
return concurrencyGate.acquire()
if (running < MAX_CONCURRENT) {
running += 1
return Promise.resolve()
}
return new Promise((resolve) =>
queue.push(() => {
running += 1
resolve()
})
)
}
export function release(): void {
concurrencyGate.release()
running -= 1
const next = queue.shift()
if (next) {
next()
}
}
@@ -1,14 +0,0 @@
import type { GitHubOwnerRepo } from '../../shared/types'
const GITHUB_OWNER_SLUG_RE = /^[A-Za-z0-9][A-Za-z0-9-]*$/
const GITHUB_REPO_SLUG_RE = /^[A-Za-z0-9._-]+$/
// Why: renderer/RPC overrides are interpolated into authenticated REST paths.
export function isValidGitHubApiRepository(repository: GitHubOwnerRepo): boolean {
return (
GITHUB_OWNER_SLUG_RE.test(repository.owner) &&
GITHUB_REPO_SLUG_RE.test(repository.repo) &&
repository.repo !== '.' &&
repository.repo !== '..'
)
}
+15 -12
View File
@@ -14,8 +14,6 @@ import {
getEnterpriseGitHubRepoSlugForRemote,
isGitHubHostAuthenticated
} from './github-enterprise-repository'
import { isValidGitHubApiRepository } from './github-api-repository-validation'
import { cacheIdentityDigest } from '../cache-identity-digest'
export type GitHubApiRepository = GitHubOwnerRepo
export type GitHubRepoExecOptions = ReturnType<typeof ghRepoExecOptions> & { host?: string }
@@ -30,12 +28,25 @@ type GitHubApiRepositoryResolution =
| undefined
| (() => Promise<GitHubApiRepository | null>)
// Why: renderer/RPC repository overrides are interpolated into REST paths.
// Reject path syntax before an authenticated gh process can target it.
const GITHUB_OWNER_SLUG_RE = /^[A-Za-z0-9][A-Za-z0-9-]*$/
const GITHUB_REPO_SLUG_RE = /^[A-Za-z0-9._-]+$/
function isValidGitHubApiRepository(repository: GitHubApiRepository): boolean {
return (
GITHUB_OWNER_SLUG_RE.test(repository.owner) &&
GITHUB_REPO_SLUG_RE.test(repository.repo) &&
repository.repo !== '.' &&
repository.repo !== '..'
)
}
// Why: the enterprise branch spawns an uncached `git remote get-url` (an SSH
// round trip on connection-backed repos) — hot paths like per-file contents
// and viewed-state toggles resolve per call, so cache like ownerRepoCache does.
const ORIGIN_REPO_CACHE_TTL_MS = 30_000
const ORIGIN_REPO_CACHE_MAX_ENTRIES = 512
const ORIGIN_REPO_MAX_IN_FLIGHT = 32
const originRepoCache = new Map<string, { value: GitHubApiRepository | null; expiresAt: number }>()
const originRepoInFlight = new Map<string, Promise<GitHubApiRepository | null>>()
@@ -45,12 +56,7 @@ function originRepoCacheKey(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): string {
return cacheIdentityDigest([
connectionId ?? 'local',
localGitOptions.wslDistro ?? '',
repoPath,
remoteName
])
return `${connectionId ?? 'local'}\0${localGitOptions.wslDistro ?? ''}\0${repoPath}\0${remoteName}`
}
/** @internal - exposed for tests only */
@@ -130,9 +136,6 @@ export async function getGitHubApiRepositoryForRemote(
}
return slug ?? null
})()
if (originRepoInFlight.size >= ORIGIN_REPO_MAX_IN_FLIGHT) {
return probe
}
originRepoInFlight.set(cacheKey, probe)
try {
return await probe
@@ -13,7 +13,6 @@ import {
type LocalGitExecOptions
} from './github-repository-identity'
import { parseWslPath } from '../wsl'
import { cacheIdentityDigest } from '../cache-identity-digest'
export type GitHubEnterpriseRepoSlug = GitHubOwnerRepo & { host: string }
@@ -23,8 +22,6 @@ export type GitHubEnterpriseRepoSlug = GitHubOwnerRepo & { host: string }
// GHES remote is not left to fall through to Gitea (#8312).
const HOST_AUTH_TTL_MS = 60_000
const HOST_AUTH_CACHE_MAX_ENTRIES = 512
const HOST_AUTH_MAX_IN_FLIGHT = 16
const GITHUB_HOST_MAX_CHARS = 1024
type HostAuthCacheEntry = {
authenticatedHost: string | null
@@ -85,9 +82,6 @@ type NormalizedGitHubHost = {
}
function normalizeGitHubHost(host: string): NormalizedGitHubHost | null {
if (host.length > GITHUB_HOST_MAX_CHARS) {
return null
}
const match = host
.trim()
.toLowerCase()
@@ -134,14 +128,8 @@ async function resolveAuthenticatedGitHubHost(
connectionId?: string | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<string | null | undefined> {
const normalizedHost = normalizeGitHubHost(host)?.authority
if (!normalizedHost) {
return null
}
const cacheKey = cacheIdentityDigest([
runtimeCacheKey(repoPath, localGitOptions.wslDistro),
normalizedHost
])
const normalizedHost = normalizeGitHubHost(host)?.authority ?? host.trim().toLowerCase()
const cacheKey = `${runtimeCacheKey(repoPath, localGitOptions.wslDistro)}\0${normalizedHost}`
const now = Date.now()
pruneHostAuthCache(now)
const cached = hostAuthCache.get(cacheKey)
@@ -180,9 +168,6 @@ async function resolveAuthenticatedGitHubHost(
pruneHostAuthCache(Date.now())
return authenticatedHost
})()
if (hostAuthInFlight.size >= HOST_AUTH_MAX_IN_FLIGHT) {
return probe
}
hostAuthInFlight.set(cacheKey, probe)
try {
return await probe
@@ -1,22 +1,8 @@
import { describe, expect, it } from 'vitest'
import {
GITHUB_REMOTE_REPO_MAX_BYTES,
GITHUB_REMOTE_URL_MAX_BYTES,
parseGitHubOwnerRepo,
parseGitHubRemoteIdentity
} from './github-remote-identity-parsing'
import { parseGitHubOwnerRepo, parseGitHubRemoteIdentity } from './github-remote-identity-parsing'
describe('parseGitHubRemoteIdentity', () => {
it('rejects oversized remote URLs and identity fields before retention', () => {
expect(parseGitHubRemoteIdentity('x'.repeat(GITHUB_REMOTE_URL_MAX_BYTES + 1))).toBeNull()
expect(
parseGitHubRemoteIdentity(
`git@github.com:owner/${'r'.repeat(GITHUB_REMOTE_REPO_MAX_BYTES + 1)}.git`
)
).toBeNull()
})
it('parses a plain github.com https remote', () => {
expect(parseGitHubRemoteIdentity('https://github.com/team/orca.git')).toEqual({
host: 'github.com',
@@ -1,15 +1,6 @@
import type { GitHubOwnerRepo } from '../../shared/types'
import { measureUtf8ByteLength } from '../../shared/utf8-byte-limits'
export type GitHubRemoteIdentity = GitHubOwnerRepo & { host: string }
export const GITHUB_REMOTE_URL_MAX_BYTES = 64 * 1024
export const GITHUB_REMOTE_HOST_MAX_BYTES = 1024
export const GITHUB_REMOTE_OWNER_MAX_BYTES = 256
export const GITHUB_REMOTE_REPO_MAX_BYTES = 1024
function fitsRemoteField(value: string, maxBytes: number): boolean {
return !measureUtf8ByteLength(value, { stopAfterBytes: maxBytes }).exceededLimit
}
function normalizeGitHubRemoteHost(host: string): string {
const normalizedHost = host.toLowerCase()
@@ -31,31 +22,16 @@ function parseGitHubRemotePath(path: string): Pick<GitHubRemoteIdentity, 'owner'
}
const [owner, repoWithSuffix] = parts
const repo = repoWithSuffix.replace(/\.git$/i, '')
if (
!owner ||
!repo ||
!fitsRemoteField(owner, GITHUB_REMOTE_OWNER_MAX_BYTES) ||
!fitsRemoteField(repo, GITHUB_REMOTE_REPO_MAX_BYTES)
) {
if (!owner || !repo) {
return null
}
return { owner, repo }
}
export function parseGitHubRemoteIdentity(remoteUrl: string): GitHubRemoteIdentity | null {
if (!fitsRemoteField(remoteUrl, GITHUB_REMOTE_URL_MAX_BYTES)) {
return null
}
const trimmed = remoteUrl.trim()
const sshMatch = trimmed.match(/^git@([^:]+):([^/]+)\/([^/]+?)(?:\.git)?$/i)
if (sshMatch) {
if (
!fitsRemoteField(sshMatch[1], GITHUB_REMOTE_HOST_MAX_BYTES) ||
!fitsRemoteField(sshMatch[2], GITHUB_REMOTE_OWNER_MAX_BYTES) ||
!fitsRemoteField(sshMatch[3], GITHUB_REMOTE_REPO_MAX_BYTES)
) {
return null
}
return { host: normalizeGitHubRemoteHost(sshMatch[1]), owner: sshMatch[2], repo: sshMatch[3] }
}
@@ -65,8 +41,7 @@ export function parseGitHubRemoteIdentity(remoteUrl: string): GitHubRemoteIdenti
return null
}
const path = parseGitHubRemotePath(url.pathname)
const host = normalizeGitHubRemoteHost(hostFromRemoteUrl(url))
return path && fitsRemoteField(host, GITHUB_REMOTE_HOST_MAX_BYTES) ? { host, ...path } : null
return path ? { host: normalizeGitHubRemoteHost(hostFromRemoteUrl(url)), ...path } : null
} catch {
return null
}
@@ -9,7 +9,6 @@ import {
} from './github-remote-identity-parsing'
import { isStableMissingGitRemoteError } from './stable-missing-git-remote-error'
import { githubRepoIdentityKey } from '../../shared/github-repository-identity-key'
import { cacheIdentityDigest } from '../cache-identity-digest'
export type OwnerRepo = GitHubOwnerRepo
@@ -54,7 +53,6 @@ export function ghRepoExecOptions(context: GitHubRepoContext): {
const OWNER_REPO_POSITIVE_CACHE_TTL_MS = 30_000
const OWNER_REPO_NEGATIVE_CACHE_TTL_MS = 5 * 60_000
const OWNER_REPO_CACHE_MAX_ENTRIES = 512
const OWNER_REPO_MAX_IN_FLIGHT = 32
type OwnerRepoCacheEntry = {
value: OwnerRepo | null
@@ -125,7 +123,7 @@ export async function getOwnerRepoForRemote(
): Promise<OwnerRepo | null> {
const context = githubRepoContext(repoPath, connectionId, localGitOptions)
const runtimeKey = context.connectionId ?? `local:${context.wslDistro ?? 'host'}`
const cacheKey = cacheIdentityDigest([runtimeKey, context.repoPath, remoteName])
const cacheKey = `${runtimeKey}\0${context.repoPath}\0${remoteName}`
const now = Date.now()
pruneOwnerRepoCache(now)
const cached = ownerRepoCache.get(cacheKey)
@@ -160,9 +158,6 @@ export async function getOwnerRepoForRemote(
// Why: startup can resolve issue sources, PR candidates, and repo metadata
// for the same repo concurrently. Coalesce missing-remote probes.
const probe = resolveOwnerRepoForRemote(context, remoteName, cacheKey, nextConfigSignature)
if (ownerRepoInFlight.size >= OWNER_REPO_MAX_IN_FLIGHT) {
return probe
}
ownerRepoInFlight.set(cacheKey, probe)
try {
return await probe
+11 -62
View File
@@ -1,11 +1,7 @@
import { stat } from 'node:fs/promises'
import { createHash } from 'node:crypto'
import { readFile, stat } from 'node:fs/promises'
import { homedir } from 'node:os'
import { dirname, isAbsolute, join, resolve } from 'node:path'
import { readNodeFileWithinLimit } from '../../shared/node-bounded-file-reader'
import { measureUtf8ByteLength } from '../../shared/utf8-byte-limits'
import type { GitHubRepoContext } from './github-repository-identity'
import { cacheIdentityDigest } from '../cache-identity-digest'
type LocalGitConfigPaths = {
commonConfigPath: string
@@ -13,18 +9,6 @@ type LocalGitConfigPaths = {
}
const localGitConfigSignatureInFlight = new Map<string, Promise<string | undefined>>()
const LOCAL_GIT_CONFIG_SIGNATURE_MAX_IN_FLIGHT = 32
const MAX_GIT_CONFIG_BYTES = 4 * 1024 * 1024
const MAX_GIT_POINTER_FILE_BYTES = 64 * 1024
const MAX_INCLUDED_CONFIG_FILES = 256
const MAX_INCLUDED_CONFIG_DEPTH = 8
const MAX_INCLUDED_CONFIG_PATH_BYTES = 16 * 1024
const MAX_INCLUDED_CONFIG_AGGREGATE_PATH_BYTES = 2 * 1024 * 1024
type ConfigSignatureBudget = {
admittedFiles: number
pathBytes: number
}
export async function readLocalGitConfigSignature(
context: GitHubRepoContext
@@ -34,16 +18,13 @@ export async function readLocalGitConfigSignature(
// runtimes are already separated by cache key and probed through git.
return undefined
}
const cacheKey = cacheIdentityDigest([context.repoPath])
const cacheKey = context.repoPath
const inFlight = localGitConfigSignatureInFlight.get(cacheKey)
if (inFlight) {
return inFlight
}
const read = readUncachedLocalGitConfigSignature(context.repoPath)
if (localGitConfigSignatureInFlight.size >= LOCAL_GIT_CONFIG_SIGNATURE_MAX_IN_FLIGHT) {
return read
}
localGitConfigSignatureInFlight.set(cacheKey, read)
try {
return await read
@@ -67,54 +48,31 @@ async function readUncachedLocalGitConfigSignature(repoPath: string): Promise<st
readConfigPathSignatures(configPaths.commonConfigPath),
readConfigPathSignatures(configPaths.worktreeConfigPath)
])
const digest = createHash('sha256')
for (const signature of signatures.flat()) {
digest.update(`${signature.length}:`)
digest.update(signature)
}
return digest.digest('base64url')
return signatures.flat().join('\0')
}
async function readConfigPathSignatures(
configPath: string,
visited = new Set<string>(),
budget: ConfigSignatureBudget = { admittedFiles: 0, pathBytes: 0 },
depth = 0
visited = new Set<string>()
): Promise<string[]> {
if (visited.has(configPath)) {
return []
}
const measuredPath = measureUtf8ByteLength(configPath, {
stopAfterBytes: MAX_INCLUDED_CONFIG_PATH_BYTES
})
if (
measuredPath.exceededLimit ||
depth > MAX_INCLUDED_CONFIG_DEPTH ||
budget.admittedFiles >= MAX_INCLUDED_CONFIG_FILES ||
budget.pathBytes + measuredPath.byteLength > MAX_INCLUDED_CONFIG_AGGREGATE_PATH_BYTES
) {
return []
}
visited.add(configPath)
budget.admittedFiles += 1
budget.pathBytes += measuredPath.byteLength
const ownSignature = await readConfigPathSignature(configPath)
let configText: string
try {
configText = (await readNodeFileWithinLimit(configPath, MAX_GIT_CONFIG_BYTES)).buffer.toString(
'utf8'
)
configText = await readFile(configPath, 'utf8')
} catch {
return [ownSignature]
}
const includedPaths = parseIncludedConfigPaths(configText, dirname(configPath))
const signatures = [ownSignature]
for (const includedPath of includedPaths) {
signatures.push(...(await readConfigPathSignatures(includedPath, visited, budget, depth + 1)))
}
return signatures
const includedSignatures = await Promise.all(
includedPaths.map((includedPath) => readConfigPathSignatures(includedPath, visited))
)
return [ownSignature, ...includedSignatures.flat()]
}
async function readConfigPathSignature(configPath: string): Promise<string> {
@@ -145,9 +103,6 @@ function parseIncludedConfigPaths(configText: string, baseDir: string): string[]
const includePath = parseIncludedConfigPath(line)
if (includePath) {
includedPaths.push(resolveIncludedConfigPath(includePath, baseDir))
if (includedPaths.length >= MAX_INCLUDED_CONFIG_FILES) {
break
}
}
}
return includedPaths
@@ -259,9 +214,7 @@ async function resolveLocalGitConfigPaths(repoPath: string): Promise<LocalGitCon
}
try {
const gitFile = (
await readNodeFileWithinLimit(dotGitPath, MAX_GIT_POINTER_FILE_BYTES)
).buffer.toString('utf8')
const gitFile = await readFile(dotGitPath, 'utf8')
const match = gitFile.match(/^gitdir:\s*(.+?)\s*$/im)
if (!match) {
return null
@@ -269,11 +222,7 @@ async function resolveLocalGitConfigPaths(repoPath: string): Promise<LocalGitCon
const gitDir = resolve(dirname(dotGitPath), match[1])
let commonGitDir = gitDir
try {
const commonDir = (
await readNodeFileWithinLimit(join(gitDir, 'commondir'), MAX_GIT_POINTER_FILE_BYTES)
).buffer
.toString('utf8')
.trim()
const commonDir = (await readFile(join(gitDir, 'commondir'), 'utf8')).trim()
if (commonDir) {
commonGitDir = resolve(gitDir, commonDir)
}
@@ -3,7 +3,6 @@ import { noteRepositoryRateLimitSpend, repositoryRateLimitGuard } from './rate-l
import { githubHostExecOptions } from './github-api-repository'
import { githubRepoIdentityKey } from '../../shared/github-repository-identity-key'
import type { OwnerRepo } from './github-repository-identity'
import { cacheIdentityDigest } from '../cache-identity-digest'
type GhExecOptions = Parameters<typeof ghExecFileAsync>[1]
@@ -62,11 +61,7 @@ export async function isCommitPartOfMergedPR(args: {
}
const owner = args.ownerRepo.owner
const repo = args.ownerRepo.repo
const cacheKey = cacheIdentityDigest([
githubRepoIdentityKey(args.ownerRepo),
String(args.prNumber),
oid
])
const cacheKey = `${githubRepoIdentityKey(args.ownerRepo)}#${args.prNumber}@${oid}`
const ghOptions = { ...args.ghOptions, ...githubHostExecOptions(args.ownerRepo) }
const now = Date.now()
pruneMergedPRCommitMembershipCache(now)
@@ -1,84 +0,0 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type { GitHubPRRefreshCandidate } from '../../shared/types'
import {
PR_REFRESH_ALIAS_LIMIT,
PR_REFRESH_QUEUE_ENTRY_LIMIT
} from '../../shared/pr-refresh-memory-limits'
const { sendToTrustedUIRendererMock } = vi.hoisted(() => ({
sendToTrustedUIRendererMock: vi.fn()
}))
vi.mock('electron', () => ({
webContents: { getAllWebContents: () => [] }
}))
vi.mock('./client', () => ({ getPRForBranchOutcome: vi.fn() }))
vi.mock('./github-api-repository', () => ({ getOriginGitHubApiRepository: vi.fn() }))
vi.mock('./rate-limit', () => ({
getRateLimit: vi.fn(),
noteRepositoryRateLimitSpend: vi.fn(),
repositoryRateLimitGuard: vi.fn(() => ({ blocked: false })),
spendsSharedGitHubComQuota: vi.fn(() => false)
}))
vi.mock('../crash-reporting/crash-breadcrumb-store', () => ({
recordCoalescedCrashBreadcrumb: vi.fn()
}))
vi.mock('../ipc/ui', () => ({ sendToTrustedUIRenderer: sendToTrustedUIRendererMock }))
function candidate(index: number, linkedPRNumber?: number): GitHubPRRefreshCandidate {
return {
cacheKey: `/repo-${index}::feature-${index}`,
repoPath: linkedPRNumber === undefined ? `/repo-${index}` : '/repo',
branch: `feature-${index}`,
repoKind: 'git',
repoId: `repo-${index}`,
worktreeId: `worktree-${index}`,
cachedFetchedAt: Date.now(),
linkedPRNumber
}
}
describe('PR refresh coordinator memory admission', () => {
beforeEach(() => {
vi.resetModules()
vi.useFakeTimers()
vi.setSystemTime(1_000)
sendToTrustedUIRendererMock.mockReset()
})
afterEach(() => {
vi.useRealTimers()
})
it('caps distinct queued refreshes while GitHub work is delayed', async () => {
const { enqueuePRRefresh, _getPRRefreshQueueSizeForTests } =
await import('./pr-refresh-coordinator')
for (let index = 0; index <= PR_REFRESH_QUEUE_ENTRY_LIMIT; index += 1) {
enqueuePRRefresh(candidate(index), 'visible', 40, 1)
}
expect(_getPRRefreshQueueSizeForTests()).toBe(PR_REFRESH_QUEUE_ENTRY_LIMIT)
expect(sendToTrustedUIRendererMock).toHaveBeenLastCalledWith(
'gh:prRefreshEvent',
expect.objectContaining({ status: 'skipped', skippedReason: 'capacity' })
)
})
it('caps aliases coalesced behind one linked review', async () => {
const { enqueuePRRefresh, _getPRRefreshAliasCountForTests } =
await import('./pr-refresh-coordinator')
for (let index = 0; index <= PR_REFRESH_ALIAS_LIMIT; index += 1) {
enqueuePRRefresh(candidate(index, 42), 'visible', 40, 1)
}
expect(_getPRRefreshAliasCountForTests('local::runtime:host::/repo::pr::42')).toBe(
PR_REFRESH_ALIAS_LIMIT
)
expect(sendToTrustedUIRendererMock).toHaveBeenCalledWith(
'gh:prRefreshEvent',
expect.objectContaining({ status: 'skipped', skippedReason: 'capacity' })
)
})
})
+18 -114
View File
@@ -11,14 +11,6 @@ import type {
import { getPRForBranchOutcome, type GitHubPRBranchLookupOptions } from './client'
import { getOriginGitHubApiRepository } from './github-api-repository'
import { ghRepoExecOptions, githubRepoContext } from './gh-utils'
import {
boundedVisiblePRRefreshCandidates,
PR_REFRESH_ACTIVE_SCOPE_LIMIT,
PR_REFRESH_QUEUE_ENTRY_LIMIT,
PR_REFRESH_RETRY_STATE_LIMIT,
retainPRRefreshAlias,
retainPRRefreshState
} from './pr-refresh-memory-bounds'
import {
getRateLimit,
noteRepositoryRateLimitSpend,
@@ -105,19 +97,8 @@ let lastBackgroundStartAt = 0
* Only a rate-limit outcome carrying `retryDisabledUntil` sets a gate; any other settled outcome clears it.
*/
function noteManualRetryGate(key: string, outcome: PRRefreshOutcome): void {
const now = Date.now()
for (const [gateKey, retryAt] of manualRetryGates) {
if (retryAt <= now) {
manualRetryGates.delete(gateKey)
}
}
if (outcome.kind === 'upstream-error' && outcome.retryDisabledUntil !== undefined) {
retainPRRefreshState(
manualRetryGates,
key,
outcome.retryDisabledUntil,
PR_REFRESH_RETRY_STATE_LIMIT
)
manualRetryGates.set(key, outcome.retryDisabledUntil)
} else {
manualRetryGates.delete(key)
}
@@ -137,14 +118,6 @@ const diagnosticsCounters = {
backgroundPauses: 0
}
function setBoundedQueueEntry(entry: QueueEntry): boolean {
if (!queue.has(entry.key) && queue.size >= PR_REFRESH_QUEUE_ENTRY_LIMIT) {
return false
}
queue.set(entry.key, entry)
return true
}
export function setPRRefreshOutcomeObserver(observer: PRRefreshOutcomeObserver | null): void {
outcomeObserver = observer
}
@@ -185,26 +158,6 @@ function recordPRRefreshQueueDiagnostic(
})
}
function broadcastCapacitySkip(
aliases: GitHubPRRefreshAlias[],
reason: GitHubPRRefreshReason
): void {
diagnosticsCounters.skipped += 1
recordPRRefreshQueueDiagnostic('skipped', reason, 'capacity')
broadcast({ aliases, reason, status: 'skipped', skippedReason: 'capacity' })
}
function addBoundedQueueAlias(
entry: QueueEntry,
alias: GitHubPRRefreshAlias,
reason: GitHubPRRefreshReason
): void {
const evicted = retainPRRefreshAlias(entry.aliases, alias, entry.candidate.cacheKey)
if (evicted) {
broadcastCapacitySkip([evicted], reason)
}
}
function clearActiveBurstWindow(windowId: number): void {
const windowPrefix = `${windowId}::`
for (const scope of Array.from(activeStartsByScope.keys())) {
@@ -411,18 +364,15 @@ function visibleCandidateAfterOutcome(
}
}
function setVisibleFollowUp(entry: QueueEntry): boolean {
function setVisibleFollowUp(entry: QueueEntry): void {
const existing = queue.get(entry.key)
if (!existing) {
if (!setBoundedQueueEntry(entry)) {
broadcastCapacitySkip(Array.from(entry.aliases.values()), entry.reason)
return false
}
return true
queue.set(entry.key, entry)
return
}
for (const alias of entry.aliases.values()) {
addBoundedQueueAlias(existing, alias, entry.reason)
existing.aliases.set(alias.cacheKey, alias)
}
// Why: a user activation can arrive while a background refresh awaits gh; the follow-up must not overwrite that pending active/manual work.
@@ -431,14 +381,13 @@ function setVisibleFollowUp(entry: QueueEntry): boolean {
existing.priority > entry.priority ||
existing.dueAt <= entry.dueAt
) {
return true
return
}
setBoundedQueueEntry({
queue.set(entry.key, {
...entry,
aliases: existing.aliases
})
return true
}
function removeQueuedAliasForInvalidCandidate(key: string, alias: GitHubPRRefreshAlias): void {
@@ -477,7 +426,7 @@ 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))
retainPRRefreshState(errorBackoff, key, { failures, retryAt }, PR_REFRESH_RETRY_STATE_LIMIT)
errorBackoff.set(key, { failures, retryAt })
return retryAt
}
@@ -514,7 +463,7 @@ function scheduleVisibleFollowUp(
if (outcome.kind === 'upstream-error') {
// Why: reuse the retry time already computed for the broadcast so the same failure isn't counted twice against the backoff.
const retryAt = options?.plannedRetryAt ?? nextVisibleErrorRetryAt(key)
const retained = setVisibleFollowUp({
setVisibleFollowUp({
key,
candidate,
aliases: new Map(aliases.map((alias) => [alias.cacheKey, alias])),
@@ -524,10 +473,6 @@ function scheduleVisibleFollowUp(
queuedAt: nextQueueOrder(),
windowId
})
if (!retained) {
resetKeyRetryState(key)
return
}
// Why: this is a delayed retry, not active work; a spinner would make visible worktrees look stuck until backoff expires.
scheduleDrain(retryAt - Date.now())
return
@@ -544,7 +489,7 @@ function scheduleVisibleFollowUp(
? regularDueAt
: Math.min(regularDueAt, pendingMergeabilityDueAt)
// Why: a coalesced linked-PR refresh may represent several branches; preserve every alias so all cache entries keep getting updates.
const retained = setVisibleFollowUp({
setVisibleFollowUp({
key,
candidate: followUpCandidate,
aliases: new Map(aliases.map((alias) => [alias.cacheKey, alias])),
@@ -556,10 +501,6 @@ function scheduleVisibleFollowUp(
bypassBackgroundBudget: pendingMergeabilityDueAt !== null,
windowId
})
if (!retained) {
resetKeyRetryState(key)
return
}
scheduleDrain(Math.max(0, dueAt - Date.now()))
}
@@ -654,12 +595,6 @@ function pruneActiveStarts(scope: string, now: number): number[] {
return activeStarts
}
function pruneExpiredActiveScopes(now: number): void {
for (const scope of Array.from(activeStartsByScope.keys())) {
pruneActiveStarts(scope, now)
}
}
function nextActiveBurstDelay(entry: QueueEntry): number {
const now = Date.now()
const activeStarts = pruneActiveStarts(activeBurstScope(entry), now)
@@ -672,10 +607,9 @@ function nextActiveBurstDelay(entry: QueueEntry): number {
function noteActiveStart(entry: QueueEntry): void {
const now = Date.now()
const scope = activeBurstScope(entry)
pruneExpiredActiveScopes(now)
const activeStarts = pruneActiveStarts(scope, now)
activeStarts.push(now)
retainPRRefreshState(activeStartsByScope, scope, activeStarts, PR_REFRESH_ACTIVE_SCOPE_LIMIT)
activeStartsByScope.set(scope, activeStarts)
}
function activeOrder(a: QueueEntry, b: QueueEntry): number {
@@ -824,12 +758,7 @@ async function drainQueue(): Promise<void> {
.find((guard) => guard.blocked)
if (blockedGuard?.blocked) {
const retryAt = blockedGuard.resetAt * 1000
const retained = setBoundedQueueEntry({ ...next, dueAt: retryAt })
if (!retained) {
resetKeyRetryState(next.key)
broadcastCapacitySkip(aliases, next.reason)
continue
}
queue.set(next.key, { ...next, dueAt: retryAt })
broadcast({
aliases,
reason: next.reason,
@@ -916,7 +845,7 @@ export function enqueuePRRefresh(
const freshDueAt = shouldSkipFresh(candidate, reason) ? freshRetryAt(candidate) : null
const dueAt = freshDueAt ?? Date.now() + (reason === 'post-push' ? POST_PUSH_DELAY_MS : 0)
if (existing) {
addBoundedQueueAlias(existing, alias, reason)
existing.aliases.set(alias.cacheKey, alias)
diagnosticsCounters.coalesced += 1
recordPRRefreshQueueDiagnostic('coalesced', reason)
const shouldPromoteExisting =
@@ -942,13 +871,9 @@ export function enqueuePRRefresh(
}
}
} else {
if (queue.size >= PR_REFRESH_QUEUE_ENTRY_LIMIT) {
broadcastCapacitySkip([alias], reason)
return
}
diagnosticsCounters.enqueued += 1
recordPRRefreshQueueDiagnostic('enqueued', reason)
setBoundedQueueEntry({
queue.set(key, {
key,
candidate,
aliases: new Map([[alias.cacheKey, alias]]),
@@ -975,13 +900,9 @@ export function reportVisiblePRRefreshCandidates(
if (existingVisible && generation < existingVisible.generation) {
return
}
const retainedCandidates = boundedVisiblePRRefreshCandidates(candidates)
visibleByWindow.set(windowId, {
generation,
keys: new Set(retainedCandidates.map(refreshKey))
})
visibleByWindow.set(windowId, { generation, keys: new Set(candidates.map(refreshKey)) })
removeInvisibleVisibleRefreshes()
for (const candidate of retainedCandidates) {
for (const candidate of candidates) {
enqueuePRRefresh(candidate, 'visible', 40, windowId)
}
}
@@ -1007,14 +928,7 @@ export async function refreshPRNow(candidate: GitHubPRRefreshCandidate): Promise
const key = refreshKey(candidate)
const existing = queue.get(key)
const aliasMap = new Map(existing ? existing.aliases : [])
const evictedAlias = retainPRRefreshAlias(
aliasMap,
alias,
existing?.candidate.cacheKey ?? alias.cacheKey
)
if (evictedAlias) {
broadcastCapacitySkip([evictedAlias], 'manual')
}
aliasMap.set(alias.cacheKey, alias)
const aliases = Array.from(aliasMap.values())
const skippedReason = validateCandidate(candidate)
if (skippedReason) {
@@ -1049,7 +963,7 @@ export async function refreshPRNow(candidate: GitHubPRRefreshCandidate): Promise
if (gateUntil > Date.now()) {
const retryAt = gateUntil
// Why: paused maps `pausedUntil` into the renderer's auto-retry, so requeue at reset (finding 12) — don't advertise an unscheduled retry.
const retained = setBoundedQueueEntry({
queue.set(key, {
key,
candidate,
aliases: aliasMap,
@@ -1058,16 +972,6 @@ export async function refreshPRNow(candidate: GitHubPRRefreshCandidate): Promise
dueAt: retryAt,
queuedAt: nextQueueOrder()
})
if (!retained) {
broadcastCapacitySkip(aliases, 'manual')
return {
kind: 'upstream-error',
errorType: 'rate_limited',
message: 'GitHub is temporarily limiting requests. Try again after the limit resets.',
fetchedAt: Date.now(),
retryDisabledUntil: retryAt
}
}
broadcast({
aliases,
reason: 'manual',
@@ -1,93 +0,0 @@
import { describe, expect, it } from 'vitest'
import type { GitHubPRRefreshAlias, GitHubPRRefreshCandidate } from '../../shared/types'
import {
boundedVisiblePRRefreshCandidates,
PR_REFRESH_ALIAS_LIMIT,
PR_REFRESH_RETRY_STATE_LIMIT,
retainPRRefreshAlias,
retainPRRefreshState
} from './pr-refresh-memory-bounds'
import { PR_REFRESH_VISIBLE_CANDIDATE_LIMIT } from '../../shared/pr-refresh-memory-limits'
function alias(index: number): GitHubPRRefreshAlias {
return {
cacheKey: `cache-${index}`,
repoId: 'repo-1',
repoPath: '/repo',
branch: `feature-${index}`,
worktreeId: `worktree-${index}`,
connectionId: null,
currentHeadOid: null,
linkedPRNumber: 42,
fallbackPRNumber: null,
fallbackPRSource: null
}
}
function candidate(index: number): GitHubPRRefreshCandidate {
return {
cacheKey: `cache-${index}`,
repoId: 'repo-1',
repoPath: '/repo',
branch: `feature-${index}`,
worktreeId: `worktree-${index}`,
connectionId: null,
currentHeadOid: null,
linkedPRNumber: 42,
fallbackPRNumber: null,
fallbackPRSource: null,
repoKind: 'git',
cachedFetchedAt: null
}
}
describe('PR refresh memory bounds', () => {
it('retains ordinary aliases without changing their identity or order', () => {
const aliases = new Map<string, GitHubPRRefreshAlias>()
expect(retainPRRefreshAlias(aliases, alias(0), 'cache-0')).toBeNull()
expect(retainPRRefreshAlias(aliases, alias(1), 'cache-0')).toBeNull()
expect(Array.from(aliases.keys())).toEqual(['cache-0', 'cache-1'])
})
it('caps aliases while preserving the representative candidate', () => {
const aliases = new Map<string, GitHubPRRefreshAlias>()
for (let index = 0; index < PR_REFRESH_ALIAS_LIMIT; index += 1) {
retainPRRefreshAlias(aliases, alias(index), 'cache-0')
}
const evicted = retainPRRefreshAlias(aliases, alias(PR_REFRESH_ALIAS_LIMIT), 'cache-0')
expect(aliases).toHaveLength(PR_REFRESH_ALIAS_LIMIT)
expect(aliases.has('cache-0')).toBe(true)
expect(aliases.has(`cache-${PR_REFRESH_ALIAS_LIMIT}`)).toBe(true)
expect(evicted?.cacheKey).toBe('cache-1')
})
it('caps retry state and refreshes existing keys without growing', () => {
const states = new Map<number, number>()
for (let index = 0; index < PR_REFRESH_RETRY_STATE_LIMIT; index += 1) {
retainPRRefreshState(states, index, index, PR_REFRESH_RETRY_STATE_LIMIT)
}
expect(retainPRRefreshState(states, 1, 99, PR_REFRESH_RETRY_STATE_LIMIT)).toBeNull()
expect(
retainPRRefreshState(states, PR_REFRESH_RETRY_STATE_LIMIT, 1, PR_REFRESH_RETRY_STATE_LIMIT)
).toBe(0)
expect(states).toHaveLength(PR_REFRESH_RETRY_STATE_LIMIT)
expect(states.get(1)).toBe(99)
})
it('passes ordinary visible candidates through and truncates adversarial batches', () => {
const ordinary = [candidate(0), candidate(1)]
expect(boundedVisiblePRRefreshCandidates(ordinary)).toBe(ordinary)
const oversized = Array.from({ length: PR_REFRESH_VISIBLE_CANDIDATE_LIMIT + 1 }, (_, index) =>
candidate(index)
)
expect(boundedVisiblePRRefreshCandidates(oversized)).toHaveLength(
PR_REFRESH_VISIBLE_CANDIDATE_LIMIT
)
})
})
@@ -1,75 +0,0 @@
import type { GitHubPRRefreshAlias, GitHubPRRefreshCandidate } from '../../shared/types'
import {
PR_REFRESH_ALIAS_LIMIT,
PR_REFRESH_VISIBLE_CANDIDATE_LIMIT
} from '../../shared/pr-refresh-memory-limits'
export {
PR_REFRESH_ACTIVE_SCOPE_LIMIT,
PR_REFRESH_ALIAS_LIMIT,
PR_REFRESH_QUEUE_ENTRY_LIMIT,
PR_REFRESH_RETRY_STATE_LIMIT,
PR_REFRESH_VISIBLE_CANDIDATE_LIMIT
} from '../../shared/pr-refresh-memory-limits'
export function retainPRRefreshAlias(
aliases: Map<string, GitHubPRRefreshAlias>,
alias: GitHubPRRefreshAlias,
protectedCacheKey: string
): GitHubPRRefreshAlias | null {
if (aliases.has(alias.cacheKey)) {
aliases.delete(alias.cacheKey)
aliases.set(alias.cacheKey, alias)
return null
}
if (aliases.size < PR_REFRESH_ALIAS_LIMIT) {
aliases.set(alias.cacheKey, alias)
return null
}
let evictionKey: string | undefined
for (const cacheKey of aliases.keys()) {
if (cacheKey !== protectedCacheKey) {
evictionKey = cacheKey
break
}
}
if (evictionKey === undefined) {
return alias
}
const evicted = aliases.get(evictionKey) ?? null
aliases.delete(evictionKey)
aliases.set(alias.cacheKey, alias)
return evicted
}
export function retainPRRefreshState<K, V>(
entries: Map<K, V>,
key: K,
value: V,
limit: number
): K | null {
if (entries.has(key)) {
entries.delete(key)
entries.set(key, value)
return null
}
let evictedKey: K | null = null
if (entries.size >= limit) {
const oldest = entries.keys().next()
if (!oldest.done) {
evictedKey = oldest.value
entries.delete(oldest.value)
}
}
entries.set(key, value)
return evictedKey
}
export function boundedVisiblePRRefreshCandidates(
candidates: GitHubPRRefreshCandidate[]
): GitHubPRRefreshCandidate[] {
return candidates.length <= PR_REFRESH_VISIBLE_CANDIDATE_LIMIT
? candidates
: candidates.slice(0, PR_REFRESH_VISIBLE_CANDIDATE_LIMIT)
}
@@ -1,7 +1,6 @@
import { createHash } from 'node:crypto'
import { resolve } from 'node:path'
import { recordCoalescedCrashBreadcrumb } from '../crash-reporting/crash-breadcrumb-store'
import { cacheIdentityDigest } from '../cache-identity-digest'
const VALIDATION_BACKOFF_TTL_MS = 5 * 60_000
const MAX_VALIDATION_BACKOFF_ENTRIES = 256
@@ -36,7 +35,7 @@ const counters: ValidationBackoffCounters = {
}
function validationIdentityKey(identity: ValidationBackoffIdentity): string {
return cacheIdentityDigest([identity.repoId ?? '', resolve(identity.repoPath), identity.reason])
return [identity.repoId ?? '', resolve(identity.repoPath), identity.reason].join('\0')
}
function validationIdentityToken(key: string): string {
+3 -3
View File
@@ -52,7 +52,6 @@ import {
isGitHubProjectRefInputTooLarge
} from '../../shared/github-project-ref-input'
import { githubProjectHost } from '../../shared/github-project-identity'
import { cacheIdentityDigest } from '../cache-identity-digest'
// Re-export the public API so existing `./project-view` call sites keep working; the split is internal-only.
export { isValidOwnerSlug, isValidRepoSlug, isValidSlug } from './project-view/internals'
@@ -130,11 +129,12 @@ const parentFieldProbeInFlight = new Map<string, Promise<void>>()
// host's probe result can't leak into another. Normalize github.com so
// host-less callers share the same probe state as explicitly pinned calls.
function ownerScopeKey(owner: string, ownerType: GitHubProjectOwnerType, host?: string): string {
return cacheIdentityDigest([owner, ownerType, githubProjectHost(host)])
const base = `${owner}\u0000${ownerType}`
return `${base}\u0000${githubProjectHost(host)}`
}
function ownerTypeCacheKey(owner: string, host?: string): string {
return cacheIdentityDigest([owner, githubProjectHost(host)])
return `${owner}\u0000${githubProjectHost(host)}`
}
function rememberOwnerType(
+1 -9
View File
@@ -68,23 +68,15 @@ export async function projectHostAuthenticationError(
const OWNER_SLUG_RE = /^[A-Za-z0-9][A-Za-z0-9-]*$/
const REPO_SLUG_RE = /^[A-Za-z0-9._-]+$/
const REPO_SLUG_RESERVED = new Set(['.', '..'])
const OWNER_SLUG_MAX_CHARS = 256
const REPO_SLUG_MAX_CHARS = 1024
export function isValidOwnerSlug(value: unknown): value is string {
return (
typeof value === 'string' &&
value.length > 0 &&
value.length <= OWNER_SLUG_MAX_CHARS &&
OWNER_SLUG_RE.test(value)
)
return typeof value === 'string' && value.length > 0 && OWNER_SLUG_RE.test(value)
}
export function isValidRepoSlug(value: unknown): value is string {
return (
typeof value === 'string' &&
value.length > 0 &&
value.length <= REPO_SLUG_MAX_CHARS &&
REPO_SLUG_RE.test(value) &&
!REPO_SLUG_RESERVED.has(value)
)
+7 -18
View File
@@ -22,7 +22,6 @@ import {
registerGhRateLimitResetProbe,
type GhRateLimitBucket
} from '../git/gh-rate-limit-breaker'
import { cacheIdentityDigest } from '../cache-identity-digest'
// Why: GET /rate_limit is exempt from limits, so caching only avoids a gh subprocess per render; 30s stays live while absorbing 1/s polling.
const RATE_LIMIT_CACHE_TTL_MS = 30_000
@@ -197,21 +196,15 @@ const DEFAULT_BREAKER_SCOPE = ghRateLimitScopeKey('native', 'github.com')
const scopeRefinementInFlight = new Map<string, Promise<void>>()
const scopeProbeFailureAtMs = new Map<string, number>()
const SCOPE_PROBE_FAILURE_MAX_ENTRIES = 512
const SCOPE_REFINEMENT_MAX_IN_FLIGHT = 16
function scopeRetentionKey(scope: string): string {
return cacheIdentityDigest([scope])
}
function rememberScopeProbeFailure(scope: string, failedAt: number): void {
const retentionKey = scopeRetentionKey(scope)
for (const [key, at] of scopeProbeFailureAtMs) {
if (failedAt - at >= RATE_LIMIT_CACHE_TTL_MS) {
scopeProbeFailureAtMs.delete(key)
}
}
scopeProbeFailureAtMs.delete(retentionKey)
scopeProbeFailureAtMs.set(retentionKey, failedAt)
scopeProbeFailureAtMs.delete(scope)
scopeProbeFailureAtMs.set(scope, failedAt)
while (scopeProbeFailureAtMs.size > SCOPE_PROBE_FAILURE_MAX_ENTRIES) {
const oldestKey = scopeProbeFailureAtMs.keys().next().value
if (oldestKey === undefined) {
@@ -228,20 +221,16 @@ function refineBreakerForScope(scope: string): void {
return
}
const parts = parseGhRateLimitScopeKey(scope)
const retentionKey = scopeRetentionKey(scope)
if (!parts || scopeRefinementInFlight.has(retentionKey)) {
if (!parts || scopeRefinementInFlight.has(scope)) {
return
}
// Why: GHES with rate limiting disabled 404s every probe. Fail open (the
// fallback block stands) and don't re-probe in a tight loop while the
// breaker keeps tripping.
const failedAt = scopeProbeFailureAtMs.get(retentionKey)
const failedAt = scopeProbeFailureAtMs.get(scope)
if (failedAt !== undefined && Date.now() - failedAt < RATE_LIMIT_CACHE_TTL_MS) {
return
}
if (scopeRefinementInFlight.size >= SCOPE_REFINEMENT_MAX_IN_FLIGHT) {
return
}
const probe = (async () => {
try {
await acquire()
@@ -254,7 +243,7 @@ function refineBreakerForScope(scope: string): void {
...(parts.runtime === 'wsl' ? { wslDistro: parts.wslDistro } : {})
})
const parsed = JSON.parse(stdout) as GhRateLimitPayload
scopeProbeFailureAtMs.delete(retentionKey)
scopeProbeFailureAtMs.delete(scope)
// Why: mirrors the default-scope refinement, but records into the
// per-scope breaker only — the shared snapshot must keep describing
// native github.com exclusively.
@@ -274,10 +263,10 @@ function refineBreakerForScope(scope: string): void {
// negative cache so repeated failing hosts cannot accumulate forever.
rememberScopeProbeFailure(scope, Date.now())
} finally {
scopeRefinementInFlight.delete(retentionKey)
scopeRefinementInFlight.delete(scope)
}
})()
scopeRefinementInFlight.set(retentionKey, probe)
scopeRefinementInFlight.set(scope, probe)
}
registerGhRateLimitResetProbe((_bucket, scope) => refineBreakerForScope(scope))
@@ -1,33 +0,0 @@
import { describe, expect, it } from 'vitest'
import {
_parseTrackedUpstreamBranchesForTests,
TRACKED_UPSTREAM_SNAPSHOT_MAX_BRANCHES,
TRACKED_UPSTREAM_SNAPSHOT_MAX_BYTES
} from './client'
describe('tracked upstream snapshot bounds', () => {
it('caps branch count while preserving the currently requested branch', () => {
const requested = `branch-${TRACKED_UPSTREAM_SNAPSHOT_MAX_BRANCHES}`
const stdout = Array.from(
{ length: TRACKED_UPSTREAM_SNAPSHOT_MAX_BRANCHES + 1 },
(_, index) => `refs/heads/branch-${index}\0refs/remotes/origin/branch-${index}\n`
).join('')
const parsed = _parseTrackedUpstreamBranchesForTests(stdout, requested)
expect(parsed.size).toBe(TRACKED_UPSTREAM_SNAPSHOT_MAX_BRANCHES)
expect(parsed.get(requested)).toEqual({
remoteName: 'origin',
branchName: requested
})
expect(parsed.has('branch-0')).toBe(false)
})
it('skips an individual branch that cannot fit the byte budget', () => {
const oversized = 'x'.repeat(TRACKED_UPSTREAM_SNAPSHOT_MAX_BYTES + 1)
const parsed = _parseTrackedUpstreamBranchesForTests(
`refs/heads/${oversized}\0refs/remotes/origin/main\n`,
oversized
)
expect(parsed.size).toBe(0)
})
})