From b6a24ecac12f196979a9253c8617a5bd7182429f Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 24 Aug 2026 20:46:07 -0700 Subject: [PATCH] refactor: split GitHub backend modules (#16194) --- config/max-lines-baseline.txt | 3 - src/main/github/issue-timeline.ts | 189 +++ src/main/github/issue-work-item-details.ts | 260 ++++ .../github/pr-refresh-candidate-policy.ts | 183 +++ ...resh-coordinator-rate-limit-budget.test.ts | 54 + ...refresh-coordinator-refresh-events.test.ts | 77 + src/main/github/pr-refresh-coordinator.ts | 963 +----------- src/main/github/pr-refresh-event-publisher.ts | 69 + src/main/github/pr-refresh-pacing.ts | 114 ++ src/main/github/pr-refresh-queue-drainer.ts | 242 +++ src/main/github/pr-refresh-queue.ts | 225 +++ src/main/github/pr-refresh-rate-limit-gate.ts | 27 + src/main/github/pr-refresh-retry-state.ts | 46 + src/main/github/pr-refresh-visibility.ts | 44 + src/main/github/pull-request-file-contents.ts | 132 ++ src/main/github/pull-request-file-data.ts | 247 +++ .../work-item-details-api-parity.test.ts | 37 + .../github/work-item-details-pr-files.test.ts | 30 + src/main/github/work-item-details.ts | 1143 +------------- src/main/github/work-item-participants.ts | 241 +++ src/main/ipc/github-account-handlers.ts | 31 + .../ipc/github-ipc-channel-parity.test.ts | 100 ++ .../ipc/github-issue-mutation-handlers.ts | 106 ++ src/main/ipc/github-pr-mutation-handlers.ts | 212 +++ src/main/ipc/github-pr-read-handlers.ts | 178 +++ src/main/ipc/github-pr-refresh-handlers.ts | 161 ++ src/main/ipc/github-pr-review-handlers.ts | 180 +++ src/main/ipc/github-project-view-handlers.ts | 89 ++ src/main/ipc/github-repo-routing.ts | 130 ++ src/main/ipc/github-work-item-handlers.ts | 214 +++ .../ipc/github-work-item-mutation-events.ts | 15 + src/main/ipc/github.ts | 1323 +---------------- 32 files changed, 3745 insertions(+), 3320 deletions(-) create mode 100644 src/main/github/issue-timeline.ts create mode 100644 src/main/github/issue-work-item-details.ts create mode 100644 src/main/github/pr-refresh-candidate-policy.ts create mode 100644 src/main/github/pr-refresh-event-publisher.ts create mode 100644 src/main/github/pr-refresh-pacing.ts create mode 100644 src/main/github/pr-refresh-queue-drainer.ts create mode 100644 src/main/github/pr-refresh-queue.ts create mode 100644 src/main/github/pr-refresh-rate-limit-gate.ts create mode 100644 src/main/github/pr-refresh-retry-state.ts create mode 100644 src/main/github/pr-refresh-visibility.ts create mode 100644 src/main/github/pull-request-file-contents.ts create mode 100644 src/main/github/pull-request-file-data.ts create mode 100644 src/main/github/work-item-details-api-parity.test.ts create mode 100644 src/main/github/work-item-participants.ts create mode 100644 src/main/ipc/github-account-handlers.ts create mode 100644 src/main/ipc/github-ipc-channel-parity.test.ts create mode 100644 src/main/ipc/github-issue-mutation-handlers.ts create mode 100644 src/main/ipc/github-pr-mutation-handlers.ts create mode 100644 src/main/ipc/github-pr-read-handlers.ts create mode 100644 src/main/ipc/github-pr-refresh-handlers.ts create mode 100644 src/main/ipc/github-pr-review-handlers.ts create mode 100644 src/main/ipc/github-project-view-handlers.ts create mode 100644 src/main/ipc/github-repo-routing.ts create mode 100644 src/main/ipc/github-work-item-handlers.ts create mode 100644 src/main/ipc/github-work-item-mutation-events.ts diff --git a/config/max-lines-baseline.txt b/config/max-lines-baseline.txt index 9e24dc1b39f..f2a9bebb747 100644 --- a/config/max-lines-baseline.txt +++ b/config/max-lines-baseline.txt @@ -27,14 +27,11 @@ inline src/main/daemon/pty-subprocess.ts inline src/main/git/runner.ts inline src/main/git/status.ts inline src/main/git/worktree.ts -inline src/main/github/pr-refresh-coordinator.ts inline src/main/github/project-view.ts inline src/main/github/project-view/mutations.ts -inline src/main/github/work-item-details.ts inline src/main/index.ts inline src/main/ipc/filesystem-watcher.ts inline src/main/ipc/filesystem.ts -inline src/main/ipc/github.ts inline src/main/ipc/repos.ts inline src/main/ipc/ssh.ts inline src/main/ipc/worktree-remote.ts diff --git a/src/main/github/issue-timeline.ts b/src/main/github/issue-timeline.ts new file mode 100644 index 00000000000..3cc1f36eaac --- /dev/null +++ b/src/main/github/issue-timeline.ts @@ -0,0 +1,189 @@ +import type { + GitHubIssueTimelineItem, + GitHubIssueTimelineTarget +} from '../../shared/github/comment-types' +import { ghExecFileAsync } from './gh-utils' +import type { GitHubApiRepository, GitHubRepoExecOptions } from './github-api-repository' +import { githubHostExecOptions } from './github-api-repository' +import { noteRepositoryRateLimitSpend, repositoryRateLimitGuard } from './rate-limit' + +const MAX_ISSUE_TIMELINE_ITEMS = 300 +const GITHUB_REST_PAGE_SIZE = 100 + +type RestTimelineUser = { + login?: string | null + avatar_url?: string | null +} + +type RestTimelineIssue = { + number?: number | null + title?: string | null + html_url?: string | null + repository?: { + name?: string | null + owner?: { login?: string | null } | null + } | null + pull_request?: unknown +} + +type RestTimelineEvent = { + id?: number | string | null + node_id?: string | null + event?: string | null + actor?: RestTimelineUser | null + user?: RestTimelineUser | null + assignee?: RestTimelineUser | null + created_at?: string | null + source?: { issue?: RestTimelineIssue | null } | null + closer?: RestTimelineIssue | null + state_reason?: string | null + project_card?: { + column_name?: string | null + previous_column_name?: string | null + project_url?: string | null + } | null + project?: { name?: string | null } | null + project_column_name?: string | null + previous_column_name?: string | null +} + +function isSupportedTimelineEvent( + eventName: string | null | undefined +): eventName is GitHubIssueTimelineItem['event'] { + return ( + eventName === 'assigned' || + eventName === 'unassigned' || + eventName === 'mentioned' || + eventName === 'cross-referenced' || + eventName === 'closed' || + eventName === 'reopened' || + eventName === 'moved_columns_in_project' + ) +} + +function mapTimelineTarget( + issue: RestTimelineIssue | null | undefined +): GitHubIssueTimelineTarget | undefined { + if (!issue || typeof issue.number !== 'number' || !issue.html_url) { + return undefined + } + const owner = issue.repository?.owner?.login + const repo = issue.repository?.name + return { + type: issue.pull_request ? 'pr' : 'issue', + number: issue.number, + title: issue.title ?? '', + url: issue.html_url, + repository: owner && repo ? `${owner}/${repo}` : undefined + } +} + +function getTimelineActor(event: RestTimelineEvent): { login: string; avatarUrl: string } { + const actor = event.actor ?? event.user + return { + login: actor?.login ?? 'ghost', + avatarUrl: actor?.avatar_url ?? '' + } +} + +function mapRestTimelineEvent(event: RestTimelineEvent): GitHubIssueTimelineItem | null { + const eventName = event.event + if (!isSupportedTimelineEvent(eventName) || !event.created_at) { + return null + } + const actor = getTimelineActor(event) + const id = String(event.node_id ?? event.id ?? `${eventName}:${event.created_at}`) + const base = { + id, + event: eventName, + actor: actor.login, + actorAvatarUrl: actor.avatarUrl, + createdAt: event.created_at + } + if (eventName === 'assigned' || eventName === 'unassigned') { + return { ...base, assignee: event.assignee?.login ?? undefined } + } + if (eventName === 'mentioned' || eventName === 'cross-referenced') { + return { ...base, source: mapTimelineTarget(event.source?.issue) } + } + if (eventName === 'closed') { + return { + ...base, + stateReason: event.state_reason ?? null, + closer: mapTimelineTarget(event.closer ?? event.source?.issue) + } + } + if (eventName === 'moved_columns_in_project') { + return { + ...base, + previousColumnName: + event.previous_column_name ?? event.project_card?.previous_column_name ?? null, + columnName: event.project_column_name ?? event.project_card?.column_name ?? null, + projectName: event.project?.name ?? null + } + } + return base +} + +function parseRestTimelineEventLines(stdout: string): RestTimelineEvent[] { + const events: RestTimelineEvent[] = [] + for (const line of stdout.split('\n')) { + const trimmed = line.trim() + if (!trimmed) { + continue + } + try { + const parsed = JSON.parse(trimmed) as unknown + if (typeof parsed === 'object' && parsed !== null && !Array.isArray(parsed)) { + events.push(parsed) + } + } catch { + // Timeline activity is auxiliary to issue details. + } + } + return events +} + +export async function getIssueTimelineItems( + repository: GitHubApiRepository, + issueNumber: number, + ghOptions: GitHubRepoExecOptions +): Promise { + try { + const items: GitHubIssueTimelineItem[] = [] + for (let page = 1; items.length < MAX_ISSUE_TIMELINE_ITEMS; page += 1) { + if (repositoryRateLimitGuard(repository, 'core', ghOptions).blocked) { + return items + } + noteRepositoryRateLimitSpend(repository, 'core', 1, ghOptions) + const { stdout } = await ghExecFileAsync( + [ + 'api', + '--cache', + '60s', + `repos/${repository.owner}/${repository.repo}/issues/${issueNumber}/timeline?per_page=${GITHUB_REST_PAGE_SIZE}&page=${page}`, + '--jq', + '.[] | @json' + ], + { ...ghOptions, ...githubHostExecOptions(repository) } + ) + const pageEvents = parseRestTimelineEventLines(stdout) + for (const event of pageEvents) { + const item = mapRestTimelineEvent(event) + if (!item) { + continue + } + items.push(item) + if (items.length === MAX_ISSUE_TIMELINE_ITEMS) { + break + } + } + if (pageEvents.length < GITHUB_REST_PAGE_SIZE) { + break + } + } + return items + } catch { + return [] + } +} diff --git a/src/main/github/issue-work-item-details.ts b/src/main/github/issue-work-item-details.ts new file mode 100644 index 00000000000..d58491abd87 --- /dev/null +++ b/src/main/github/issue-work-item-details.ts @@ -0,0 +1,260 @@ +import type { GitHubIssueTimelineItem, PRComment } from '../../shared/github/comment-types' +import type { GitHubAssignableUser } from '../../shared/github/pull-request-types' +import { ghExecFileAsync, ghRepoExecOptions, githubRepoContext } from './gh-utils' +import type { LocalGitExecOptions } from './gh-utils' +import { githubHostExecOptions, type GitHubApiRepository } from './github-api-repository' +import { getIssueTimelineItems } from './issue-timeline' +import { noteRepositoryRateLimitSpend, repositoryRateLimitGuard } from './rate-limit' + +const ISSUE_DETAILS_QUERY = `query($owner: String!, $repo: String!, $number: Int!) { + repository(owner: $owner, name: $repo) { + issue(number: $number) { + body + assignees(first: 50) { nodes { login avatarUrl(size: 48) ... on User { name } } } + participants(first: 100) { + nodes { login avatarUrl(size: 48) ... on User { name } } + } + comments(first: 100) { + nodes { + databaseId + body + createdAt + url + author { + login + avatarUrl(size: 48) + ... on Bot { __typename } + } + } + } + } + } +}` + +type GraphQLIssueDetailsResponse = { + data?: { + repository?: { + issue?: { + body?: string | null + assignees?: { nodes?: { login?: string; avatarUrl?: string; name?: string | null }[] } + participants?: { nodes?: GitHubAssignableUser[] } + comments?: { + nodes?: { + databaseId?: number | null + body?: string | null + createdAt?: string | null + url?: string | null + author?: { + login?: string | null + avatarUrl?: string | null + __typename?: string + } | null + }[] + } + } | null + } | null + } + errors?: { message?: string }[] +} + +export type CollapsedIssueDetails = { + body: string + comments: PRComment[] + assignees: string[] + assigneeUsers: GitHubAssignableUser[] + participants: GitHubAssignableUser[] + timelineItems: GitHubIssueTimelineItem[] +} + +export async function getIssueDetailsViaGraphQL( + repoPath: string, + issueNumber: number, + repository: GitHubApiRepository | null, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + const ghOptions = { + ...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)), + ...githubHostExecOptions(repository) + } + if (!repository) { + return null + } + if (repositoryRateLimitGuard(repository, 'graphql', ghOptions).blocked) { + return null + } + try { + noteRepositoryRateLimitSpend(repository, 'graphql', 1, ghOptions) + const { stdout } = await ghExecFileAsync( + [ + 'api', + 'graphql', + '-f', + `query=${ISSUE_DETAILS_QUERY}`, + '-f', + `owner=${repository.owner}`, + '-f', + `repo=${repository.repo}`, + '-F', + `number=${issueNumber}` + ], + ghOptions + ) + const parsed = JSON.parse(stdout) as GraphQLIssueDetailsResponse + if (parsed.errors && parsed.errors.length > 0) { + return null + } + const issue = parsed.data?.repository?.issue + if (!issue) { + return null + } + const comments: PRComment[] = (issue.comments?.nodes ?? []) + .filter((comment) => typeof comment.databaseId === 'number') + .map((comment) => ({ + id: comment.databaseId as number, + author: comment.author?.login ?? 'ghost', + authorAvatarUrl: comment.author?.avatarUrl ?? '', + body: comment.body ?? '', + createdAt: comment.createdAt ?? '', + url: comment.url ?? '', + isBot: comment.author?.__typename === 'Bot' + })) + const assigneeUsers: GitHubAssignableUser[] = (issue.assignees?.nodes ?? []) + .filter((assignee): assignee is { login: string; avatarUrl?: string; name?: string | null } => + Boolean(assignee.login) + ) + .map((assignee) => ({ + login: assignee.login, + name: assignee.name ?? null, + avatarUrl: assignee.avatarUrl ?? '' + })) + const participants: GitHubAssignableUser[] = (issue.participants?.nodes ?? []) + .filter((user) => Boolean(user.login)) + .map((user) => ({ + login: user.login, + name: user.name ?? null, + avatarUrl: user.avatarUrl ?? '' + })) + return { + body: issue.body ?? '', + comments, + assignees: assigneeUsers.map((assignee) => assignee.login), + assigneeUsers, + participants, + timelineItems: await getIssueTimelineItems(repository, issueNumber, ghOptions) + } + } catch { + return null + } +} + +export async function getIssueBodyAndComments( + repoPath: string, + issueNumber: number, + repository: GitHubApiRepository | null, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise<{ + body: string + comments: PRComment[] + assignees: string[] + timelineItems: GitHubIssueTimelineItem[] +}> { + const ghOptions = { + ...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)), + ...githubHostExecOptions(repository) + } + try { + if (repository) { + if (repositoryRateLimitGuard(repository, 'core', ghOptions).blocked) { + return { body: '', comments: [], assignees: [], timelineItems: [] } + } + noteRepositoryRateLimitSpend(repository, 'core', 2, ghOptions) + const [issueResult, commentsResult, timelineItems] = await Promise.all([ + ghExecFileAsync( + [ + 'api', + '--cache', + '60s', + `repos/${repository.owner}/${repository.repo}/issues/${issueNumber}` + ], + ghOptions + ), + ghExecFileAsync( + [ + 'api', + '--cache', + '60s', + `repos/${repository.owner}/${repository.repo}/issues/${issueNumber}/comments?per_page=100` + ], + ghOptions + ), + getIssueTimelineItems(repository, issueNumber, ghOptions) + ]) + const issue = JSON.parse(issueResult.stdout) as { + body?: string | null + assignees?: { login: string }[] + } + type RESTComment = { + id: number + user: { login: string; avatar_url: string; type?: string } | null + body: string + created_at: string + html_url: string + } + const comments = (JSON.parse(commentsResult.stdout) as RESTComment[]).map( + (comment): PRComment => ({ + id: comment.id, + author: comment.user?.login ?? 'ghost', + authorAvatarUrl: comment.user?.avatar_url ?? '', + body: comment.body ?? '', + createdAt: comment.created_at, + url: comment.html_url, + isBot: comment.user?.type === 'Bot' + }) + ) + return { + body: issue.body ?? '', + comments, + assignees: (issue.assignees ?? []).map((assignee) => assignee.login), + timelineItems + } + } + if (connectionId) { + // A connection-backed lookup must not use ambient local GH_REPO/GH_HOST. + return { body: '', comments: [], assignees: [], timelineItems: [] } + } + const { stdout } = await ghExecFileAsync( + ['issue', 'view', String(issueNumber), '--json', 'body,comments,assignees'], + ghOptions + ) + const data = JSON.parse(stdout) as { + body?: string + comments?: { + author: { login: string } + body: string + createdAt: string + url: string + }[] + assignees?: { login: string }[] + } + const comments = (data.comments ?? []).map( + (comment, index): PRComment => ({ + id: index, + author: comment.author?.login ?? 'ghost', + authorAvatarUrl: '', + body: comment.body ?? '', + createdAt: comment.createdAt, + url: comment.url ?? '' + }) + ) + return { + body: data.body ?? '', + comments, + assignees: (data.assignees ?? []).map((assignee) => assignee.login), + timelineItems: [] + } + } catch { + return { body: '', comments: [], assignees: [], timelineItems: [] } + } +} diff --git a/src/main/github/pr-refresh-candidate-policy.ts b/src/main/github/pr-refresh-candidate-policy.ts new file mode 100644 index 00000000000..e2868edf051 --- /dev/null +++ b/src/main/github/pr-refresh-candidate-policy.ts @@ -0,0 +1,183 @@ +import type { + GitHubPRRefreshAlias, + GitHubPRRefreshCandidate, + GitHubPRRefreshReason, + GitHubPRRefreshSkippedReason, + PRRefreshOutcome +} from '../../shared/github/pull-request-refresh-types' +import type { GitHubPRBranchLookupOptions } from './client' +import { NO_REVIEW_REFRESH_INTERVAL_MS } from '../source-control/hosted-review-refresh-pacing' + +export const MANUAL_MERGEABILITY_PENDING_REFRESH_MS = 2_500 +export const POST_PUSH_DELAY_MS = 2_500 + +type PRBranchLookupCandidate = Pick< + GitHubPRRefreshCandidate, + 'localGitOptions' | 'linkedPRNumber' | 'fallbackPRNumber' | 'fallbackPRSource' | 'currentHeadOid' +> + +function shouldAcceptMergedFallbackPR(candidate: PRBranchLookupCandidate): boolean { + return ( + candidate.linkedPRNumber == null && + candidate.fallbackPRNumber != null && + candidate.fallbackPRSource != null + ) +} + +export function hostedReviewOptionArgs( + candidate: PRBranchLookupCandidate +): [] | [GitHubPRBranchLookupOptions] { + const options: GitHubPRBranchLookupOptions = {} + if (candidate.localGitOptions?.wslDistro) { + options.localGitExecOptions = { wslDistro: candidate.localGitOptions.wslDistro } + } + if (shouldAcceptMergedFallbackPR(candidate)) { + options.acceptMergedFallbackPR = true + } + if (typeof candidate.currentHeadOid === 'string' && candidate.currentHeadOid.trim().length > 0) { + options.currentHeadOid = candidate.currentHeadOid.trim() + } + return Object.keys(options).length > 0 ? [options] : [] +} + +export function refreshKey(candidate: GitHubPRRefreshCandidate): string { + const connectionScope = candidate.connectionId ?? 'local' + const runtimeScope = candidate.connectionId + ? 'remote' + : `runtime:${candidate.localGitOptions?.wslDistro ? `wsl:${candidate.localGitOptions.wslDistro}` : 'host'}` + if (typeof candidate.linkedPRNumber === 'number') { + return `${connectionScope}::${runtimeScope}::${candidate.repoPath}::pr::${candidate.linkedPRNumber}` + } + return `${connectionScope}::${runtimeScope}::${candidate.repoPath}::branch::${candidate.branch}` +} + +export function validateCandidate( + candidate: GitHubPRRefreshCandidate +): GitHubPRRefreshSkippedReason | null { + if (candidate.repoKind !== 'git') { + return 'not-git' + } + if (candidate.isBare) { + return 'bare' + } + if (candidate.isArchived) { + return 'archived' + } + if (candidate.connectionId && candidate.connectionState === 'disconnected') { + return 'disconnected' + } + if (!candidate.branch && typeof candidate.linkedPRNumber !== 'number') { + return 'fresh' + } + return null +} + +export function bypassesFreshnessDelay(reason: GitHubPRRefreshReason): boolean { + return reason === 'manual' || reason === 'active' || reason === 'post-push' +} + +export function isBackground(reason: GitHubPRRefreshReason): boolean { + return reason !== 'manual' +} + +export function isBudgetedBackground(reason: GitHubPRRefreshReason): boolean { + return reason === 'visible' || reason === 'swr' +} + +export function shouldBroadcastQueued(reason: GitHubPRRefreshReason, dueAt: number): boolean { + if (isBudgetedBackground(reason)) { + return false + } + const delay = dueAt - Date.now() + return delay > 0 && delay <= 5_000 +} + +export function shouldSkipFresh( + candidate: GitHubPRRefreshCandidate, + reason: GitHubPRRefreshReason +): boolean { + if (bypassesFreshnessDelay(reason) || candidate.cachedFetchedAt == null) { + return false + } + return Date.now() - candidate.cachedFetchedAt < refreshIntervalForCandidate(candidate) +} + +export function freshRetryAt(candidate: GitHubPRRefreshCandidate): number | null { + return candidate.cachedFetchedAt == null + ? null + : candidate.cachedFetchedAt + refreshIntervalForCandidate(candidate) +} + +export function aliasFromCandidate(candidate: GitHubPRRefreshCandidate): GitHubPRRefreshAlias { + return { + cacheKey: candidate.cacheKey, + repoId: candidate.repoId, + repoPath: candidate.repoPath, + branch: candidate.branch, + worktreeId: candidate.worktreeId, + connectionId: candidate.connectionId ?? null, + currentHeadOid: candidate.currentHeadOid ?? null, + linkedPRNumber: candidate.linkedPRNumber ?? null, + fallbackPRNumber: + candidate.linkedPRNumber == null ? (candidate.fallbackPRNumber ?? null) : null, + fallbackPRSource: candidate.linkedPRNumber == null ? (candidate.fallbackPRSource ?? null) : null + } +} + +export function visibleCandidateAfterOutcome( + candidate: GitHubPRRefreshCandidate, + outcome: PRRefreshOutcome +): GitHubPRRefreshCandidate { + if (outcome.kind === 'upstream-error') { + return candidate + } + return { + ...candidate, + cachedFetchedAt: outcome.fetchedAt, + cachedHasPR: outcome.kind === 'found', + cachedPRState: outcome.kind === 'found' ? outcome.pr.state : null, + cachedChecksStatus: outcome.kind === 'found' ? outcome.pr.checksStatus : null, + cachedMergeable: outcome.kind === 'found' ? outcome.pr.mergeable : null, + cachedMergeStateStatus: outcome.kind === 'found' ? (outcome.pr.mergeStateStatus ?? null) : null + } +} + +function refreshIntervalForCandidate(candidate: GitHubPRRefreshCandidate): number { + if (candidate.cachedPRState === 'closed' || candidate.cachedPRState === 'merged') { + return 30 * 60_000 + } + if (candidate.cachedHasPR === false) { + return NO_REVIEW_REFRESH_INTERVAL_MS + } + if ( + candidate.cachedHasPR === true && + candidate.cachedPRState === 'open' && + candidate.cachedMergeable === 'UNKNOWN' && + !hasResolvedMergeStateStatus(candidate.cachedMergeStateStatus) + ) { + return 10_000 + } + if (candidate.cachedChecksStatus === 'success') { + return 10 * 60_000 + } + if (candidate.cachedChecksStatus === 'failure') { + return 3 * 60_000 + } + if (candidate.cachedChecksStatus === 'pending') { + return 90_000 + } + return 60_000 +} + +function hasResolvedMergeStateStatus(status: string | null | undefined): boolean { + return status === 'CLEAN' || status === 'BEHIND' || status === 'BLOCKED' +} + +export function isMergeabilityPendingOutcome(outcome: PRRefreshOutcome): boolean { + return ( + outcome.kind === 'found' && + outcome.pr.state === 'open' && + outcome.pr.mergeable === 'UNKNOWN' && + !hasResolvedMergeStateStatus(outcome.pr.mergeStateStatus) + ) +} diff --git a/src/main/github/pr-refresh-coordinator-rate-limit-budget.test.ts b/src/main/github/pr-refresh-coordinator-rate-limit-budget.test.ts index 8d81faeb0f7..29b5309d26b 100644 --- a/src/main/github/pr-refresh-coordinator-rate-limit-budget.test.ts +++ b/src/main/github/pr-refresh-coordinator-rate-limit-budget.test.ts @@ -164,4 +164,58 @@ describe('pr-refresh-coordinator', () => { 'feature/0' ]) }) + + it.each([ + { primaryResetAt: 4, retryDisabledUntil: 5_000, expectedGateUntil: 5_000 }, + { primaryResetAt: 6, retryDisabledUntil: 5_000, expectedGateUntil: 6_000 } + ])( + 'uses the later manual gate when the primary reset is $primaryResetAt seconds', + async ({ primaryResetAt, retryDisabledUntil, expectedGateUntil }) => { + const { refreshPRNow, reportVisiblePRRefreshCandidates } = + await import('./pr-refresh-coordinator') + const candidate = makeCandidate() + getPRForBranchOutcomeMock.mockResolvedValueOnce({ + kind: 'upstream-error', + errorType: 'rate_limited', + message: 'limited', + fetchedAt: Date.now(), + retryDisabledUntil + }) + + reportVisiblePRRefreshCandidates([candidate], 1, 1) + await refreshPRNow(candidate) + getPRForBranchOutcomeMock.mockClear() + repositoryRateLimitGuardMock.mockImplementation((_repository, bucket) => + bucket === 'core' + ? { blocked: true, remaining: 0, limit: 5_000, resetAt: primaryResetAt } + : { blocked: false } + ) + + const outcome = await refreshPRNow(candidate) + + expect(outcome).toEqual({ + kind: 'upstream-error', + errorType: 'rate_limited', + message: 'GitHub is temporarily limiting requests. Try again after the limit resets.', + fetchedAt: 1_000, + nextAutoRetryAt: expectedGateUntil, + retryDisabledUntil: expectedGateUntil + }) + expect(getPRForBranchOutcomeMock).not.toHaveBeenCalled() + expect(sendMock.mock.calls.map(([, event]) => event).at(-1)).toEqual( + expect.objectContaining({ + reason: 'manual', + status: 'paused', + pausedUntil: expectedGateUntil, + skippedReason: 'rate-limit' + }) + ) + + getPRForBranchOutcomeMock.mockResolvedValue({ kind: 'no-pr', fetchedAt: expectedGateUntil }) + await vi.advanceTimersByTimeAsync(expectedGateUntil - Date.now() - 1) + expect(getPRForBranchOutcomeMock).not.toHaveBeenCalled() + await vi.advanceTimersByTimeAsync(1) + expect(getPRForBranchOutcomeMock).toHaveBeenCalledOnce() + } + ) }) diff --git a/src/main/github/pr-refresh-coordinator-refresh-events.test.ts b/src/main/github/pr-refresh-coordinator-refresh-events.test.ts index 9d8d0745645..45a026dabbd 100644 --- a/src/main/github/pr-refresh-coordinator-refresh-events.test.ts +++ b/src/main/github/pr-refresh-coordinator-refresh-events.test.ts @@ -27,6 +27,43 @@ describe('pr-refresh-coordinator', () => { vi.useRealTimers() }) + it('preserves the coordinator public module API', async () => { + const coordinator = await import('./pr-refresh-coordinator') + + expect(Object.keys(coordinator).sort()).toEqual([ + '_getPRRefreshAliasCountForTests', + '_getPRRefreshErrorBackoffCountForTests', + '_getPRRefreshQueueSizeForTests', + '_getVisiblePRRefreshWindowCountForTests', + 'clearVisiblePRRefreshWindow', + 'enqueuePRRefresh', + 'pruneWorktreePRRefreshAliases', + 'refreshPRNow', + 'reportVisiblePRRefreshCandidates', + 'setPRRefreshOutcomeObserver' + ]) + }) + + it('ignores a stale visibility generation before it can replace queued work', async () => { + const { reportVisiblePRRefreshCandidates } = await import('./pr-refresh-coordinator') + getPRForBranchOutcomeMock.mockResolvedValue({ kind: 'no-pr', fetchedAt: Date.now() }) + + reportVisiblePRRefreshCandidates( + [makeCandidate({ branch: 'feature/current', cacheKey: '/repo::feature/current' })], + 2, + 1 + ) + reportVisiblePRRefreshCandidates( + [makeCandidate({ branch: 'feature/stale', cacheKey: '/repo::feature/stale' })], + 1, + 1 + ) + await vi.advanceTimersByTimeAsync(0) + + expect(getPRForBranchOutcomeMock).toHaveBeenCalledOnce() + expect(getPRForBranchOutcomeMock.mock.calls[0]?.[1]).toBe('feature/current') + }) + it('sends each refresh event once without broadcasting to 100 browser guests', async () => { const guestSends = Array.from({ length: 100 }, () => vi.fn()) getAllWebContentsMock.mockReturnValue( @@ -109,6 +146,46 @@ describe('pr-refresh-coordinator', () => { expect(outcome?.sequence).toBe(inFlight?.sequence) }) + it('keeps each manual outcome on its request sequence when completions race', async () => { + const { refreshPRNow } = await import('./pr-refresh-coordinator') + let resolveFirst!: (outcome: ReturnType) => void + let resolveSecond!: (outcome: ReturnType) => void + const first = new Promise>((resolve) => { + resolveFirst = resolve + }) + const second = new Promise>((resolve) => { + resolveSecond = resolve + }) + getPRForBranchOutcomeMock + .mockImplementationOnce(async () => ({ + kind: 'found', + pr: await first, + fetchedAt: Date.now() + })) + .mockImplementationOnce(async () => ({ + kind: 'found', + pr: await second, + fetchedAt: Date.now() + })) + + const firstRefresh = refreshPRNow(makeCandidate()) + const secondRefresh = refreshPRNow(makeCandidate()) + await vi.advanceTimersByTimeAsync(0) + resolveSecond(makePR({ number: 22 })) + await secondRefresh + resolveFirst(makePR({ number: 11 })) + await firstRefresh + + const events = sendMock.mock.calls.map(([, event]) => event) + const inFlight = events.filter((event) => event.status === 'in-flight') + const outcomes = events.filter((event) => event.outcome) + const firstOutcome = outcomes.find((event) => event.outcome.pr?.number === 11) + const secondOutcome = outcomes.find((event) => event.outcome.pr?.number === 22) + expect(firstOutcome?.sequence).toBe(inFlight[0]?.sequence) + expect(secondOutcome?.sequence).toBe(inFlight[1]?.sequence) + expect(firstOutcome?.sequence).toBeLessThan(secondOutcome?.sequence) + }) + it('accepts merged fallback PRs for visible fallback refreshes', async () => { const { refreshPRNow } = await import('./pr-refresh-coordinator') getPRForBranchOutcomeMock.mockResolvedValueOnce({ diff --git a/src/main/github/pr-refresh-coordinator.ts b/src/main/github/pr-refresh-coordinator.ts index 2ff9d0e410b..22846cbe17c 100644 --- a/src/main/github/pr-refresh-coordinator.ts +++ b/src/main/github/pr-refresh-coordinator.ts @@ -1,817 +1,61 @@ -/* eslint-disable max-lines -- Why: queueing, pacing, and broadcast rules stay together so freshness and rate-limit invariants are reviewable in one place. */ -import { webContents } from 'electron' import type { - GitHubPRRefreshAlias, GitHubPRRefreshCandidate, - GitHubPRRefreshEvent, GitHubPRRefreshReason, - GitHubPRRefreshSkippedReason, PRRefreshOutcome } from '../../shared/github/pull-request-refresh-types' -import { getPRForBranchOutcome, type GitHubPRBranchLookupOptions } from './client' -import { getOriginGitHubApiRepository } from './github-api-repository' -import { ghRepoExecOptions, githubRepoContext } from './gh-utils' -import { getRateLimit, repositoryRateLimitGuard, spendsSharedGitHubComQuota } from './rate-limit' +import { getPRForBranchOutcome } from './client' import { - lookupBackoffDelayMs, - NO_REVIEW_REFRESH_INTERVAL_MS -} from '../source-control/hosted-review-refresh-pacing' -import { recordCoalescedCrashBreadcrumb } from '../crash-reporting/crash-breadcrumb-store' -import { sendToTrustedUIRenderer } from '../ipc/ui' + aliasFromCandidate, + hostedReviewOptionArgs, + MANUAL_MERGEABILITY_PENDING_REFRESH_MS, + refreshKey, + shouldBroadcastQueued, + validateCandidate +} from './pr-refresh-candidate-policy' +import { + PRRefreshEventPublisher, + type PRRefreshOutcomeObserver +} from './pr-refresh-event-publisher' +import { PRRefreshPacing } from './pr-refresh-pacing' +import { PRRefreshQueue } from './pr-refresh-queue' +import { PRRefreshQueueDrainer } from './pr-refresh-queue-drainer' +import { prRefreshRateLimitPausedUntil } from './pr-refresh-rate-limit-gate' +import { PRRefreshRetryState } from './pr-refresh-retry-state' +import { PRRefreshVisibility } from './pr-refresh-visibility' -type QueueEntry = { - key: string - candidate: GitHubPRRefreshCandidate - aliases: Map - reason: GitHubPRRefreshReason - priority: number - dueAt: number - queuedAt: number - bypassBackgroundBudget?: boolean - activeDelayNotified?: boolean - windowId?: number -} - -type PRRefreshOutcomeObserver = ( - candidate: GitHubPRRefreshCandidate, - outcome: PRRefreshOutcome -) => void - -type PRBranchLookupCandidate = Pick< - GitHubPRRefreshCandidate, - 'localGitOptions' | 'linkedPRNumber' | 'fallbackPRNumber' | 'fallbackPRSource' | 'currentHeadOid' -> - -function shouldAcceptMergedFallbackPR(candidate: PRBranchLookupCandidate): boolean { - return ( - candidate.linkedPRNumber == null && - candidate.fallbackPRNumber != null && - candidate.fallbackPRSource != null - ) -} - -function hostedReviewOptionArgs( - candidate: PRBranchLookupCandidate -): [] | [GitHubPRBranchLookupOptions] { - const options: GitHubPRBranchLookupOptions = {} - if (candidate.localGitOptions?.wslDistro) { - options.localGitExecOptions = { wslDistro: candidate.localGitOptions.wslDistro } - } - if (shouldAcceptMergedFallbackPR(candidate)) { - options.acceptMergedFallbackPR = true - } - if (typeof candidate.currentHeadOid === 'string' && candidate.currentHeadOid.trim().length > 0) { - options.currentHeadOid = candidate.currentHeadOid.trim() - } - return Object.keys(options).length > 0 ? [options] : [] -} - -const MIN_BACKGROUND_REFRESH_AGE_MS = 60_000 -const MERGEABILITY_PENDING_REFRESH_MS = 10_000 -const MANUAL_MERGEABILITY_PENDING_REFRESH_MS = 2_500 -const BACKGROUND_BUDGET_WINDOW_MS = 5 * 60_000 -const MIN_BACKGROUND_SPACING_MS = 10_000 -const BACKGROUND_BUDGET_MAX = 20 -const POST_PUSH_DELAY_MS = 2_500 -const DIAGNOSTIC_BREADCRUMB_MIN_INTERVAL_MS = 30_000 -const ACTIVE_BURST_WINDOW_MS = 30_000 -const ACTIVE_BURST_MAX = 3 - -let sequence = 0 -let queueOrder = 0 -let draining = false -let drainTimer: ReturnType | null = null -const queue = new Map() -const backgroundStarts: number[] = [] -const activeStartsByScope = new Map() -const errorBackoff = new Map() -// Per-key manual-retry cooldown (secondary rate-limit Retry-After); refreshPRNow refuses manual retry until this time so a stale renderer can't bypass it. -const manualRetryGates = new Map() -let lastBackgroundStartAt = 0 - -/** - * Track (or clear) the manual-retry cooldown for a key from a broadcast outcome. - * Only a rate-limit outcome carrying `retryDisabledUntil` sets a gate; any other settled outcome clears it. - */ -function noteManualRetryGate(key: string, outcome: PRRefreshOutcome): void { - if (outcome.kind === 'upstream-error' && outcome.retryDisabledUntil !== undefined) { - manualRetryGates.set(key, outcome.retryDisabledUntil) - } else { - manualRetryGates.delete(key) - } -} - -/** Reset all per-key retry state (backoff + manual cooldown) when a key resets. */ -function resetKeyRetryState(key: string): void { - errorBackoff.delete(key) - manualRetryGates.delete(key) -} -const visibleByWindow = new Map }>() -let outcomeObserver: PRRefreshOutcomeObserver | null = null -const diagnosticsCounters = { - enqueued: 0, - coalesced: 0, - skipped: 0, - backgroundPauses: 0 -} +const retry = new PRRefreshRetryState() +const queue = new PRRefreshQueue((key) => retry.reset(key)) +const pacing = new PRRefreshPacing() +const visibility = new PRRefreshVisibility() +const events = new PRRefreshEventPublisher() +const drainer = new PRRefreshQueueDrainer(queue, pacing, visibility, retry, events) export function setPRRefreshOutcomeObserver(observer: PRRefreshOutcomeObserver | null): void { - outcomeObserver = observer + events.setOutcomeObserver(observer) } function removeInvisibleVisibleRefreshes(): void { - for (const [key, entry] of queue) { - if (entry.reason === 'visible' && !isVisibleKey(key)) { - queue.delete(key) - resetKeyRetryState(key) - broadcast({ - aliases: Array.from(entry.aliases.values()), - reason: 'visible', - status: 'skipped', - skippedReason: 'fresh' - }) - } - } -} - -function recordPRRefreshQueueDiagnostic( - event: 'enqueued' | 'coalesced' | 'skipped' | 'background-pause', - reason: GitHubPRRefreshReason, - skippedReason?: GitHubPRRefreshSkippedReason -): void { - recordCoalescedCrashBreadcrumb({ - name: 'pr_refresh_queue', - coalesceKey: `pr-refresh-queue:${event}:${reason}:${skippedReason ?? ''}`, - minIntervalMs: DIAGNOSTIC_BREADCRUMB_MIN_INTERVAL_MS, - data: { - event, - reason, - ...(skippedReason ? { skippedReason } : {}), - enqueued: diagnosticsCounters.enqueued, - coalesced: diagnosticsCounters.coalesced, - skipped: diagnosticsCounters.skipped, - backgroundPauses: diagnosticsCounters.backgroundPauses - } - }) -} - -function clearActiveBurstWindow(windowId: number): void { - const windowPrefix = `${windowId}::` - for (const scope of Array.from(activeStartsByScope.keys())) { - if (scope.startsWith(windowPrefix)) { - activeStartsByScope.delete(scope) - } + const removed = queue.removeInvisibleVisibleEntries((key) => visibility.has(key)) + for (const entry of removed) { + events.broadcast({ + aliases: Array.from(entry.aliases.values()), + reason: 'visible', + status: 'skipped', + skippedReason: 'fresh' + }) } } export function clearVisiblePRRefreshWindow(windowId: number): void { - const hadVisibleRefreshes = visibleByWindow.delete(windowId) - clearActiveBurstWindow(windowId) + const hadVisibleRefreshes = visibility.clearWindow(windowId) + pacing.clearActiveBurstWindow(windowId) if (hadVisibleRefreshes) { - // Why: visible follow-ups are owned by the reporting renderer; if its WebContents is destroyed, no later visibility report arrives. removeInvisibleVisibleRefreshes() } } -/** - * Drop a removed worktree's aliases from every queue entry. - * - * Why: aliases are otherwise only pruned when a candidate is re-enqueued as invalid, so churning worktrees grow these maps unbounded (OOM creep). - */ export function pruneWorktreePRRefreshAliases(worktreeId: string): void { - for (const [key, entry] of queue) { - let removed = false - for (const [cacheKey, alias] of entry.aliases) { - if (alias.worktreeId === worktreeId) { - entry.aliases.delete(cacheKey) - removed = true - } - } - if (!removed) { - continue - } - // No aliases left means no worktree still cares about this refresh. - if (entry.aliases.size === 0) { - queue.delete(key) - resetKeyRetryState(key) - continue - } - // Keep the entry alive but replace its representative so it isn't a dangling reference to the removed worktree. - if (entry.candidate.worktreeId === worktreeId) { - const replacementAlias = entry.aliases.values().next().value - if (replacementAlias) { - entry.candidate = { - ...entry.candidate, - cacheKey: replacementAlias.cacheKey, - branch: replacementAlias.branch, - worktreeId: replacementAlias.worktreeId, - // Why: the probe now represents the replacement worktree, so use its head — else the removed worktree's link never clears. - currentHeadOid: replacementAlias.currentHeadOid ?? null - } - } - } - } -} - -function nextSequence(): number { - sequence += 1 - return sequence -} - -function nextQueueOrder(): number { - queueOrder += 1 - return queueOrder -} - -function broadcast(event: Omit, sequenceOverride?: number): void { - const payload = { ...event, sequence: sequenceOverride ?? nextSequence() } as GitHubPRRefreshEvent - sendToTrustedUIRenderer('gh:prRefreshEvent', payload) -} - -function refreshKey(candidate: GitHubPRRefreshCandidate): string { - const connectionScope = candidate.connectionId ?? 'local' - const runtimeScope = candidate.connectionId - ? 'remote' - : `runtime:${candidate.localGitOptions?.wslDistro ? `wsl:${candidate.localGitOptions.wslDistro}` : 'host'}` - if (typeof candidate.linkedPRNumber === 'number') { - return `${connectionScope}::${runtimeScope}::${candidate.repoPath}::pr::${candidate.linkedPRNumber}` - } - return `${connectionScope}::${runtimeScope}::${candidate.repoPath}::branch::${candidate.branch}` -} - -function isVisibleKey(key: string): boolean { - const liveWindowIds = new Set( - webContents - .getAllWebContents() - .filter((wc) => !wc.isDestroyed()) - .map((wc) => wc.id) - ) - for (const windowId of Array.from(visibleByWindow.keys())) { - if (!liveWindowIds.has(windowId)) { - visibleByWindow.delete(windowId) - } - } - for (const visible of visibleByWindow.values()) { - if (visible.keys.has(key)) { - return true - } - } - return false -} - -function isManual(reason: GitHubPRRefreshReason): boolean { - return reason === 'manual' -} - -function bypassesFreshnessDelay(reason: GitHubPRRefreshReason): boolean { - return reason === 'manual' || reason === 'active' || reason === 'post-push' -} - -function isBackground(reason: GitHubPRRefreshReason): boolean { - return reason !== 'manual' -} - -function isBudgetedBackground(reason: GitHubPRRefreshReason): boolean { - return reason === 'visible' || reason === 'swr' -} - -function isBudgetedQueueEntry(entry: QueueEntry): boolean { - return isBudgetedBackground(entry.reason) && entry.bypassBackgroundBudget !== true -} - -function validateCandidate( - candidate: GitHubPRRefreshCandidate -): GitHubPRRefreshSkippedReason | null { - if (candidate.repoKind !== 'git') { - return 'not-git' - } - if (candidate.isBare) { - return 'bare' - } - if (candidate.isArchived) { - return 'archived' - } - if (candidate.connectionId && candidate.connectionState === 'disconnected') { - return 'disconnected' - } - if (!candidate.branch && typeof candidate.linkedPRNumber !== 'number') { - return 'fresh' - } - return null -} - -function shouldSkipFresh( - candidate: GitHubPRRefreshCandidate, - reason: GitHubPRRefreshReason -): boolean { - if (bypassesFreshnessDelay(reason) || candidate.cachedFetchedAt == null) { - return false - } - return Date.now() - candidate.cachedFetchedAt < refreshIntervalForCandidate(candidate) -} - -function shouldBroadcastQueued(reason: GitHubPRRefreshReason, dueAt: number): boolean { - if (isBudgetedBackground(reason)) { - return false - } - const delay = dueAt - Date.now() - if (delay <= 0) { - return false - } - return delay <= 5_000 -} - -function freshRetryAt(candidate: GitHubPRRefreshCandidate): number | null { - return candidate.cachedFetchedAt == null - ? null - : candidate.cachedFetchedAt + refreshIntervalForCandidate(candidate) -} - -function aliasFromCandidate(candidate: GitHubPRRefreshCandidate): GitHubPRRefreshAlias { - return { - cacheKey: candidate.cacheKey, - repoId: candidate.repoId, - repoPath: candidate.repoPath, - branch: candidate.branch, - worktreeId: candidate.worktreeId, - connectionId: candidate.connectionId ?? null, - currentHeadOid: candidate.currentHeadOid ?? null, - linkedPRNumber: candidate.linkedPRNumber ?? null, - fallbackPRNumber: - candidate.linkedPRNumber == null ? (candidate.fallbackPRNumber ?? null) : null, - fallbackPRSource: candidate.linkedPRNumber == null ? (candidate.fallbackPRSource ?? null) : null - } -} - -function visibleCandidateAfterOutcome( - candidate: GitHubPRRefreshCandidate, - outcome: PRRefreshOutcome -): GitHubPRRefreshCandidate { - if (outcome.kind === 'upstream-error') { - return candidate - } - return { - ...candidate, - cachedFetchedAt: outcome.fetchedAt, - cachedHasPR: outcome.kind === 'found', - cachedPRState: outcome.kind === 'found' ? outcome.pr.state : null, - cachedChecksStatus: outcome.kind === 'found' ? outcome.pr.checksStatus : null, - cachedMergeable: outcome.kind === 'found' ? outcome.pr.mergeable : null, - cachedMergeStateStatus: outcome.kind === 'found' ? (outcome.pr.mergeStateStatus ?? null) : null - } -} - -function setVisibleFollowUp(entry: QueueEntry): void { - const existing = queue.get(entry.key) - if (!existing) { - queue.set(entry.key, entry) - return - } - - for (const alias of entry.aliases.values()) { - 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. - if ( - bypassesFreshnessDelay(existing.reason) || - existing.priority > entry.priority || - existing.dueAt <= entry.dueAt - ) { - return - } - - queue.set(entry.key, { - ...entry, - aliases: existing.aliases - }) -} - -function removeQueuedAliasForInvalidCandidate(key: string, alias: GitHubPRRefreshAlias): void { - const existing = queue.get(key) - if (!existing) { - return - } - - existing.aliases.delete(alias.cacheKey) - const replacementAlias = existing.aliases.values().next().value - if (!replacementAlias) { - queue.delete(key) - resetKeyRetryState(key) - return - } - - if (existing.candidate.cacheKey === alias.cacheKey) { - existing.candidate = { - ...existing.candidate, - cacheKey: replacementAlias.cacheKey, - branch: replacementAlias.branch, - worktreeId: replacementAlias.worktreeId, - // Why: the probe now represents the replacement worktree, so use its head — else the survivor's link never clears. - currentHeadOid: replacementAlias.currentHeadOid ?? null, - isArchived: false, - isBare: false - } - } -} - -/** - * Advances the visible-key error backoff and returns the earliest retry time. - * Why: only visible keys auto-retry, so callers must gate on `isVisibleKey` first. - */ -function nextVisibleErrorRetryAt(key: string): number { - const failures = (errorBackoff.get(key)?.failures ?? 0) + 1 - const retryAt = Date.now() + lookupBackoffDelayMs(failures) - errorBackoff.set(key, { failures, retryAt }) - return retryAt -} - -/** - * Stamps the auto-retry time onto an error outcome before broadcast. - * Why: sets only `nextAutoRetryAt`, never `retryDisabledUntil` — that disables *manual* Retry and is reserved for a real rate-limit gate. - */ -function withErrorSchedule(outcome: PRRefreshOutcome, retryAt: number): PRRefreshOutcome { - if (outcome.kind !== 'upstream-error') { - return outcome - } - // Why: honor a real Retry-After cooldown the client stamped (`retryDisabledUntil`) so we don't auto-retry into an active secondary rate limit. - const cooldownUntil = outcome.retryDisabledUntil - return { - ...outcome, - nextAutoRetryAt: cooldownUntil !== undefined ? Math.max(retryAt, cooldownUntil) : retryAt - } -} - -function scheduleVisibleFollowUp( - key: string, - candidate: GitHubPRRefreshCandidate, - outcome: PRRefreshOutcome, - priority: number, - aliases: GitHubPRRefreshAlias[], - windowId?: number, - options?: { pendingMergeabilityDelayMs?: number; plannedRetryAt?: number } -): void { - if (!isVisibleKey(key)) { - // Why: manual/active refreshes can remove the queued visible retry after its owner window is gone, orphaning the backoff. - resetKeyRetryState(key) - return - } - 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) - setVisibleFollowUp({ - key, - candidate, - aliases: new Map(aliases.map((alias) => [alias.cacheKey, alias])), - reason: 'visible', - priority, - dueAt: retryAt, - queuedAt: nextQueueOrder(), - windowId - }) - // 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 - } - resetKeyRetryState(key) - const followUpCandidate = visibleCandidateAfterOutcome(candidate, outcome) - const regularDueAt = freshRetryAt(followUpCandidate) ?? Date.now() - const pendingMergeabilityDueAt = - options?.pendingMergeabilityDelayMs !== undefined && isMergeabilityPendingOutcome(outcome) - ? outcome.fetchedAt + options.pendingMergeabilityDelayMs - : null - const dueAt = - pendingMergeabilityDueAt === null - ? 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. - setVisibleFollowUp({ - key, - candidate: followUpCandidate, - aliases: new Map(aliases.map((alias) => [alias.cacheKey, alias])), - reason: 'visible', - priority, - dueAt, - queuedAt: nextQueueOrder(), - // Why: this manual one-shot fixes GitHub's transient UNKNOWN state; visible spacing would delay it past the prompt retry window. - bypassBackgroundBudget: pendingMergeabilityDueAt !== null, - windowId - }) - scheduleDrain(Math.max(0, dueAt - Date.now())) -} - -function refreshIntervalForCandidate(candidate: GitHubPRRefreshCandidate): number { - if (candidate.cachedPRState === 'closed' || candidate.cachedPRState === 'merged') { - return 30 * 60_000 - } - if (candidate.cachedHasPR === false) { - return NO_REVIEW_REFRESH_INTERVAL_MS - } - if ( - candidate.cachedHasPR === true && - candidate.cachedPRState === 'open' && - candidate.cachedMergeable === 'UNKNOWN' && - !hasResolvedMergeStateStatus(candidate.cachedMergeStateStatus) - ) { - // Why: GitHub returns transient UNKNOWN mergeability while computing the test merge; visible merge buttons need a prompt follow-up. - return MERGEABILITY_PENDING_REFRESH_MS - } - if (candidate.cachedChecksStatus === 'success') { - return 10 * 60_000 - } - if (candidate.cachedChecksStatus === 'failure') { - return 3 * 60_000 - } - if (candidate.cachedChecksStatus === 'pending') { - return 90_000 - } - return MIN_BACKGROUND_REFRESH_AGE_MS -} - -function hasResolvedMergeStateStatus(status: string | null | undefined): boolean { - return status === 'CLEAN' || status === 'BEHIND' || status === 'BLOCKED' -} - -function isMergeabilityPendingOutcome(outcome: PRRefreshOutcome): boolean { - return ( - outcome.kind === 'found' && - outcome.pr.state === 'open' && - outcome.pr.mergeable === 'UNKNOWN' && - !hasResolvedMergeStateStatus(outcome.pr.mergeStateStatus) - ) -} - -function backgroundRefreshBuckets(): ('core' | 'graphql')[] { - // Why: branch refreshes prefer REST but can fall back to `gh pr list`, so guard both buckets until the client exposes a per-lookup cost plan. - return ['core', 'graphql'] -} - -function noteBackgroundStart(): void { - const now = Date.now() - lastBackgroundStartAt = now - backgroundStarts.push(now) - while (backgroundStarts.length > 0 && now - backgroundStarts[0] > BACKGROUND_BUDGET_WINDOW_MS) { - backgroundStarts.shift() - } -} - -function nextBudgetDelay(): number { - const now = Date.now() - while (backgroundStarts.length > 0 && now - backgroundStarts[0] > BACKGROUND_BUDGET_WINDOW_MS) { - backgroundStarts.shift() - } - const spacingDelay = - lastBackgroundStartAt > 0 - ? Math.max(0, MIN_BACKGROUND_SPACING_MS - (now - lastBackgroundStartAt)) - : 0 - const windowDelay = - backgroundStarts.length < BACKGROUND_BUDGET_MAX - ? 0 - : Math.max(1_000, BACKGROUND_BUDGET_WINDOW_MS - (now - backgroundStarts[0])) - return Math.max(spacingDelay, windowDelay) -} - -function activeBurstScope(entry: QueueEntry): string { - const runtimeScope = entry.candidate.connectionId - ? `ssh:${entry.candidate.connectionId}` - : `local:${entry.candidate.localGitOptions?.wslDistro ?? 'host'}` - return `${entry.windowId ?? 'global'}::${runtimeScope}` -} - -function pruneActiveStarts(scope: string, now: number): number[] { - const activeStarts = activeStartsByScope.get(scope) ?? [] - while (activeStarts.length > 0 && now - activeStarts[0] >= ACTIVE_BURST_WINDOW_MS) { - activeStarts.shift() - } - if (activeStarts.length === 0) { - activeStartsByScope.delete(scope) - } else { - activeStartsByScope.set(scope, activeStarts) - } - return activeStarts -} - -function nextActiveBurstDelay(entry: QueueEntry): number { - const now = Date.now() - const activeStarts = pruneActiveStarts(activeBurstScope(entry), now) - if (activeStarts.length < ACTIVE_BURST_MAX) { - return 0 - } - return Math.max(1, ACTIVE_BURST_WINDOW_MS - (now - activeStarts[0])) -} - -function noteActiveStart(entry: QueueEntry): void { - const now = Date.now() - const scope = activeBurstScope(entry) - const activeStarts = pruneActiveStarts(scope, now) - activeStarts.push(now) - activeStartsByScope.set(scope, activeStarts) -} - -function activeOrder(a: QueueEntry, b: QueueEntry): number { - if (a.reason !== 'active' || b.reason !== 'active') { - return 0 - } - if (activeBurstScope(a) !== activeBurstScope(b)) { - return 0 - } - // Why: refresh the worktree the user lands on before stale transient selections in the same window/runtime scope. - return b.queuedAt - a.queuedAt -} - -function entryDelay(entry: QueueEntry): number { - const activeBurstDelay = entry.reason === 'active' ? nextActiveBurstDelay(entry) : 0 - if (activeBurstDelay > 0) { - return activeBurstDelay - } - return isBudgetedQueueEntry(entry) ? nextBudgetDelay() : 0 -} - -function isActiveBurstDelayed(entry: QueueEntry): boolean { - return entry.reason === 'active' && nextActiveBurstDelay(entry) > 0 -} - -function nextQueuedWakeDelay(excludedKey: string): number | null { - const now = Date.now() - let nextDelay = Number.POSITIVE_INFINITY - for (const entry of queue.values()) { - if (entry.key === excludedKey) { - continue - } - const delay = entry.dueAt > now ? entry.dueAt - now : entryDelay(entry) - nextDelay = Math.min(nextDelay, delay) - } - return Number.isFinite(nextDelay) ? Math.max(0, nextDelay) : null -} - -function scheduleDrain(delay = 0): void { - if (drainTimer) { - clearTimeout(drainTimer) - } - drainTimer = setTimeout(() => { - drainTimer = null - void drainQueue() - }, delay) -} - -function queuedEntriesByPriority(): QueueEntry[] { - const now = Date.now() - return Array.from(queue.values()).sort((a, b) => { - const aReady = a.dueAt <= now - const bReady = b.dueAt <= now - if (aReady && bReady) { - return b.priority - a.priority || activeOrder(a, b) || a.dueAt - b.dueAt - } - if (aReady !== bReady) { - return aReady ? -1 : 1 - } - return a.dueAt - b.dueAt || b.priority - a.priority - }) -} - -async function drainQueue(): Promise { - if (draining) { - return - } - draining = true - try { - while (queue.size > 0) { - let next = queuedEntriesByPriority()[0] - const waitMs = next.dueAt - Date.now() - if (waitMs > 0) { - scheduleDrain(waitMs) - return - } - - let delay = entryDelay(next) - if (delay > 0) { - const runnable = queuedEntriesByPriority().find( - (entry) => entry.dueAt <= Date.now() && entryDelay(entry) === 0 - ) - if (runnable && runnable.key !== next.key) { - next = runnable - delay = 0 - } else { - if (isActiveBurstDelayed(next) && !next.activeDelayNotified) { - next.activeDelayNotified = true - broadcast({ - aliases: Array.from(next.aliases.values()), - reason: next.reason, - status: 'queued' - }) - } - if (isBudgetedQueueEntry(next) && nextBudgetDelay() > 0) { - diagnosticsCounters.backgroundPauses += 1 - recordPRRefreshQueueDiagnostic('background-pause', next.reason) - } - scheduleDrain(Math.min(delay, nextQueuedWakeDelay(next.key) ?? delay)) - return - } - } - - queue.delete(next.key) - const aliases = Array.from(next.aliases.values()) - const skippedReason = validateCandidate(next.candidate) - if (skippedReason) { - diagnosticsCounters.skipped += 1 - recordPRRefreshQueueDiagnostic('skipped', next.reason, skippedReason) - broadcast({ aliases, reason: next.reason, status: 'skipped', skippedReason }) - continue - } - if (next.reason === 'visible' && !isVisibleKey(next.key)) { - resetKeyRetryState(next.key) - broadcast({ aliases, reason: next.reason, status: 'skipped', skippedReason: 'fresh' }) - continue - } - const requestSequence = nextSequence() - const requestStartedAt = Date.now() - broadcast( - { aliases, reason: next.reason, status: 'in-flight', requestStartedAt }, - requestSequence - ) - - if (isBackground(next.reason)) { - const executionOptions = ghRepoExecOptions( - githubRepoContext( - next.candidate.repoPath, - next.candidate.connectionId, - next.candidate.localGitOptions - ) - ) - const repository = await getOriginGitHubApiRepository( - next.candidate.repoPath, - next.candidate.connectionId, - executionOptions - ) - // Why: only native github.com uses the singleton snapshot; scoped breakers protect GHES and WSL. - if (spendsSharedGitHubComQuota(repository, executionOptions)) { - // Why: the probe only warms the cache, so failures must fail open (#7553). - await getRateLimit() - } - const buckets = backgroundRefreshBuckets() - const blockedGuard = buckets - .map((bucket) => repositoryRateLimitGuard(repository, bucket, executionOptions)) - .find((guard) => guard.blocked) - if (blockedGuard?.blocked) { - const retryAt = blockedGuard.resetAt * 1000 - queue.set(next.key, { ...next, dueAt: retryAt }) - broadcast({ - aliases, - reason: next.reason, - status: 'paused', - pausedUntil: retryAt, - skippedReason: 'rate-limit' - }) - scheduleDrain(Math.max(1_000, retryAt - Date.now())) - continue - } - if (isBudgetedQueueEntry(next)) { - noteBackgroundStart() - } - if (next.reason === 'active') { - // Why: tab/worktree churn can enqueue many distinct active refreshes that each probe local Git. - noteActiveStart(next) - } - // Why (#11532): the lookup itself now debits the snapshot, so every - // caller is accounted for; debiting here too would double-count. - } - - const outcome = await getPRForBranchOutcome( - next.candidate.repoPath, - next.candidate.branch, - next.candidate.linkedPRNumber ?? null, - next.candidate.connectionId ?? null, - next.candidate.linkedPRNumber == null ? (next.candidate.fallbackPRNumber ?? null) : null, - ...hostedReviewOptionArgs(next.candidate) - ) - // Why: compute the retry schedule before broadcasting so the outcome and its follow-up share one timing; only visible keys auto-retry. - let plannedRetryAt: number | undefined - let broadcastOutcome = outcome - if (outcome.kind === 'upstream-error' && isVisibleKey(next.key)) { - plannedRetryAt = nextVisibleErrorRetryAt(next.key) - broadcastOutcome = withErrorSchedule(outcome, plannedRetryAt) - } - outcomeObserver?.(next.candidate, outcome) - noteManualRetryGate(next.key, broadcastOutcome) - broadcast( - { aliases, reason: next.reason, outcome: broadcastOutcome, requestStartedAt }, - requestSequence - ) - scheduleVisibleFollowUp( - next.key, - next.candidate, - outcome, - next.priority, - aliases, - next.windowId, - { - plannedRetryAt - } - ) - } - } finally { - draining = false - } + queue.pruneWorktreeAliases(worktreeId) } export function enqueuePRRefresh( @@ -824,66 +68,18 @@ export function enqueuePRRefresh( const key = refreshKey(candidate) const skippedReason = validateCandidate(candidate) if (skippedReason) { - removeQueuedAliasForInvalidCandidate(key, alias) - diagnosticsCounters.skipped += 1 - recordPRRefreshQueueDiagnostic('skipped', reason, skippedReason) - broadcast({ - aliases: [alias], - reason, - status: 'skipped', - skippedReason - }) + queue.removeInvalidAlias(key, alias) + events.record('skipped', reason, skippedReason) + events.broadcast({ aliases: [alias], reason, status: 'skipped', skippedReason }) return } - const existing = queue.get(key) - const freshDueAt = shouldSkipFresh(candidate, reason) ? freshRetryAt(candidate) : null - const dueAt = freshDueAt ?? Date.now() + (reason === 'post-push' ? POST_PUSH_DELAY_MS : 0) - if (existing) { - existing.aliases.set(alias.cacheKey, alias) - diagnosticsCounters.coalesced += 1 - recordPRRefreshQueueDiagnostic('coalesced', reason) - const shouldPromoteExisting = - priority > existing.priority || - isManual(reason) || - (reason === 'active' && existing.reason === 'active') || - (priority >= existing.priority && dueAt < existing.dueAt && bypassesFreshnessDelay(reason)) - if (shouldPromoteExisting) { - existing.priority = priority - existing.reason = reason - existing.dueAt = Math.min(existing.dueAt, dueAt) - existing.queuedAt = nextQueueOrder() - existing.activeDelayNotified = false - existing.candidate = candidate - existing.windowId = windowId ?? existing.windowId - } else if (existing.candidate.worktreeId === candidate.worktreeId) { - // Why: the representative drives the probe head; refresh probe inputs when its worktree moved head/branch, else a merged linked PR lingers after a switch. - existing.candidate = { - ...existing.candidate, - cacheKey: candidate.cacheKey, - branch: candidate.branch, - currentHeadOid: candidate.currentHeadOid ?? null - } - } - } else { - diagnosticsCounters.enqueued += 1 - recordPRRefreshQueueDiagnostic('enqueued', reason) - queue.set(key, { - key, - candidate, - aliases: new Map([[alias.cacheKey, alias]]), - reason, - priority, - dueAt, - queuedAt: nextQueueOrder(), - windowId - }) + const enqueued = queue.enqueue(candidate, reason, priority, windowId) + events.record(enqueued.coalesced ? 'coalesced' : 'enqueued', reason) + if (shouldBroadcastQueued(reason, enqueued.dueAt)) { + events.broadcast({ aliases: [enqueued.alias], reason, status: 'queued' }) } - // Why: visible/SWR are background maintenance behind the budget queue; only user/action-driven queueing should surface in UI. - if (shouldBroadcastQueued(reason, dueAt)) { - broadcast({ aliases: [alias], reason, status: 'queued' }) - } - scheduleDrain() + drainer.schedule() } export function reportVisiblePRRefreshCandidates( @@ -891,11 +87,9 @@ export function reportVisiblePRRefreshCandidates( generation: number, windowId: number ): void { - const existingVisible = visibleByWindow.get(windowId) - if (existingVisible && generation < existingVisible.generation) { + if (!visibility.report(candidates, generation, windowId)) { return } - visibleByWindow.set(windowId, { generation, keys: new Set(candidates.map(refreshKey)) }) removeInvisibleVisibleRefreshes() for (const candidate of candidates) { enqueuePRRefresh(candidate, 'visible', 40, windowId) @@ -903,11 +97,11 @@ export function reportVisiblePRRefreshCandidates( } export function _getVisiblePRRefreshWindowCountForTests(): number { - return visibleByWindow.size + return visibility.windowCount } export function _getPRRefreshErrorBackoffCountForTests(): number { - return errorBackoff.size + return retry.errorBackoffCount } export function _getPRRefreshQueueSizeForTests(): number { @@ -915,7 +109,7 @@ export function _getPRRefreshQueueSizeForTests(): number { } export function _getPRRefreshAliasCountForTests(key: string): number { - return queue.get(key)?.aliases.size ?? 0 + return queue.aliasCount(key) } export async function refreshPRNow(candidate: GitHubPRRefreshCandidate): Promise { @@ -927,68 +121,54 @@ export async function refreshPRNow(candidate: GitHubPRRefreshCandidate): Promise const aliases = Array.from(aliasMap.values()) const skippedReason = validateCandidate(candidate) if (skippedReason) { - removeQueuedAliasForInvalidCandidate(key, alias) + queue.removeInvalidAlias(key, alias) const outcome: PRRefreshOutcome = { kind: 'upstream-error', errorType: 'unknown', message: `Cannot refresh PR for this worktree: ${skippedReason}`, fetchedAt: Date.now() } - broadcast({ aliases: [alias], reason: 'manual', status: 'skipped', skippedReason }) + events.broadcast({ aliases: [alias], reason: 'manual', status: 'skipped', skippedReason }) return outcome } - // Why: enforce the rate-limit gate so a stale renderer can't bypass it; refuse without spending quota until the later of the two cooldowns. - const manualExecutionOptions = ghRepoExecOptions( - githubRepoContext(candidate.repoPath, candidate.connectionId, candidate.localGitOptions) - ) - const manualRepository = await getOriginGitHubApiRepository( - candidate.repoPath, - candidate.connectionId, - manualExecutionOptions - ) - const manualBlockedGuard = backgroundRefreshBuckets() - .map((bucket) => repositoryRateLimitGuard(manualRepository, bucket, manualExecutionOptions)) - .find((guard) => guard.blocked) - const secondaryGateUntil = manualRetryGates.get(key) - const gateUntil = Math.max( - manualBlockedGuard?.blocked ? manualBlockedGuard.resetAt * 1000 : 0, - secondaryGateUntil !== undefined ? secondaryGateUntil : 0 - ) + const primaryGateUntil = await prRefreshRateLimitPausedUntil(candidate, false) + const gateUntil = Math.max(primaryGateUntil ?? 0, retry.manualGateUntil(key)) 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. queue.set(key, { key, candidate, aliases: aliasMap, reason: 'manual', priority: 40, - dueAt: retryAt, - queuedAt: nextQueueOrder() + dueAt: gateUntil, + queuedAt: queue.nextOrder() }) - broadcast({ + events.broadcast({ aliases, reason: 'manual', status: 'paused', - pausedUntil: retryAt, + pausedUntil: gateUntil, skippedReason: 'rate-limit' }) - scheduleDrain(Math.max(1_000, retryAt - Date.now())) + drainer.schedule(Math.max(1_000, gateUntil - Date.now())) return { kind: 'upstream-error', errorType: 'rate_limited', message: 'GitHub is temporarily limiting requests. Try again after the limit resets.', fetchedAt: Date.now(), - nextAutoRetryAt: retryAt, - retryDisabledUntil: retryAt + nextAutoRetryAt: gateUntil, + retryDisabledUntil: gateUntil } } queue.delete(key) - const requestSequence = nextSequence() + const requestSequence = events.nextSequence() const requestStartedAt = Date.now() - broadcast({ aliases, reason: 'manual', status: 'in-flight', requestStartedAt }, requestSequence) + events.broadcast( + { aliases, reason: 'manual', status: 'in-flight', requestStartedAt }, + requestSequence + ) const outcome = await getPRForBranchOutcome( candidate.repoPath, candidate.branch, @@ -999,19 +179,18 @@ export async function refreshPRNow(candidate: GitHubPRRefreshCandidate): Promise ) let plannedRetryAt: number | undefined let broadcastOutcome = outcome - if (outcome.kind === 'upstream-error' && isVisibleKey(key)) { - plannedRetryAt = nextVisibleErrorRetryAt(key) - broadcastOutcome = withErrorSchedule(outcome, plannedRetryAt) + if (outcome.kind === 'upstream-error' && visibility.has(key)) { + plannedRetryAt = retry.nextVisibleErrorRetryAt(key) + broadcastOutcome = retry.withErrorSchedule(outcome, plannedRetryAt) } - outcomeObserver?.(candidate, outcome) - noteManualRetryGate(key, broadcastOutcome) - broadcast( + events.observe(candidate, outcome) + retry.noteManualGate(key, broadcastOutcome) + events.broadcast( { aliases, reason: 'manual', outcome: broadcastOutcome, requestStartedAt }, requestSequence ) - scheduleVisibleFollowUp(key, candidate, outcome, 40, aliases, undefined, { + drainer.scheduleVisibleFollowUp(key, candidate, outcome, 40, aliases, undefined, { plannedRetryAt, - // Why: GitHub reports UNKNOWN right after `gh pr reopen`; one prompt visible retry replaces the transient label. pendingMergeabilityDelayMs: MANUAL_MERGEABILITY_PENDING_REFRESH_MS }) return broadcastOutcome diff --git a/src/main/github/pr-refresh-event-publisher.ts b/src/main/github/pr-refresh-event-publisher.ts new file mode 100644 index 00000000000..b028729f1f5 --- /dev/null +++ b/src/main/github/pr-refresh-event-publisher.ts @@ -0,0 +1,69 @@ +import type { + GitHubPRRefreshCandidate, + GitHubPRRefreshEvent, + GitHubPRRefreshReason, + GitHubPRRefreshSkippedReason, + PRRefreshOutcome +} from '../../shared/github/pull-request-refresh-types' +import { recordCoalescedCrashBreadcrumb } from '../crash-reporting/crash-breadcrumb-store' +import { sendToTrustedUIRenderer } from '../ipc/ui' + +export type PRRefreshOutcomeObserver = ( + candidate: GitHubPRRefreshCandidate, + outcome: PRRefreshOutcome +) => void + +const DIAGNOSTIC_BREADCRUMB_MIN_INTERVAL_MS = 30_000 + +export class PRRefreshEventPublisher { + private sequence = 0 + private observer: PRRefreshOutcomeObserver | null = null + private readonly counters = { enqueued: 0, coalesced: 0, skipped: 0, backgroundPauses: 0 } + + setOutcomeObserver(observer: PRRefreshOutcomeObserver | null): void { + this.observer = observer + } + + observe(candidate: GitHubPRRefreshCandidate, outcome: PRRefreshOutcome): void { + this.observer?.(candidate, outcome) + } + + nextSequence(): number { + this.sequence += 1 + return this.sequence + } + + broadcast(event: Omit, sequenceOverride?: number): void { + const payload = { + ...event, + sequence: sequenceOverride ?? this.nextSequence() + } as GitHubPRRefreshEvent + sendToTrustedUIRenderer('gh:prRefreshEvent', payload) + } + + record( + event: 'enqueued' | 'coalesced' | 'skipped' | 'background-pause', + reason: GitHubPRRefreshReason, + skippedReason?: GitHubPRRefreshSkippedReason + ): void { + if (event === 'background-pause') { + this.counters.backgroundPauses += 1 + } else { + this.counters[event] += 1 + } + recordCoalescedCrashBreadcrumb({ + name: 'pr_refresh_queue', + coalesceKey: `pr-refresh-queue:${event}:${reason}:${skippedReason ?? ''}`, + minIntervalMs: DIAGNOSTIC_BREADCRUMB_MIN_INTERVAL_MS, + data: { + event, + reason, + ...(skippedReason ? { skippedReason } : {}), + enqueued: this.counters.enqueued, + coalesced: this.counters.coalesced, + skipped: this.counters.skipped, + backgroundPauses: this.counters.backgroundPauses + } + }) + } +} diff --git a/src/main/github/pr-refresh-pacing.ts b/src/main/github/pr-refresh-pacing.ts new file mode 100644 index 00000000000..7a88bd74a0a --- /dev/null +++ b/src/main/github/pr-refresh-pacing.ts @@ -0,0 +1,114 @@ +import type { PRRefreshQueueEntry } from './pr-refresh-queue' + +const BACKGROUND_BUDGET_WINDOW_MS = 5 * 60_000 +const MIN_BACKGROUND_SPACING_MS = 10_000 +const BACKGROUND_BUDGET_MAX = 20 +const ACTIVE_BURST_WINDOW_MS = 30_000 +const ACTIVE_BURST_MAX = 3 + +export class PRRefreshPacing { + private readonly backgroundStarts: number[] = [] + private readonly activeStartsByScope = new Map() + private lastBackgroundStartAt = 0 + + clearActiveBurstWindow(windowId: number): void { + const windowPrefix = `${windowId}::` + for (const scope of Array.from(this.activeStartsByScope.keys())) { + if (scope.startsWith(windowPrefix)) { + this.activeStartsByScope.delete(scope) + } + } + } + + noteBackgroundStart(): void { + const now = Date.now() + this.lastBackgroundStartAt = now + this.backgroundStarts.push(now) + this.pruneBackgroundStarts(now) + } + + nextBudgetDelay(): number { + const now = Date.now() + this.pruneBackgroundStarts(now) + const spacingDelay = + this.lastBackgroundStartAt > 0 + ? Math.max(0, MIN_BACKGROUND_SPACING_MS - (now - this.lastBackgroundStartAt)) + : 0 + const windowDelay = + this.backgroundStarts.length < BACKGROUND_BUDGET_MAX + ? 0 + : Math.max(1_000, BACKGROUND_BUDGET_WINDOW_MS - (now - this.backgroundStarts[0])) + return Math.max(spacingDelay, windowDelay) + } + + activeOrder(a: PRRefreshQueueEntry, b: PRRefreshQueueEntry): number { + if (a.reason !== 'active' || b.reason !== 'active') { + return 0 + } + if (this.activeBurstScope(a) !== this.activeBurstScope(b)) { + return 0 + } + return b.queuedAt - a.queuedAt + } + + entryDelay(entry: PRRefreshQueueEntry): number { + const activeDelay = entry.reason === 'active' ? this.nextActiveBurstDelay(entry) : 0 + if (activeDelay > 0) { + return activeDelay + } + return entry.bypassBackgroundBudget !== true && + (entry.reason === 'visible' || entry.reason === 'swr') + ? this.nextBudgetDelay() + : 0 + } + + isActiveBurstDelayed(entry: PRRefreshQueueEntry): boolean { + return entry.reason === 'active' && this.nextActiveBurstDelay(entry) > 0 + } + + noteActiveStart(entry: PRRefreshQueueEntry): void { + const now = Date.now() + const scope = this.activeBurstScope(entry) + const activeStarts = this.pruneActiveStarts(scope, now) + activeStarts.push(now) + this.activeStartsByScope.set(scope, activeStarts) + } + + private pruneBackgroundStarts(now: number): void { + while ( + this.backgroundStarts.length > 0 && + now - this.backgroundStarts[0] > BACKGROUND_BUDGET_WINDOW_MS + ) { + this.backgroundStarts.shift() + } + } + + private activeBurstScope(entry: PRRefreshQueueEntry): string { + const runtimeScope = entry.candidate.connectionId + ? `ssh:${entry.candidate.connectionId}` + : `local:${entry.candidate.localGitOptions?.wslDistro ?? 'host'}` + return `${entry.windowId ?? 'global'}::${runtimeScope}` + } + + private pruneActiveStarts(scope: string, now: number): number[] { + const activeStarts = this.activeStartsByScope.get(scope) ?? [] + while (activeStarts.length > 0 && now - activeStarts[0] >= ACTIVE_BURST_WINDOW_MS) { + activeStarts.shift() + } + if (activeStarts.length === 0) { + this.activeStartsByScope.delete(scope) + } else { + this.activeStartsByScope.set(scope, activeStarts) + } + return activeStarts + } + + private nextActiveBurstDelay(entry: PRRefreshQueueEntry): number { + const now = Date.now() + const activeStarts = this.pruneActiveStarts(this.activeBurstScope(entry), now) + if (activeStarts.length < ACTIVE_BURST_MAX) { + return 0 + } + return Math.max(1, ACTIVE_BURST_WINDOW_MS - (now - activeStarts[0])) + } +} diff --git a/src/main/github/pr-refresh-queue-drainer.ts b/src/main/github/pr-refresh-queue-drainer.ts new file mode 100644 index 00000000000..d1e37492e32 --- /dev/null +++ b/src/main/github/pr-refresh-queue-drainer.ts @@ -0,0 +1,242 @@ +import type { + GitHubPRRefreshAlias, + GitHubPRRefreshCandidate, + PRRefreshOutcome +} from '../../shared/github/pull-request-refresh-types' +import { getPRForBranchOutcome } from './client' +import { + freshRetryAt, + hostedReviewOptionArgs, + isBackground, + isMergeabilityPendingOutcome, + validateCandidate, + visibleCandidateAfterOutcome +} from './pr-refresh-candidate-policy' +import type { PRRefreshEventPublisher } from './pr-refresh-event-publisher' +import type { PRRefreshPacing } from './pr-refresh-pacing' +import type { PRRefreshQueue, PRRefreshQueueEntry } from './pr-refresh-queue' +import { prRefreshRateLimitPausedUntil } from './pr-refresh-rate-limit-gate' +import type { PRRefreshRetryState } from './pr-refresh-retry-state' +import type { PRRefreshVisibility } from './pr-refresh-visibility' + +export class PRRefreshQueueDrainer { + private draining = false + private timer: ReturnType | null = null + + constructor( + private readonly queue: PRRefreshQueue, + private readonly pacing: PRRefreshPacing, + private readonly visibility: PRRefreshVisibility, + private readonly retry: PRRefreshRetryState, + private readonly events: PRRefreshEventPublisher + ) {} + + schedule(delay = 0): void { + if (this.timer) { + clearTimeout(this.timer) + } + this.timer = setTimeout(() => { + this.timer = null + void this.drain() + }, delay) + } + + scheduleVisibleFollowUp( + key: string, + candidate: GitHubPRRefreshCandidate, + outcome: PRRefreshOutcome, + priority: number, + aliases: GitHubPRRefreshAlias[], + windowId?: number, + options?: { pendingMergeabilityDelayMs?: number; plannedRetryAt?: number } + ): void { + if (!this.visibility.has(key)) { + this.retry.reset(key) + return + } + if (outcome.kind === 'upstream-error') { + const retryAt = options?.plannedRetryAt ?? this.retry.nextVisibleErrorRetryAt(key) + this.queue.setVisibleFollowUp({ + key, + candidate, + aliases: new Map(aliases.map((alias) => [alias.cacheKey, alias])), + reason: 'visible', + priority, + dueAt: retryAt, + queuedAt: this.queue.nextOrder(), + windowId + }) + this.schedule(retryAt - Date.now()) + return + } + this.retry.reset(key) + const followUpCandidate = visibleCandidateAfterOutcome(candidate, outcome) + const regularDueAt = freshRetryAt(followUpCandidate) ?? Date.now() + const pendingDueAt = + options?.pendingMergeabilityDelayMs !== undefined && isMergeabilityPendingOutcome(outcome) + ? outcome.fetchedAt + options.pendingMergeabilityDelayMs + : null + const dueAt = pendingDueAt === null ? regularDueAt : Math.min(regularDueAt, pendingDueAt) + this.queue.setVisibleFollowUp({ + key, + candidate: followUpCandidate, + aliases: new Map(aliases.map((alias) => [alias.cacheKey, alias])), + reason: 'visible', + priority, + dueAt, + queuedAt: this.queue.nextOrder(), + bypassBackgroundBudget: pendingDueAt !== null, + windowId + }) + this.schedule(Math.max(0, dueAt - Date.now())) + } + + private ordered(): PRRefreshQueueEntry[] { + return this.queue.ordered((a, b) => this.pacing.activeOrder(a, b)) + } + + private nextQueuedWakeDelay(excludedKey: string): number | null { + const now = Date.now() + let nextDelay = Number.POSITIVE_INFINITY + for (const entry of this.queue.values()) { + if (entry.key === excludedKey) { + continue + } + const delay = entry.dueAt > now ? entry.dueAt - now : this.pacing.entryDelay(entry) + nextDelay = Math.min(nextDelay, delay) + } + return Number.isFinite(nextDelay) ? Math.max(0, nextDelay) : null + } + + private async drain(): Promise { + if (this.draining) { + return + } + this.draining = true + try { + while (this.queue.size > 0) { + let next = this.ordered()[0] + const waitMs = next.dueAt - Date.now() + if (waitMs > 0) { + this.schedule(waitMs) + return + } + + let delay = this.pacing.entryDelay(next) + if (delay > 0) { + const runnable = this.ordered().find( + (entry) => entry.dueAt <= Date.now() && this.pacing.entryDelay(entry) === 0 + ) + if (runnable && runnable.key !== next.key) { + next = runnable + delay = 0 + } else { + this.notePacingDelay(next) + this.schedule(Math.min(delay, this.nextQueuedWakeDelay(next.key) ?? delay)) + return + } + } + + this.queue.delete(next.key) + const aliases = Array.from(next.aliases.values()) + const skippedReason = validateCandidate(next.candidate) + if (skippedReason) { + this.events.record('skipped', next.reason, skippedReason) + this.events.broadcast({ aliases, reason: next.reason, status: 'skipped', skippedReason }) + continue + } + if (next.reason === 'visible' && !this.visibility.has(next.key)) { + this.retry.reset(next.key) + this.events.broadcast({ + aliases, + reason: next.reason, + status: 'skipped', + skippedReason: 'fresh' + }) + continue + } + const requestSequence = this.events.nextSequence() + const requestStartedAt = Date.now() + this.events.broadcast( + { aliases, reason: next.reason, status: 'in-flight', requestStartedAt }, + requestSequence + ) + + if (isBackground(next.reason)) { + const pausedUntil = await prRefreshRateLimitPausedUntil(next.candidate, true) + if (pausedUntil !== null) { + this.queue.set(next.key, { ...next, dueAt: pausedUntil }) + this.events.broadcast({ + aliases, + reason: next.reason, + status: 'paused', + pausedUntil, + skippedReason: 'rate-limit' + }) + this.schedule(Math.max(1_000, pausedUntil - Date.now())) + continue + } + if ( + next.bypassBackgroundBudget !== true && + (next.reason === 'visible' || next.reason === 'swr') + ) { + this.pacing.noteBackgroundStart() + } + if (next.reason === 'active') { + this.pacing.noteActiveStart(next) + } + } + + const outcome = await getPRForBranchOutcome( + next.candidate.repoPath, + next.candidate.branch, + next.candidate.linkedPRNumber ?? null, + next.candidate.connectionId ?? null, + next.candidate.linkedPRNumber == null ? (next.candidate.fallbackPRNumber ?? null) : null, + ...hostedReviewOptionArgs(next.candidate) + ) + let plannedRetryAt: number | undefined + let broadcastOutcome = outcome + if (outcome.kind === 'upstream-error' && this.visibility.has(next.key)) { + plannedRetryAt = this.retry.nextVisibleErrorRetryAt(next.key) + broadcastOutcome = this.retry.withErrorSchedule(outcome, plannedRetryAt) + } + this.events.observe(next.candidate, outcome) + this.retry.noteManualGate(next.key, broadcastOutcome) + this.events.broadcast( + { aliases, reason: next.reason, outcome: broadcastOutcome, requestStartedAt }, + requestSequence + ) + this.scheduleVisibleFollowUp( + next.key, + next.candidate, + outcome, + next.priority, + aliases, + next.windowId, + { plannedRetryAt } + ) + } + } finally { + this.draining = false + } + } + + private notePacingDelay(entry: PRRefreshQueueEntry): void { + if (this.pacing.isActiveBurstDelayed(entry) && !entry.activeDelayNotified) { + entry.activeDelayNotified = true + this.events.broadcast({ + aliases: Array.from(entry.aliases.values()), + reason: entry.reason, + status: 'queued' + }) + } + if ( + entry.bypassBackgroundBudget !== true && + (entry.reason === 'visible' || entry.reason === 'swr') && + this.pacing.nextBudgetDelay() > 0 + ) { + this.events.record('background-pause', entry.reason) + } + } +} diff --git a/src/main/github/pr-refresh-queue.ts b/src/main/github/pr-refresh-queue.ts new file mode 100644 index 00000000000..2cc185535fa --- /dev/null +++ b/src/main/github/pr-refresh-queue.ts @@ -0,0 +1,225 @@ +import type { + GitHubPRRefreshAlias, + GitHubPRRefreshCandidate, + GitHubPRRefreshReason +} from '../../shared/github/pull-request-refresh-types' +import { + aliasFromCandidate, + bypassesFreshnessDelay, + freshRetryAt, + POST_PUSH_DELAY_MS, + refreshKey, + shouldSkipFresh +} from './pr-refresh-candidate-policy' + +export type PRRefreshQueueEntry = { + key: string + candidate: GitHubPRRefreshCandidate + aliases: Map + reason: GitHubPRRefreshReason + priority: number + dueAt: number + queuedAt: number + bypassBackgroundBudget?: boolean + activeDelayNotified?: boolean + windowId?: number +} + +export type PRRefreshEnqueue = { + alias: GitHubPRRefreshAlias + key: string + dueAt: number + coalesced: boolean +} + +export class PRRefreshQueue { + private readonly entries = new Map() + private order = 0 + + constructor(private readonly resetRetryState: (key: string) => void) {} + + get size(): number { + return this.entries.size + } + + get(key: string): PRRefreshQueueEntry | undefined { + return this.entries.get(key) + } + + set(key: string, entry: PRRefreshQueueEntry): void { + this.entries.set(key, entry) + } + + delete(key: string): void { + this.entries.delete(key) + } + + values(): IterableIterator { + return this.entries.values() + } + + nextOrder(): number { + this.order += 1 + return this.order + } + + aliasCount(key: string): number { + return this.entries.get(key)?.aliases.size ?? 0 + } + + enqueue( + candidate: GitHubPRRefreshCandidate, + reason: GitHubPRRefreshReason, + priority: number, + windowId?: number + ): PRRefreshEnqueue { + const alias = aliasFromCandidate(candidate) + const key = refreshKey(candidate) + const existing = this.entries.get(key) + const freshDueAt = shouldSkipFresh(candidate, reason) ? freshRetryAt(candidate) : null + const dueAt = freshDueAt ?? Date.now() + (reason === 'post-push' ? POST_PUSH_DELAY_MS : 0) + if (!existing) { + this.entries.set(key, { + key, + candidate, + aliases: new Map([[alias.cacheKey, alias]]), + reason, + priority, + dueAt, + queuedAt: this.nextOrder(), + windowId + }) + return { alias, key, dueAt, coalesced: false } + } + + existing.aliases.set(alias.cacheKey, alias) + const shouldPromote = + priority > existing.priority || + reason === 'manual' || + (reason === 'active' && existing.reason === 'active') || + (priority >= existing.priority && dueAt < existing.dueAt && bypassesFreshnessDelay(reason)) + if (shouldPromote) { + existing.priority = priority + existing.reason = reason + existing.dueAt = Math.min(existing.dueAt, dueAt) + existing.queuedAt = this.nextOrder() + existing.activeDelayNotified = false + existing.candidate = candidate + existing.windowId = windowId ?? existing.windowId + } else if (existing.candidate.worktreeId === candidate.worktreeId) { + existing.candidate = { + ...existing.candidate, + cacheKey: candidate.cacheKey, + branch: candidate.branch, + currentHeadOid: candidate.currentHeadOid ?? null + } + } + return { alias, key, dueAt, coalesced: true } + } + + removeInvalidAlias(key: string, alias: GitHubPRRefreshAlias): void { + const existing = this.entries.get(key) + if (!existing) { + return + } + existing.aliases.delete(alias.cacheKey) + const replacement = existing.aliases.values().next().value + if (!replacement) { + this.entries.delete(key) + this.resetRetryState(key) + return + } + if (existing.candidate.cacheKey === alias.cacheKey) { + existing.candidate = { + ...existing.candidate, + cacheKey: replacement.cacheKey, + branch: replacement.branch, + worktreeId: replacement.worktreeId, + currentHeadOid: replacement.currentHeadOid ?? null, + isArchived: false, + isBare: false + } + } + } + + pruneWorktreeAliases(worktreeId: string): void { + for (const [key, entry] of this.entries) { + let removed = false + for (const [cacheKey, alias] of entry.aliases) { + if (alias.worktreeId === worktreeId) { + entry.aliases.delete(cacheKey) + removed = true + } + } + if (!removed) { + continue + } + if (entry.aliases.size === 0) { + this.entries.delete(key) + this.resetRetryState(key) + continue + } + if (entry.candidate.worktreeId === worktreeId) { + const replacement = entry.aliases.values().next().value + if (replacement) { + entry.candidate = { + ...entry.candidate, + cacheKey: replacement.cacheKey, + branch: replacement.branch, + worktreeId: replacement.worktreeId, + currentHeadOid: replacement.currentHeadOid ?? null + } + } + } + } + } + + removeInvisibleVisibleEntries(isVisible: (key: string) => boolean): PRRefreshQueueEntry[] { + const removed: PRRefreshQueueEntry[] = [] + for (const [key, entry] of this.entries) { + if (entry.reason !== 'visible' || isVisible(key)) { + continue + } + this.entries.delete(key) + this.resetRetryState(key) + removed.push(entry) + } + return removed + } + + setVisibleFollowUp(entry: PRRefreshQueueEntry): void { + const existing = this.entries.get(entry.key) + if (!existing) { + this.entries.set(entry.key, entry) + return + } + for (const alias of entry.aliases.values()) { + existing.aliases.set(alias.cacheKey, alias) + } + if ( + bypassesFreshnessDelay(existing.reason) || + existing.priority > entry.priority || + existing.dueAt <= entry.dueAt + ) { + return + } + this.entries.set(entry.key, { ...entry, aliases: existing.aliases }) + } + + ordered( + activeOrder: (a: PRRefreshQueueEntry, b: PRRefreshQueueEntry) => number + ): PRRefreshQueueEntry[] { + const now = Date.now() + return Array.from(this.entries.values()).sort((a, b) => { + const aReady = a.dueAt <= now + const bReady = b.dueAt <= now + if (aReady && bReady) { + return b.priority - a.priority || activeOrder(a, b) || a.dueAt - b.dueAt + } + if (aReady !== bReady) { + return aReady ? -1 : 1 + } + return a.dueAt - b.dueAt || b.priority - a.priority + }) + } +} diff --git a/src/main/github/pr-refresh-rate-limit-gate.ts b/src/main/github/pr-refresh-rate-limit-gate.ts new file mode 100644 index 00000000000..19cee2e215d --- /dev/null +++ b/src/main/github/pr-refresh-rate-limit-gate.ts @@ -0,0 +1,27 @@ +import type { GitHubPRRefreshCandidate } from '../../shared/github/pull-request-refresh-types' +import { getOriginGitHubApiRepository } from './github-api-repository' +import { ghRepoExecOptions, githubRepoContext } from './gh-utils' +import { getRateLimit, repositoryRateLimitGuard, spendsSharedGitHubComQuota } from './rate-limit' + +const BUCKETS = ['core', 'graphql'] as const + +export async function prRefreshRateLimitPausedUntil( + candidate: GitHubPRRefreshCandidate, + warmSharedSnapshot: boolean +): Promise { + const executionOptions = ghRepoExecOptions( + githubRepoContext(candidate.repoPath, candidate.connectionId, candidate.localGitOptions) + ) + const repository = await getOriginGitHubApiRepository( + candidate.repoPath, + candidate.connectionId, + executionOptions + ) + if (warmSharedSnapshot && spendsSharedGitHubComQuota(repository, executionOptions)) { + await getRateLimit() + } + const blockedGuard = BUCKETS.map((bucket) => + repositoryRateLimitGuard(repository, bucket, executionOptions) + ).find((guard) => guard.blocked) + return blockedGuard?.blocked ? blockedGuard.resetAt * 1000 : null +} diff --git a/src/main/github/pr-refresh-retry-state.ts b/src/main/github/pr-refresh-retry-state.ts new file mode 100644 index 00000000000..fa66bf021a4 --- /dev/null +++ b/src/main/github/pr-refresh-retry-state.ts @@ -0,0 +1,46 @@ +import type { PRRefreshOutcome } from '../../shared/github/pull-request-refresh-types' +import { lookupBackoffDelayMs } from '../source-control/hosted-review-refresh-pacing' + +export class PRRefreshRetryState { + private readonly errorBackoff = new Map() + private readonly manualRetryGates = new Map() + + get errorBackoffCount(): number { + return this.errorBackoff.size + } + + reset(key: string): void { + this.errorBackoff.delete(key) + this.manualRetryGates.delete(key) + } + + noteManualGate(key: string, outcome: PRRefreshOutcome): void { + if (outcome.kind === 'upstream-error' && outcome.retryDisabledUntil !== undefined) { + this.manualRetryGates.set(key, outcome.retryDisabledUntil) + } else { + this.manualRetryGates.delete(key) + } + } + + manualGateUntil(key: string): number { + return this.manualRetryGates.get(key) ?? 0 + } + + nextVisibleErrorRetryAt(key: string): number { + const failures = (this.errorBackoff.get(key)?.failures ?? 0) + 1 + const retryAt = Date.now() + lookupBackoffDelayMs(failures) + this.errorBackoff.set(key, { failures, retryAt }) + return retryAt + } + + withErrorSchedule(outcome: PRRefreshOutcome, retryAt: number): PRRefreshOutcome { + if (outcome.kind !== 'upstream-error') { + return outcome + } + const cooldownUntil = outcome.retryDisabledUntil + return { + ...outcome, + nextAutoRetryAt: cooldownUntil !== undefined ? Math.max(retryAt, cooldownUntil) : retryAt + } + } +} diff --git a/src/main/github/pr-refresh-visibility.ts b/src/main/github/pr-refresh-visibility.ts new file mode 100644 index 00000000000..9673252b6fe --- /dev/null +++ b/src/main/github/pr-refresh-visibility.ts @@ -0,0 +1,44 @@ +import { webContents } from 'electron' +import type { GitHubPRRefreshCandidate } from '../../shared/github/pull-request-refresh-types' +import { refreshKey } from './pr-refresh-candidate-policy' + +export class PRRefreshVisibility { + private readonly visibleByWindow = new Map }>() + + get windowCount(): number { + return this.visibleByWindow.size + } + + clearWindow(windowId: number): boolean { + return this.visibleByWindow.delete(windowId) + } + + report(candidates: GitHubPRRefreshCandidate[], generation: number, windowId: number): boolean { + const existing = this.visibleByWindow.get(windowId) + if (existing && generation < existing.generation) { + return false + } + this.visibleByWindow.set(windowId, { generation, keys: new Set(candidates.map(refreshKey)) }) + return true + } + + has(key: string): boolean { + const liveWindowIds = new Set( + webContents + .getAllWebContents() + .filter((contents) => !contents.isDestroyed()) + .map((contents) => contents.id) + ) + for (const windowId of Array.from(this.visibleByWindow.keys())) { + if (!liveWindowIds.has(windowId)) { + this.visibleByWindow.delete(windowId) + } + } + for (const visible of this.visibleByWindow.values()) { + if (visible.keys.has(key)) { + return true + } + } + return false + } +} diff --git a/src/main/github/pull-request-file-contents.ts b/src/main/github/pull-request-file-contents.ts new file mode 100644 index 00000000000..fa367e1c68b --- /dev/null +++ b/src/main/github/pull-request-file-contents.ts @@ -0,0 +1,132 @@ +import type { GitHubPRFile, GitHubPRFileContents } from '../../shared/github/pull-request-types' +import { acquire, ghExecFileAsync, ghRepoExecOptions, githubRepoContext, release } from './gh-utils' +import type { LocalGitExecOptions } from './gh-utils' +import { + githubHostExecOptions, + resolveGitHubRepoExecution, + type GitHubApiRepository +} from './github-api-repository' +import { isMaxBufferOverflowError } from '../git/max-buffer-overflow' +import { noteRepositoryRateLimitSpend, repositoryRateLimitGuard } from './rate-limit' + +const GITHUB_RAW_CONTENT_MAX_BUFFER_BYTES = 8 * 1024 * 1024 + +function encodeGitHubContentPath(path: string): string { + return path.split('/').map(encodeURIComponent).join('/') +} + +async function fetchContentAtRef(args: { + repoPath: string + connectionId?: string | null + localGitOptions?: LocalGitExecOptions + ownerRepo: GitHubApiRepository + path: string + ref: string +}): Promise<{ content: string; isBinary: boolean; tooLarge?: boolean }> { + const ghOptions = { + ...ghRepoExecOptions(githubRepoContext(args.repoPath, args.connectionId, args.localGitOptions)), + ...githubHostExecOptions(args.ownerRepo), + maxBuffer: GITHUB_RAW_CONTENT_MAX_BUFFER_BYTES + } + if (repositoryRateLimitGuard(args.ownerRepo, 'core', ghOptions).blocked) { + return { content: '', isBinary: false } + } + try { + noteRepositoryRateLimitSpend(args.ownerRepo, 'core', 1, ghOptions) + const { stdout } = await ghExecFileAsync( + [ + 'api', + '--cache', + '300s', + '-H', + 'Accept: application/vnd.github.raw', + `repos/${args.ownerRepo.owner}/${args.ownerRepo.repo}/contents/${encodeGitHubContentPath(args.path)}?ref=${encodeURIComponent(args.ref)}` + ], + ghOptions + ) + const sample = stdout.slice(0, 2048) + if (sample.includes('\u0000')) { + return { content: '', isBinary: true } + } + return { content: stdout, isBinary: false } + } catch (error) { + if (isMaxBufferOverflowError(error)) { + return { content: '', isBinary: false, tooLarge: true } + } + return { content: '', isBinary: false } + } +} + +export async function getPRFileContents(args: { + repoPath: string + connectionId?: string | null + localGitOptions?: LocalGitExecOptions + prRepo?: GitHubApiRepository | null + prNumber: number + path: string + oldPath?: string + status: GitHubPRFile['status'] + headSha: string + baseSha: string +}): Promise { + const { ownerRepo } = await resolveGitHubRepoExecution( + args.repoPath, + args.prRepo, + args.connectionId, + args.localGitOptions + ) + if (!ownerRepo) { + return { + original: '', + modified: '', + originalIsBinary: false, + modifiedIsBinary: false + } + } + + await acquire() + try { + // Added and removed files have no content on one side of the comparison. + const needsOriginal = args.status !== 'added' + const needsModified = args.status !== 'removed' + const originalPath = args.oldPath ?? args.path + const [original, modified] = await Promise.all([ + needsOriginal + ? fetchContentAtRef({ + repoPath: args.repoPath, + connectionId: args.connectionId, + localGitOptions: args.localGitOptions, + ownerRepo, + path: originalPath, + ref: args.baseSha + }) + : Promise.resolve<{ content: string; isBinary: boolean; tooLarge?: boolean }>({ + content: '', + isBinary: false + }), + needsModified + ? fetchContentAtRef({ + repoPath: args.repoPath, + connectionId: args.connectionId, + localGitOptions: args.localGitOptions, + ownerRepo, + path: args.path, + ref: args.headSha + }) + : Promise.resolve<{ content: string; isBinary: boolean; tooLarge?: boolean }>({ + content: '', + isBinary: false + }) + ]) + return { + original: original.content, + modified: modified.content, + originalIsBinary: original.isBinary, + modifiedIsBinary: modified.isBinary, + originalTooLarge: original.tooLarge, + modifiedTooLarge: modified.tooLarge + } + } finally { + release() + } +} diff --git a/src/main/github/pull-request-file-data.ts b/src/main/github/pull-request-file-data.ts new file mode 100644 index 00000000000..62622e4583f --- /dev/null +++ b/src/main/github/pull-request-file-data.ts @@ -0,0 +1,247 @@ +import type { GitHubPRFile, GitHubPRFileViewedState } from '../../shared/github/pull-request-types' +import { ghExecFileAsync, ghRepoExecOptions, githubRepoContext } from './gh-utils' +import type { LocalGitExecOptions } from './gh-utils' +import { githubHostExecOptions, type GitHubApiRepository } from './github-api-repository' +import { getPRReviewCommentLineNumbersFromPatch } from './pr-review-comment-lines' +import { noteRepositoryRateLimitSpend, repositoryRateLimitGuard } from './rate-limit' + +const MAX_PR_FILES = 300 + +const PR_FILE_VIEWED_STATES_QUERY = `query($owner: String!, $repo: String!, $number: Int!, $after: String) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $number) { + id + files(first: 100, after: $after) { + pageInfo { hasNextPage endCursor } + nodes { + path + viewerViewedState + } + } + } + } +}` + +type RESTPRFile = { + filename: string + previous_filename?: string + status: string + additions: number + deletions: number + changes: number + patch?: string +} + +export type PRFileViewedStatesResult = { + pullRequestId: string + viewedStates: Map +} + +function mapFileStatus(raw: string): GitHubPRFile['status'] { + switch (raw) { + case 'added': + return 'added' + case 'removed': + return 'removed' + case 'modified': + return 'modified' + case 'renamed': + return 'renamed' + case 'copied': + return 'copied' + case 'changed': + return 'changed' + case 'unchanged': + return 'unchanged' + default: + return 'modified' + } +} + +function isBinaryHint(file: RESTPRFile): boolean { + return file.patch === undefined && file.changes > 0 +} + +export async function getPRMetadata( + repoPath: string, + prNumber: number, + repository: GitHubApiRepository | null, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise<{ body: string; headSha?: string; baseSha?: string }> { + if (!repository) { + return { body: '' } + } + const ghOptions = { + ...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)), + ...githubHostExecOptions(repository) + } + if (repositoryRateLimitGuard(repository, 'core', ghOptions).blocked) { + return { body: '' } + } + try { + noteRepositoryRateLimitSpend(repository, 'core', 1, ghOptions) + const { stdout } = await ghExecFileAsync( + ['api', '--cache', '60s', `repos/${repository.owner}/${repository.repo}/pulls/${prNumber}`], + ghOptions + ) + const data = JSON.parse(stdout) as { + body?: string | null + head?: { sha?: string } + base?: { sha?: string } + } + return { + body: data.body ?? '', + ...(data.head?.sha ? { headSha: data.head.sha } : {}), + ...(data.base?.sha ? { baseSha: data.base.sha } : {}) + } + } catch { + return { body: '' } + } +} + +// null means unavailable; [] means a successfully fetched empty PR. +export async function getPRFiles( + repoPath: string, + prNumber: number, + repository: GitHubApiRepository | null, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + if (!repository) { + return null + } + const ghOptions = { + ...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)), + ...githubHostExecOptions(repository) + } + try { + const data: RESTPRFile[] = [] + for (let page = 1; data.length < MAX_PR_FILES; page += 1) { + if (repositoryRateLimitGuard(repository, 'core', ghOptions).blocked) { + return null + } + const pageSuffix = page === 1 ? '' : `&page=${page}` + noteRepositoryRateLimitSpend(repository, 'core', 1, ghOptions) + const { stdout } = await ghExecFileAsync( + [ + 'api', + '--cache', + '60s', + `repos/${repository.owner}/${repository.repo}/pulls/${prNumber}/files?per_page=100${pageSuffix}` + ], + ghOptions + ) + const pageData = JSON.parse(stdout) as RESTPRFile[] + data.push(...pageData.slice(0, MAX_PR_FILES - data.length)) + if (pageData.length < 100) { + break + } + } + return data.map((file) => ({ + path: file.filename, + oldPath: file.previous_filename, + status: mapFileStatus(file.status), + additions: file.additions, + deletions: file.deletions, + isBinary: isBinaryHint(file), + reviewCommentLineNumbers: getPRReviewCommentLineNumbersFromPatch(file.patch) + })) + } catch { + return null + } +} + +export async function getPRFileViewedStates( + repoPath: string, + prNumber: number, + repository: GitHubApiRepository | null, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + if (!repository) { + return null + } + const ghOptions = { + ...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)), + ...githubHostExecOptions(repository) + } + if (repositoryRateLimitGuard(repository, 'graphql', ghOptions).blocked) { + return null + } + const viewedStates = new Map() + let pullRequestId: string | null = null + let after: string | null = null + + try { + for (let fetched = 0; fetched < MAX_PR_FILES; fetched += 100) { + const args = [ + 'api', + 'graphql', + '-f', + `query=${PR_FILE_VIEWED_STATES_QUERY}`, + '-f', + `owner=${repository.owner}`, + '-f', + `repo=${repository.repo}`, + '-F', + `number=${prNumber}` + ] + if (after) { + args.push('-f', `after=${after}`) + } + noteRepositoryRateLimitSpend(repository, 'graphql', 1, ghOptions) + const { stdout } = await ghExecFileAsync(args, ghOptions) + const parsed = JSON.parse(stdout) as { + data?: { + repository?: { + pullRequest?: { + id?: string + files?: { + pageInfo?: { hasNextPage?: boolean; endCursor?: string | null } + nodes?: { + path?: string | null + viewerViewedState?: GitHubPRFileViewedState | null + }[] + } + } | null + } | null + } + errors?: { message?: string }[] + } + if (parsed.errors && parsed.errors.length > 0) { + return null + } + const pullRequest = parsed.data?.repository?.pullRequest + if (!pullRequest?.id) { + return null + } + pullRequestId = pullRequest.id + for (const file of pullRequest.files?.nodes ?? []) { + if (file.path && file.viewerViewedState) { + viewedStates.set(file.path, file.viewerViewedState) + } + } + if (!pullRequest.files?.pageInfo?.hasNextPage || !pullRequest.files.pageInfo.endCursor) { + break + } + after = pullRequest.files.pageInfo.endCursor + } + } catch { + return null + } + return pullRequestId ? { pullRequestId, viewedStates } : null +} + +export function mergePRFileViewedStates( + files: GitHubPRFile[], + viewedStates: PRFileViewedStatesResult | null +): GitHubPRFile[] { + if (!viewedStates) { + return files + } + return files.map((file) => ({ + ...file, + viewerViewedState: viewedStates.viewedStates.get(file.path) ?? 'UNVIEWED' + })) +} diff --git a/src/main/github/work-item-details-api-parity.test.ts b/src/main/github/work-item-details-api-parity.test.ts new file mode 100644 index 00000000000..187680f64cd --- /dev/null +++ b/src/main/github/work-item-details-api-parity.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, expectTypeOf, it } from 'vitest' +import type { GitHubPRFile, GitHubPRFileContents } from '../../shared/github/pull-request-types' +import type { GitHubWorkItemDetails } from '../../shared/github/work-item-types' +import type { IssueSourcePreference } from '../../shared/repo-types' +import type { LocalGitExecOptions } from './gh-utils' +import type { GitHubApiRepository } from './github-api-repository' +import * as workItemDetails from './work-item-details' + +type GetWorkItemDetails = ( + repoPath: string, + number: number, + type?: 'issue' | 'pr', + connectionId?: string | null, + localGitOptions?: LocalGitExecOptions, + preference?: IssueSourcePreference +) => Promise + +type GetPRFileContents = (args: { + repoPath: string + connectionId?: string | null + localGitOptions?: LocalGitExecOptions + prRepo?: GitHubApiRepository | null + prNumber: number + path: string + oldPath?: string + status: GitHubPRFile['status'] + headSha: string + baseSha: string +}) => Promise + +describe('work-item-details public API parity', () => { + it('retains only the established runtime exports and call signatures', () => { + expect(Object.keys(workItemDetails).sort()).toEqual(['getPRFileContents', 'getWorkItemDetails']) + expectTypeOf(workItemDetails.getWorkItemDetails).toEqualTypeOf() + expectTypeOf(workItemDetails.getPRFileContents).toEqualTypeOf() + }) +}) diff --git a/src/main/github/work-item-details-pr-files.test.ts b/src/main/github/work-item-details-pr-files.test.ts index cac3c33eaf4..d0205332bd5 100644 --- a/src/main/github/work-item-details-pr-files.test.ts +++ b/src/main/github/work-item-details-pr-files.test.ts @@ -333,6 +333,36 @@ describe('getWorkItemDetails PR file listing', () => { ).toHaveLength(2) }) + it('skips an added file base read and preserves a head max-buffer overflow', async () => { + ghExecFileAsyncMock.mockRejectedValueOnce( + Object.assign(new Error('stdout maxBuffer length exceeded'), { code: 'ENOBUFS' }) + ) + + const contents = await getPRFileContents({ + repoPath: '/repo-root', + prRepo: { owner: 'team', repo: 'orca', host: 'github.acme-corp.com' }, + prNumber: 7, + path: 'src/large.ts', + status: 'added', + headSha: 'head-sha', + baseSha: 'base-sha' + }) + + expect(contents).toEqual({ + original: '', + modified: '', + originalIsBinary: false, + modifiedIsBinary: false, + originalTooLarge: undefined, + modifiedTooLarge: true + }) + expect(ghExecFileAsyncMock).toHaveBeenCalledOnce() + expect(ghExecFileAsyncMock.mock.calls[0][0]).toContain( + 'repos/team/orca/contents/src/large.ts?ref=head-sha' + ) + expect(noteRepositoryRateLimitSpendMock).toHaveBeenCalledOnce() + }) + it('does not fetch raw PR file contents while the repository core budget is blocked', async () => { repositoryRateLimitGuardMock.mockReturnValue({ blocked: true, diff --git a/src/main/github/work-item-details.ts b/src/main/github/work-item-details.ts index 276ee9d33eb..dafc4f514d6 100644 --- a/src/main/github/work-item-details.ts +++ b/src/main/github/work-item-details.ts @@ -1,1012 +1,41 @@ -/* eslint-disable max-lines -- Why: groups the PR/Issue fetch paths and file-contents resolver so caching/rate-limit strategy lives in one place. */ import type { PRCheckDetail } from '../../shared/github/check-types' -import type { - GitHubIssueTimelineItem, - GitHubIssueTimelineTarget, - PRComment -} from '../../shared/github/comment-types' -import type { - GitHubAssignableUser, - GitHubPRFile, - GitHubPRFileContents, - GitHubPRFileViewedState -} from '../../shared/github/pull-request-types' +import type { GitHubPRFile, GitHubPRFileContents } from '../../shared/github/pull-request-types' import type { GitHubWorkItem, GitHubWorkItemDetails } from '../../shared/github/work-item-types' import type { IssueSourcePreference } from '../../shared/repo-types' -import { - ghExecFileAsync, - acquire, - release, - ghRepoExecOptions, - githubRepoContext, - type LocalGitExecOptions -} from './gh-utils' -import { getWorkItem, getPRChecks, getPRComments } from './client' +import { getPRChecks, getPRComments, getWorkItem } from './client' +import { acquire, release, type LocalGitExecOptions } from './gh-utils' import { getIssueGitHubApiRepository, - githubHostExecOptions, resolveGitHubRepoExecution, type GitHubApiRepository } from './github-api-repository' -import { noteRepositoryRateLimitSpend, repositoryRateLimitGuard } from './rate-limit' -import { getPRReviewCommentLineNumbersFromPatch } from './pr-review-comment-lines' -import { isMaxBufferOverflowError } from '../git/max-buffer-overflow' - -// Why: cap total PR files so a massive PR can't starve the gh semaphore while paging (100/page). -const MAX_PR_FILES = 300 -// Why: bound noisy issue timelines so one huge issue can't monopolize gh/API time. -const MAX_ISSUE_TIMELINE_ITEMS = 300 -const GITHUB_REST_PAGE_SIZE = 100 -// Why: raw-fetch buffer must exceed the renderer's large-diff threshold, else the UI shows an empty diff instead of the fallback. -const GITHUB_RAW_CONTENT_MAX_BUFFER_BYTES = 8 * 1024 * 1024 +import { getIssueBodyAndComments, getIssueDetailsViaGraphQL } from './issue-work-item-details' +import { + getPRFiles, + getPRFileViewedStates, + getPRMetadata, + mergePRFileViewedStates +} from './pull-request-file-data' +import { getPRFileContents as loadPRFileContents } from './pull-request-file-contents' +import { + enrichItemDisplayAvatars, + getMentionParticipants, + getWorkItemParticipants +} from './work-item-participants' function localGitOptionArgs(options: LocalGitExecOptions = {}): [] | [LocalGitExecOptions] { return Object.keys(options).length > 0 ? [options] : [] } -function encodeGitHubContentPath(path: string): string { - return path.split('/').map(encodeURIComponent).join('/') -} - -const PR_FILE_VIEWED_STATES_QUERY = `query($owner: String!, $repo: String!, $number: Int!, $after: String) { - repository(owner: $owner, name: $repo) { - pullRequest(number: $number) { - id - files(first: 100, after: $after) { - pageInfo { hasNextPage endCursor } - nodes { - path - viewerViewedState - } - } - } - } -}` - -const WORK_ITEM_PARTICIPANTS_QUERY = `query($owner: String!, $repo: String!, $number: Int!, $isPr: Boolean!) { - repository(owner: $owner, name: $repo) { - pullRequest(number: $number) @include(if: $isPr) { - participants(first: 100) { - nodes { login avatarUrl(size: 48) ... on User { name } } - } - } - issue(number: $number) @skip(if: $isPr) { - participants(first: 100) { - nodes { login avatarUrl(size: 48) ... on User { name } } - } - } - } -}` - -// Why: one GraphQL round-trip replaces the 3 serial gh subprocesses (REST issue + comments + participants); falls back to the legacy path on failure. -const ISSUE_DETAILS_QUERY = `query($owner: String!, $repo: String!, $number: Int!) { - repository(owner: $owner, name: $repo) { - issue(number: $number) { - body - assignees(first: 50) { nodes { login avatarUrl(size: 48) ... on User { name } } } - participants(first: 100) { - nodes { login avatarUrl(size: 48) ... on User { name } } - } - comments(first: 100) { - nodes { - databaseId - body - createdAt - url - author { - login - avatarUrl(size: 48) - ... on Bot { __typename } - } - } - } - } - } -}` - -type GraphQLIssueDetailsResponse = { - data?: { - repository?: { - issue?: { - body?: string | null - assignees?: { nodes?: { login?: string; avatarUrl?: string; name?: string | null }[] } - participants?: { nodes?: GitHubAssignableUser[] } - comments?: { - nodes?: { - databaseId?: number | null - body?: string | null - createdAt?: string | null - url?: string | null - author?: { - login?: string | null - avatarUrl?: string | null - __typename?: string - } | null - }[] - } - } | null - } | null - } - errors?: { message?: string }[] -} - -type RestTimelineUser = { - login?: string | null - avatar_url?: string | null -} - -type RestTimelineIssue = { - number?: number | null - title?: string | null - html_url?: string | null - repository?: { - name?: string | null - owner?: { login?: string | null } | null - } | null - pull_request?: unknown -} - -type RestTimelineEvent = { - id?: number | string | null - node_id?: string | null - event?: string | null - actor?: RestTimelineUser | null - user?: RestTimelineUser | null - assignee?: RestTimelineUser | null - created_at?: string | null - source?: { - issue?: RestTimelineIssue | null - } | null - closer?: RestTimelineIssue | null - state_reason?: string | null - project_card?: { - column_name?: string | null - previous_column_name?: string | null - project_url?: string | null - } | null - project?: { - name?: string | null - } | null - project_column_name?: string | null - previous_column_name?: string | null -} - -function isSupportedTimelineEvent( - eventName: string | null | undefined -): eventName is GitHubIssueTimelineItem['event'] { - return ( - eventName === 'assigned' || - eventName === 'unassigned' || - eventName === 'mentioned' || - eventName === 'cross-referenced' || - eventName === 'closed' || - eventName === 'reopened' || - eventName === 'moved_columns_in_project' - ) -} - -function mapTimelineTarget( - issue: RestTimelineIssue | null | undefined -): GitHubIssueTimelineTarget | undefined { - if (!issue || typeof issue.number !== 'number' || !issue.html_url) { - return undefined - } - const owner = issue.repository?.owner?.login - const repo = issue.repository?.name - return { - type: issue.pull_request ? 'pr' : 'issue', - number: issue.number, - title: issue.title ?? '', - url: issue.html_url, - repository: owner && repo ? `${owner}/${repo}` : undefined - } -} - -function getTimelineActor(event: RestTimelineEvent): { login: string; avatarUrl: string } { - const actor = event.actor ?? event.user - return { - login: actor?.login ?? 'ghost', - avatarUrl: actor?.avatar_url ?? '' - } -} - -function mapRestTimelineEvent(event: RestTimelineEvent): GitHubIssueTimelineItem | null { - const eventName = event.event - if (!isSupportedTimelineEvent(eventName)) { - return null - } - if (!event.created_at) { - return null - } - const actor = getTimelineActor(event) - const id = String(event.node_id ?? event.id ?? `${eventName}:${event.created_at}`) - const base = { - id, - event: eventName, - actor: actor.login, - actorAvatarUrl: actor.avatarUrl, - createdAt: event.created_at - } - if (eventName === 'assigned' || eventName === 'unassigned') { - return { - ...base, - assignee: event.assignee?.login ?? undefined - } - } - if (eventName === 'mentioned' || eventName === 'cross-referenced') { - return { - ...base, - source: mapTimelineTarget(event.source?.issue) - } - } - if (eventName === 'closed') { - return { - ...base, - stateReason: event.state_reason ?? null, - closer: mapTimelineTarget(event.closer ?? event.source?.issue) - } - } - if (eventName === 'moved_columns_in_project') { - return { - ...base, - previousColumnName: - event.previous_column_name ?? event.project_card?.previous_column_name ?? null, - columnName: event.project_column_name ?? event.project_card?.column_name ?? null, - projectName: event.project?.name ?? null - } - } - return base -} - -function parseRestTimelineEventLines(stdout: string): RestTimelineEvent[] { - const events: RestTimelineEvent[] = [] - for (const line of stdout.split('\n')) { - const trimmed = line.trim() - if (!trimmed) { - continue - } - try { - const parsed = JSON.parse(trimmed) as unknown - if (typeof parsed === 'object' && parsed !== null && !Array.isArray(parsed)) { - events.push(parsed) - } - } catch { - // Skip malformed jq lines; timeline activity is auxiliary to issue details. - } - } - return events -} - -async function getIssueTimelineItems( - ownerRepo: GitHubApiRepository, - issueNumber: number, - ghOptions: ReturnType -): Promise { +async function withWorkItemDetailsPermit(operation: () => Promise): Promise { + await acquire() try { - const items: GitHubIssueTimelineItem[] = [] - for (let page = 1; items.length < MAX_ISSUE_TIMELINE_ITEMS; page += 1) { - if (repositoryRateLimitGuard(ownerRepo, 'core', ghOptions).blocked) { - return items - } - noteRepositoryRateLimitSpend(ownerRepo, 'core', 1, ghOptions) - const { stdout } = await ghExecFileAsync( - [ - 'api', - '--cache', - '60s', - `repos/${ownerRepo.owner}/${ownerRepo.repo}/issues/${issueNumber}/timeline?per_page=${GITHUB_REST_PAGE_SIZE}&page=${page}`, - '--jq', - '.[] | @json' - ], - { ...ghOptions, ...githubHostExecOptions(ownerRepo) } - ) - // Why: --jq emits NDJSON and explicit paging lets us stop once we hit the drawer cap. - const pageEvents = parseRestTimelineEventLines(stdout) - for (const event of pageEvents) { - const item = mapRestTimelineEvent(event) - if (!item) { - continue - } - items.push(item) - if (items.length === MAX_ISSUE_TIMELINE_ITEMS) { - break - } - } - if (pageEvents.length < GITHUB_REST_PAGE_SIZE) { - break - } - } - return items - } catch { - return [] + return await operation() + } finally { + release() } } -/** - * Fetch an issue's body, comments, assignees, participants, and timeline in one GraphQL round-trip. - * Returns null on any partial error so the caller falls back to the strict REST path. - * Avatars are resolved here so GHE users don't render blank. - */ -async function getIssueDetailsViaGraphQL( - repoPath: string, - issueNumber: number, - ownerRepo: GitHubApiRepository | null, - connectionId?: string | null, - localGitOptions: LocalGitExecOptions = {} -): Promise<{ - body: string - comments: PRComment[] - assignees: string[] - // Avatar-bearing assignees, kept separate from the login-only `assignees`; enriches avatars `gh` leaves blank (GHE). - assigneeUsers: GitHubAssignableUser[] - participants: GitHubAssignableUser[] - timelineItems: GitHubIssueTimelineItem[] -} | null> { - const ghOptions = { - ...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)), - ...githubHostExecOptions(ownerRepo) - } - if (!ownerRepo) { - return null - } - if (repositoryRateLimitGuard(ownerRepo, 'graphql', ghOptions).blocked) { - return null - } - try { - noteRepositoryRateLimitSpend(ownerRepo, 'graphql', 1, ghOptions) - const { stdout } = await ghExecFileAsync( - [ - 'api', - 'graphql', - '-f', - `query=${ISSUE_DETAILS_QUERY}`, - '-f', - `owner=${ownerRepo.owner}`, - '-f', - `repo=${ownerRepo.repo}`, - '-F', - `number=${issueNumber}` - ], - ghOptions - ) - const parsed = JSON.parse(stdout) as GraphQLIssueDetailsResponse - if (parsed.errors && parsed.errors.length > 0) { - // Why: any partial GraphQL error forces the strict REST fallback so the drawer never paints a half-built shell. - return null - } - const issue = parsed.data?.repository?.issue - if (!issue) { - return null - } - const comments: PRComment[] = (issue.comments?.nodes ?? []) - .filter((c) => typeof c.databaseId === 'number') - .map((c) => ({ - id: c.databaseId as number, - author: c.author?.login ?? 'ghost', - authorAvatarUrl: c.author?.avatarUrl ?? '', - body: c.body ?? '', - createdAt: c.createdAt ?? '', - url: c.url ?? '', - isBot: c.author?.__typename === 'Bot' - })) - const assigneeUsers: GitHubAssignableUser[] = (issue.assignees?.nodes ?? []) - .filter((a): a is { login: string; avatarUrl?: string; name?: string | null } => - Boolean(a.login) - ) - .map((a) => ({ - login: a.login, - name: a.name ?? null, - avatarUrl: a.avatarUrl ?? '' - })) - const assignees = assigneeUsers.map((a) => a.login) - const participants: GitHubAssignableUser[] = (issue.participants?.nodes ?? []) - .filter((u) => Boolean(u.login)) - .map((u) => ({ - login: u.login, - name: u.name ?? null, - avatarUrl: u.avatarUrl ?? '' - })) - const timelineItems = await getIssueTimelineItems(ownerRepo, issueNumber, ghOptions) - return { - body: issue.body ?? '', - comments, - assignees, - assigneeUsers, - participants, - timelineItems - } - } catch { - return null - } -} - -function mergeGitHubUsers(users: GitHubAssignableUser[]): GitHubAssignableUser[] { - const byLogin = new Map() - for (const user of users) { - if (!user.login) { - continue - } - const key = user.login.toLowerCase() - const existing = byLogin.get(key) - if (existing) { - // Why: return a new merged record instead of mutating caller-provided objects. - byLogin.set(key, { - login: existing.login, - name: existing.name ?? user.name ?? null, - avatarUrl: existing.avatarUrl || user.avatarUrl || '' - }) - continue - } - byLogin.set(key, { - login: user.login, - name: user.name ?? null, - avatarUrl: user.avatarUrl ?? '' - }) - } - return Array.from(byLogin.values()) -} - -type RESTPRFile = { - filename: string - previous_filename?: string - status: string - additions: number - deletions: number - changes: number - /** Raw patch text when available; absent for binary files or patches over GitHub's size cap. */ - patch?: string -} - -function mapFileStatus(raw: string): GitHubPRFile['status'] { - switch (raw) { - case 'added': - return 'added' - case 'removed': - return 'removed' - case 'modified': - return 'modified' - case 'renamed': - return 'renamed' - case 'copied': - return 'copied' - case 'changed': - return 'changed' - case 'unchanged': - return 'unchanged' - default: - return 'modified' - } -} - -// Why: REST doesn't flag binaries but omits `patch` for them; treat "changes but no patch" as binary so the diff tab shows a placeholder. -function isBinaryHint(file: RESTPRFile): boolean { - if (file.status === 'removed' || file.status === 'added') { - // Added/removed file with changes but no patch is almost always binary (images, oversized lockfiles). - return file.patch === undefined && file.changes > 0 - } - return file.patch === undefined && file.changes > 0 -} - -async function getPRMetadata( - repoPath: string, - prNumber: number, - ownerRepo: GitHubApiRepository | null, - connectionId?: string | null, - localGitOptions: LocalGitExecOptions = {} -): Promise<{ body: string; headSha?: string; baseSha?: string }> { - if (!ownerRepo) { - // Why: a bare `gh pr view` can honor ambient GH_HOST/GH_REPO after hosted - // repository resolution fails, returning metadata for the wrong PR. - return { body: '' } - } - const ghOptions = { - ...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)), - ...githubHostExecOptions(ownerRepo) - } - if (repositoryRateLimitGuard(ownerRepo, 'core', ghOptions).blocked) { - return { body: '' } - } - try { - noteRepositoryRateLimitSpend(ownerRepo, 'core', 1, ghOptions) - const { stdout } = await ghExecFileAsync( - ['api', '--cache', '60s', `repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${prNumber}`], - ghOptions - ) - const data = JSON.parse(stdout) as { - body?: string | null - head?: { sha?: string } - base?: { sha?: string } - } - return { - body: data.body ?? '', - ...(data.head?.sha ? { headSha: data.head.sha } : {}), - ...(data.base?.sha ? { baseSha: data.base.sha } : {}) - } - } catch { - return { body: '' } - } -} - -// Why: null = failed/blocked fetch (Files tab shows retry, not "No files changed."); [] = genuinely empty PR. -async function getPRFiles( - repoPath: string, - prNumber: number, - ownerRepo: GitHubApiRepository | null, - connectionId?: string | null, - localGitOptions: LocalGitExecOptions = {} -): Promise { - if (!ownerRepo) { - return null - } - const ghOptions = { - ...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)), - ...githubHostExecOptions(ownerRepo) - } - try { - const data: RESTPRFile[] = [] - for (let page = 1; data.length < MAX_PR_FILES; page += 1) { - if (repositoryRateLimitGuard(ownerRepo, 'core', ghOptions).blocked) { - return null - } - const pageSuffix = page === 1 ? '' : `&page=${page}` - noteRepositoryRateLimitSpend(ownerRepo, 'core', 1, ghOptions) - const { stdout } = await ghExecFileAsync( - [ - 'api', - '--cache', - '60s', - `repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${prNumber}/files?per_page=100${pageSuffix}` - ], - ghOptions - ) - const pageData = JSON.parse(stdout) as RESTPRFile[] - data.push(...pageData.slice(0, MAX_PR_FILES - data.length)) - if (pageData.length < 100) { - break - } - } - return data.map((file) => ({ - path: file.filename, - oldPath: file.previous_filename, - status: mapFileStatus(file.status), - additions: file.additions, - deletions: file.deletions, - isBinary: isBinaryHint(file), - reviewCommentLineNumbers: getPRReviewCommentLineNumbersFromPatch(file.patch) - })) - } catch { - return null - } -} - -type PRFileViewedStatesResult = { - pullRequestId: string - viewedStates: Map -} - -async function getPRFileViewedStates( - repoPath: string, - prNumber: number, - ownerRepo: GitHubApiRepository | null, - connectionId?: string | null, - localGitOptions: LocalGitExecOptions = {} -): Promise { - if (!ownerRepo) { - return null - } - const ghOptions = { - ...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)), - ...githubHostExecOptions(ownerRepo) - } - if (repositoryRateLimitGuard(ownerRepo, 'graphql', ghOptions).blocked) { - return null - } - const viewedStates = new Map() - let pullRequestId: string | null = null - let after: string | null = null - - try { - for (let fetched = 0; fetched < MAX_PR_FILES; fetched += 100) { - const args = [ - 'api', - 'graphql', - '-f', - `query=${PR_FILE_VIEWED_STATES_QUERY}`, - '-f', - `owner=${ownerRepo.owner}`, - '-f', - `repo=${ownerRepo.repo}`, - '-F', - `number=${prNumber}` - ] - if (after) { - args.push('-f', `after=${after}`) - } - noteRepositoryRateLimitSpend(ownerRepo, 'graphql', 1, ghOptions) - const { stdout } = await ghExecFileAsync(args, ghOptions) - const parsed = JSON.parse(stdout) as { - data?: { - repository?: { - pullRequest?: { - id?: string - files?: { - pageInfo?: { hasNextPage?: boolean; endCursor?: string | null } - nodes?: { - path?: string | null - viewerViewedState?: GitHubPRFileViewedState | null - }[] - } - } | null - } | null - } - errors?: { message?: string }[] - } - if (parsed.errors && parsed.errors.length > 0) { - return null - } - const pullRequest = parsed.data?.repository?.pullRequest - if (!pullRequest?.id) { - return null - } - pullRequestId = pullRequest.id - for (const file of pullRequest.files?.nodes ?? []) { - if (file.path && file.viewerViewedState) { - viewedStates.set(file.path, file.viewerViewedState) - } - } - if (!pullRequest.files?.pageInfo?.hasNextPage || !pullRequest.files.pageInfo.endCursor) { - break - } - after = pullRequest.files.pageInfo.endCursor - } - } catch { - return null - } - - return pullRequestId ? { pullRequestId, viewedStates } : null -} - -function mergePRFileViewedStates( - files: GitHubPRFile[], - viewedStates: PRFileViewedStatesResult | null -): GitHubPRFile[] { - if (!viewedStates) { - return files - } - return files.map((file) => ({ - ...file, - viewerViewedState: viewedStates.viewedStates.get(file.path) ?? 'UNVIEWED' - })) -} - -async function getIssueBodyAndComments( - repoPath: string, - issueNumber: number, - ownerRepo: GitHubApiRepository | null, - connectionId?: string | null, - localGitOptions: LocalGitExecOptions = {} -): Promise<{ - body: string - comments: PRComment[] - assignees: string[] - timelineItems: GitHubIssueTimelineItem[] -}> { - const ghOptions = { - ...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)), - ...githubHostExecOptions(ownerRepo) - } - try { - if (ownerRepo) { - if (repositoryRateLimitGuard(ownerRepo, 'core', ghOptions).blocked) { - return { body: '', comments: [], assignees: [], timelineItems: [] } - } - // Why: the fallback starts the issue and comments reads together; debit - // both before spawning so a failed response cannot leave quota overstated. - noteRepositoryRateLimitSpend(ownerRepo, 'core', 2, ghOptions) - const [issueResult, commentsResult, timelineItems] = await Promise.all([ - ghExecFileAsync( - [ - 'api', - '--cache', - '60s', - `repos/${ownerRepo.owner}/${ownerRepo.repo}/issues/${issueNumber}` - ], - ghOptions - ), - ghExecFileAsync( - [ - 'api', - '--cache', - '60s', - `repos/${ownerRepo.owner}/${ownerRepo.repo}/issues/${issueNumber}/comments?per_page=100` - ], - ghOptions - ), - getIssueTimelineItems(ownerRepo, issueNumber, ghOptions) - ]) - const issue = JSON.parse(issueResult.stdout) as { - body?: string | null - assignees?: { login: string }[] - } - type RESTComment = { - id: number - user: { login: string; avatar_url: string; type?: string } | null - body: string - created_at: string - html_url: string - } - const comments = (JSON.parse(commentsResult.stdout) as RESTComment[]).map( - (c): PRComment => ({ - id: c.id, - author: c.user?.login ?? 'ghost', - authorAvatarUrl: c.user?.avatar_url ?? '', - body: c.body ?? '', - createdAt: c.created_at, - url: c.html_url, - isBot: c.user?.type === 'Bot' - }) - ) - const assignees = (issue.assignees ?? []).map((a) => a.login) - return { body: issue.body ?? '', comments, assignees, timelineItems } - } - if (connectionId) { - // Why: connection-backed gh has no cwd. A bare issue lookup could honor - // process GH_REPO/GH_HOST and return an unrelated repository's issue. - return { body: '', comments: [], assignees: [], timelineItems: [] } - } - // Fallback: non-GitHub remote - const { stdout } = await ghExecFileAsync( - ['issue', 'view', String(issueNumber), '--json', 'body,comments,assignees'], - ghOptions - ) - const data = JSON.parse(stdout) as { - body?: string - comments?: { - author: { login: string } - body: string - createdAt: string - url: string - }[] - assignees?: { login: string }[] - } - const comments = (data.comments ?? []).map( - (c, i): PRComment => ({ - id: i, - author: c.author?.login ?? 'ghost', - authorAvatarUrl: '', - body: c.body ?? '', - createdAt: c.createdAt, - url: c.url ?? '' - }) - ) - const fallbackAssignees = (data.assignees ?? []).map((a) => a.login) - return { body: data.body ?? '', comments, assignees: fallbackAssignees, timelineItems: [] } - } catch { - return { body: '', comments: [], assignees: [], timelineItems: [] } - } -} - -async function getWorkItemParticipants( - repoPath: string, - item: Pick, - resolvedRepository: GitHubApiRepository | null, - connectionId?: string | null, - localGitOptions: LocalGitExecOptions = {} -): Promise { - // Why: reuse the fan-out's repository identity so details cannot drift across hosts. - const ownerRepo = resolvedRepository - if (!ownerRepo) { - return [] - } - const ghOptions = { - ...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)), - ...githubHostExecOptions(ownerRepo) - } - if (repositoryRateLimitGuard(ownerRepo, 'graphql', ghOptions).blocked) { - return [] - } - try { - noteRepositoryRateLimitSpend(ownerRepo, 'graphql', 1, ghOptions) - const { stdout } = await ghExecFileAsync( - [ - 'api', - 'graphql', - '-f', - `query=${WORK_ITEM_PARTICIPANTS_QUERY}`, - '-f', - `owner=${ownerRepo.owner}`, - '-f', - `repo=${ownerRepo.repo}`, - '-F', - `number=${item.number}`, - '-F', - `isPr=${item.type === 'pr'}` - ], - ghOptions - ) - const data = JSON.parse(stdout) as { - data?: { - repository?: { - pullRequest?: { - participants?: { nodes?: GitHubAssignableUser[] } - } | null - issue?: { - participants?: { nodes?: GitHubAssignableUser[] } - } | null - } - } - } - const nodes = - data.data?.repository?.pullRequest?.participants?.nodes ?? - data.data?.repository?.issue?.participants?.nodes ?? - [] - return nodes - .map((user) => ({ - login: user.login, - name: user.name ?? null, - avatarUrl: user.avatarUrl ?? '' - })) - .filter((user) => user.login) - } catch { - return [] - } -} - -async function getGitHubUsersByLogin( - repoPath: string, - logins: string[], - repository: GitHubApiRepository | null, - connectionId?: string | null, - localGitOptions: LocalGitExecOptions = {} -): Promise { - if (!repository) { - return [] - } - const uniqueLogins = Array.from( - new Set(logins.filter((login) => login && login !== 'ghost').map((login) => login.trim())) - ).slice(0, 40) - if (uniqueLogins.length === 0) { - return [] - } - const ghOptions = { - ...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)), - ...githubHostExecOptions(repository) - } - const guard = repositoryRateLimitGuard(repository, 'graphql', ghOptions) - if (guard.blocked) { - // Why: log the skip — callers degrade silently to blank GHE avatars, so it's otherwise untraceable. - console.warn( - `getGitHubUsersByLogin skipped: GraphQL rate-limit budget exhausted (${uniqueLogins.length} logins unresolved)` - ) - return [] - } - const fields = uniqueLogins - .map( - (login, index) => - `u${index}: user(login: ${JSON.stringify(login)}) { login name avatarUrl(size: 48) }` - ) - .join('\n') - try { - noteRepositoryRateLimitSpend(repository, 'graphql', 1, ghOptions) - const { stdout } = await ghExecFileAsync( - ['api', 'graphql', '-f', `query=query { ${fields} }`], - ghOptions - ) - const data = JSON.parse(stdout) as { - data?: Record< - string, - { - login?: string - name?: string | null - avatarUrl?: string | null - } | null - > - } - return Object.values(data.data ?? {}) - .filter( - ( - user - ): user is { - login: string - name?: string | null - avatarUrl?: string | null - } => Boolean(user?.login) - ) - .map((user) => ({ - login: user.login, - name: user.name ?? null, - avatarUrl: user.avatarUrl ?? '' - })) - } catch { - return [] - } -} - -/** - * Stamp GraphQL-resolved avatars onto a work item's author, reviewers, review requests, latest reviews, and assignees. - * - * Why: `gh pr view` omits avatar_url, so login-based avatars 404 on GHE; knownUsers carries the GraphQL-resolved ones. See #8784. - */ -function enrichItemDisplayAvatars( - item: Omit, - knownUsers: GitHubAssignableUser[] -): Omit { - const avatarByLogin = new Map() - for (const user of knownUsers) { - if (user.login && user.avatarUrl) { - avatarByLogin.set(user.login.toLowerCase(), user.avatarUrl) - } - } - if (avatarByLogin.size === 0) { - return item - } - // Why: prefer the GraphQL-resolved avatar — `gh pr view` returns empty/`u/0` placeholders for enterprise users; fall back to the original only when the lookup is empty. - const avatarFor = (login: string): string | undefined => avatarByLogin.get(login.toLowerCase()) - const resolvedAvatar = (login: string, existing?: string | null): string | undefined => - avatarFor(login) || existing || undefined - // Callers coalesce a missing result to each field's "no avatar" sentinel ('' or null); GitHubUserAvatar falls back to login URL then initials. - const authorAvatarUrl = (item.author ? avatarFor(item.author) : undefined) || item.authorAvatarUrl - return { - ...item, - ...(authorAvatarUrl ? { authorAvatarUrl } : {}), - ...(item.reviewRequests - ? { - reviewRequests: item.reviewRequests.map((user) => ({ - ...user, - avatarUrl: resolvedAvatar(user.login, user.avatarUrl) ?? '' - })) - } - : {}), - ...(item.latestReviews - ? { - latestReviews: item.latestReviews.map((review) => ({ - ...review, - avatarUrl: resolvedAvatar(review.login, review.avatarUrl) ?? null - })) - } - : {}), - ...(item.assignees - ? { - assignees: item.assignees.map((user) => ({ - ...user, - avatarUrl: resolvedAvatar(user.login, user.avatarUrl) ?? '' - })) - } - : {}) - } -} - -async function getMentionParticipants( - repoPath: string, - item: Pick< - GitHubWorkItem, - 'author' | 'number' | 'type' | 'reviewRequests' | 'latestReviews' | 'assignees' - >, - comments: PRComment[], - participants: GitHubAssignableUser[], - repository: GitHubApiRepository | null, - connectionId?: string | null, - localGitOptions: LocalGitExecOptions = {} -): Promise { - // Why: resolve mention authors + display users (reviewers/assignees) in one aliased trip so avatars reuse it without a second rate-limited lookup (#8784). - // Why: order display users before comment authors so the 40-login cap in getGitHubUsersByLogin can't drop a reviewer/assignee avatar. - const visibleLogins = [ - item.author ?? '', - ...(item.reviewRequests ?? []).map((user) => user.login), - ...(item.latestReviews ?? []).map((review) => review.login), - ...(item.assignees ?? []).map((user) => user.login), - ...comments.map((comment) => comment.author) - ] - const graphQlUsers = await getGitHubUsersByLogin( - repoPath, - visibleLogins, - repository, - connectionId, - localGitOptions - ) - return mergeGitHubUsers([...participants, ...graphQlUsers]) -} - async function getPRChecksForDetails( repoPath: string, prNumber: number, @@ -1028,22 +57,12 @@ async function getPRChecksForDetails( connectionId, ...localGitOptionArgs(localGitOptions) ) - } catch (err) { - // Why: checks are auxiliary — a gh failure must not block opening the PR drawer. - console.warn('getWorkItemDetails PR checks failed:', err) + } catch (error) { + console.warn('getWorkItemDetails PR checks failed:', error) return [] } } -async function withWorkItemDetailsPermit(operation: () => Promise): Promise { - await acquire() - try { - return await operation() - } finally { - release() - } -} - export async function getWorkItemDetails( repoPath: string, number: number, @@ -1072,7 +91,6 @@ export async function getWorkItemDetails( if (item.type === 'issue') { return withWorkItemDetailsPermit(async () => { - // Why: one GraphQL trip returns all issue details; fall back to the legacy fan-out on failure. const collapsed = await getIssueDetailsViaGraphQL( repoPath, item.number, @@ -1082,7 +100,6 @@ export async function getWorkItemDetails( ) if (collapsed) { return { - // Include assigneeUsers: non-participating assignees are absent from participants and keep a blank avatar (GHE). item: enrichItemDisplayAvatars(item, [ ...collapsed.participants, ...collapsed.assigneeUsers @@ -1094,7 +111,6 @@ export async function getWorkItemDetails( timelineItems: collapsed.timelineItems } } - // Fallback: fetch body/comments and participants in parallel. const [{ body, comments, assignees, timelineItems }, participants] = await Promise.all([ getIssueBodyAndComments( repoPath, @@ -1125,8 +141,7 @@ export async function getWorkItemDetails( }) } - // Why: getPRComments and getPRChecks own their semaphore permits. Keeping an - // outer permit while awaiting either can deadlock four concurrent detail loads. + // Nested client reads own their semaphore permits, so they stay outside these permits. const [[metadata, files, viewedStates, participants], comments] = await Promise.all([ Promise.all([ withWorkItemDetailsPermit(() => @@ -1159,7 +174,6 @@ export async function getWorkItemDetails( : Promise.resolve([]) ]) - // Why: mention hydration spawns gh directly while checks owns a permit; bound them without nesting. const [mentionParticipants, checks] = await Promise.all([ withWorkItemDetailsPermit(() => getMentionParticipants( @@ -1193,57 +207,12 @@ export async function getWorkItemDetails( baseSha: metadata.baseSha, pullRequestId: viewedStates?.pullRequestId, checks, - // Why: null (failed fetch) vs empty PR — Files tab shows retry, not "No files changed." files: files === null ? undefined : mergePRFileViewedStates(files, viewedStates), filesUnavailable: files === null, participants: mentionParticipants } } -// Why: Monaco DiffViewer needs original/modified text (not patches); --cache bounds rate-limit spend on rapid file-expands. -async function fetchContentAtRef(args: { - repoPath: string - connectionId?: string | null - localGitOptions?: LocalGitExecOptions - ownerRepo: GitHubApiRepository - path: string - ref: string -}): Promise<{ content: string; isBinary: boolean; tooLarge?: boolean }> { - const ghOptions = { - ...ghRepoExecOptions(githubRepoContext(args.repoPath, args.connectionId, args.localGitOptions)), - ...githubHostExecOptions(args.ownerRepo), - maxBuffer: GITHUB_RAW_CONTENT_MAX_BUFFER_BYTES - } - if (repositoryRateLimitGuard(args.ownerRepo, 'core', ghOptions).blocked) { - return { content: '', isBinary: false } - } - try { - noteRepositoryRateLimitSpend(args.ownerRepo, 'core', 1, ghOptions) - const { stdout } = await ghExecFileAsync( - [ - 'api', - '--cache', - '300s', - '-H', - 'Accept: application/vnd.github.raw', - `repos/${args.ownerRepo.owner}/${args.ownerRepo.repo}/contents/${encodeGitHubContentPath(args.path)}?ref=${encodeURIComponent(args.ref)}` - ], - ghOptions - ) - // Heuristic: a NUL byte in the first 2KB means binary (execFile decodes as lossy utf-8). - const sample = stdout.slice(0, 2048) - if (sample.includes('\u0000')) { - return { content: '', isBinary: true } - } - return { content: stdout, isBinary: false } - } catch (error) { - if (isMaxBufferOverflowError(error)) { - return { content: '', isBinary: false, tooLarge: true } - } - return { content: '', isBinary: false } - } -} - export async function getPRFileContents(args: { repoPath: string connectionId?: string | null @@ -1256,67 +225,5 @@ export async function getPRFileContents(args: { headSha: string baseSha: string }): Promise { - const { ownerRepo } = await resolveGitHubRepoExecution( - args.repoPath, - args.prRepo, - args.connectionId, - args.localGitOptions - ) - if (!ownerRepo) { - return { - original: '', - modified: '', - originalIsBinary: false, - modifiedIsBinary: false - } - } - - await acquire() - try { - // Why: added files have no base-ref original, removed files no head-ref modified; skip those to avoid spurious 404s. - const needsOriginal = args.status !== 'added' - const needsModified = args.status !== 'removed' - const originalRef = args.baseSha - const originalPath = args.oldPath ?? args.path - - const [original, modified] = await Promise.all([ - needsOriginal - ? fetchContentAtRef({ - repoPath: args.repoPath, - connectionId: args.connectionId, - localGitOptions: args.localGitOptions, - ownerRepo, - path: originalPath, - ref: originalRef - }) - : Promise.resolve<{ content: string; isBinary: boolean; tooLarge?: boolean }>({ - content: '', - isBinary: false - }), - needsModified - ? fetchContentAtRef({ - repoPath: args.repoPath, - connectionId: args.connectionId, - localGitOptions: args.localGitOptions, - ownerRepo, - path: args.path, - ref: args.headSha - }) - : Promise.resolve<{ content: string; isBinary: boolean; tooLarge?: boolean }>({ - content: '', - isBinary: false - }) - ]) - - return { - original: original.content, - modified: modified.content, - originalIsBinary: original.isBinary, - modifiedIsBinary: modified.isBinary, - originalTooLarge: original.tooLarge, - modifiedTooLarge: modified.tooLarge - } - } finally { - release() - } + return loadPRFileContents(args) } diff --git a/src/main/github/work-item-participants.ts b/src/main/github/work-item-participants.ts new file mode 100644 index 00000000000..0ac793b4812 --- /dev/null +++ b/src/main/github/work-item-participants.ts @@ -0,0 +1,241 @@ +import type { PRComment } from '../../shared/github/comment-types' +import type { GitHubAssignableUser } from '../../shared/github/pull-request-types' +import type { GitHubWorkItem } from '../../shared/github/work-item-types' +import { ghExecFileAsync, ghRepoExecOptions, githubRepoContext } from './gh-utils' +import type { LocalGitExecOptions } from './gh-utils' +import { githubHostExecOptions, type GitHubApiRepository } from './github-api-repository' +import { noteRepositoryRateLimitSpend, repositoryRateLimitGuard } from './rate-limit' + +const WORK_ITEM_PARTICIPANTS_QUERY = `query($owner: String!, $repo: String!, $number: Int!, $isPr: Boolean!) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $number) @include(if: $isPr) { + participants(first: 100) { + nodes { login avatarUrl(size: 48) ... on User { name } } + } + } + issue(number: $number) @skip(if: $isPr) { + participants(first: 100) { + nodes { login avatarUrl(size: 48) ... on User { name } } + } + } + } +}` + +function mergeGitHubUsers(users: GitHubAssignableUser[]): GitHubAssignableUser[] { + const byLogin = new Map() + for (const user of users) { + if (!user.login) { + continue + } + const key = user.login.toLowerCase() + const existing = byLogin.get(key) + if (existing) { + byLogin.set(key, { + login: existing.login, + name: existing.name ?? user.name ?? null, + avatarUrl: existing.avatarUrl || user.avatarUrl || '' + }) + continue + } + byLogin.set(key, { + login: user.login, + name: user.name ?? null, + avatarUrl: user.avatarUrl ?? '' + }) + } + return Array.from(byLogin.values()) +} + +export async function getWorkItemParticipants( + repoPath: string, + item: Pick, + repository: GitHubApiRepository | null, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + if (!repository) { + return [] + } + const ghOptions = { + ...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)), + ...githubHostExecOptions(repository) + } + if (repositoryRateLimitGuard(repository, 'graphql', ghOptions).blocked) { + return [] + } + try { + noteRepositoryRateLimitSpend(repository, 'graphql', 1, ghOptions) + const { stdout } = await ghExecFileAsync( + [ + 'api', + 'graphql', + '-f', + `query=${WORK_ITEM_PARTICIPANTS_QUERY}`, + '-f', + `owner=${repository.owner}`, + '-f', + `repo=${repository.repo}`, + '-F', + `number=${item.number}`, + '-F', + `isPr=${item.type === 'pr'}` + ], + ghOptions + ) + const data = JSON.parse(stdout) as { + data?: { + repository?: { + pullRequest?: { participants?: { nodes?: GitHubAssignableUser[] } } | null + issue?: { participants?: { nodes?: GitHubAssignableUser[] } } | null + } + } + } + const nodes = + data.data?.repository?.pullRequest?.participants?.nodes ?? + data.data?.repository?.issue?.participants?.nodes ?? + [] + return nodes + .map((user) => ({ + login: user.login, + name: user.name ?? null, + avatarUrl: user.avatarUrl ?? '' + })) + .filter((user) => user.login) + } catch { + return [] + } +} + +async function getGitHubUsersByLogin( + repoPath: string, + logins: string[], + repository: GitHubApiRepository | null, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + if (!repository) { + return [] + } + const uniqueLogins = Array.from( + new Set(logins.filter((login) => login && login !== 'ghost').map((login) => login.trim())) + ).slice(0, 40) + if (uniqueLogins.length === 0) { + return [] + } + const ghOptions = { + ...ghRepoExecOptions(githubRepoContext(repoPath, connectionId, localGitOptions)), + ...githubHostExecOptions(repository) + } + if (repositoryRateLimitGuard(repository, 'graphql', ghOptions).blocked) { + console.warn( + `getGitHubUsersByLogin skipped: GraphQL rate-limit budget exhausted (${uniqueLogins.length} logins unresolved)` + ) + return [] + } + const fields = uniqueLogins + .map( + (login, index) => + `u${index}: user(login: ${JSON.stringify(login)}) { login name avatarUrl(size: 48) }` + ) + .join('\n') + try { + noteRepositoryRateLimitSpend(repository, 'graphql', 1, ghOptions) + const { stdout } = await ghExecFileAsync( + ['api', 'graphql', '-f', `query=query { ${fields} }`], + ghOptions + ) + const data = JSON.parse(stdout) as { + data?: Record< + string, + { login?: string; name?: string | null; avatarUrl?: string | null } | null + > + } + return Object.values(data.data ?? {}) + .filter((user): user is { login: string; name?: string | null; avatarUrl?: string | null } => + Boolean(user?.login) + ) + .map((user) => ({ + login: user.login, + name: user.name ?? null, + avatarUrl: user.avatarUrl ?? '' + })) + } catch { + return [] + } +} + +export async function getMentionParticipants( + repoPath: string, + item: Pick< + GitHubWorkItem, + 'author' | 'number' | 'type' | 'reviewRequests' | 'latestReviews' | 'assignees' + >, + comments: PRComment[], + participants: GitHubAssignableUser[], + repository: GitHubApiRepository | null, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + const visibleLogins = [ + item.author ?? '', + ...(item.reviewRequests ?? []).map((user) => user.login), + ...(item.latestReviews ?? []).map((review) => review.login), + ...(item.assignees ?? []).map((user) => user.login), + ...comments.map((comment) => comment.author) + ] + const graphQlUsers = await getGitHubUsersByLogin( + repoPath, + visibleLogins, + repository, + connectionId, + localGitOptions + ) + return mergeGitHubUsers([...participants, ...graphQlUsers]) +} + +export function enrichItemDisplayAvatars( + item: Omit, + knownUsers: GitHubAssignableUser[] +): Omit { + const avatarByLogin = new Map() + for (const user of knownUsers) { + if (user.login && user.avatarUrl) { + avatarByLogin.set(user.login.toLowerCase(), user.avatarUrl) + } + } + if (avatarByLogin.size === 0) { + return item + } + const avatarFor = (login: string): string | undefined => avatarByLogin.get(login.toLowerCase()) + const resolvedAvatar = (login: string, existing?: string | null): string | undefined => + avatarFor(login) || existing || undefined + const authorAvatarUrl = (item.author ? avatarFor(item.author) : undefined) || item.authorAvatarUrl + return { + ...item, + ...(authorAvatarUrl ? { authorAvatarUrl } : {}), + ...(item.reviewRequests + ? { + reviewRequests: item.reviewRequests.map((user) => ({ + ...user, + avatarUrl: resolvedAvatar(user.login, user.avatarUrl) ?? '' + })) + } + : {}), + ...(item.latestReviews + ? { + latestReviews: item.latestReviews.map((review) => ({ + ...review, + avatarUrl: resolvedAvatar(review.login, review.avatarUrl) ?? null + })) + } + : {}), + ...(item.assignees + ? { + assignees: item.assignees.map((user) => ({ + ...user, + avatarUrl: resolvedAvatar(user.login, user.avatarUrl) ?? '' + })) + } + : {}) + } +} diff --git a/src/main/ipc/github-account-handlers.ts b/src/main/ipc/github-account-handlers.ts new file mode 100644 index 00000000000..b5d91b6757a --- /dev/null +++ b/src/main/ipc/github-account-handlers.ts @@ -0,0 +1,31 @@ +import { ipcMain } from 'electron' +import { appStarSourceSchema } from '../../shared/gh-star-source' +import { diagnoseGhAuth } from '../github/auth-diagnose' +import { checkOrcaStarred, getAuthenticatedViewer, starOrca } from '../github/client' +import { getRateLimit } from '../github/rate-limit' +import { getCohortAtEmit } from '../telemetry/cohort-classifier' +import { track } from '../telemetry/client' + +export function registerGitHubAccountHandlers(): void { + ipcMain.handle('gh:viewer', () => getAuthenticatedViewer()) + ipcMain.handle('gh:checkOrcaStarred', () => checkOrcaStarred()) + ipcMain.handle('gh:starOrca', async (_event, source: unknown) => { + const sourceParse = appStarSourceSchema.safeParse(source) + const starred = await starOrca() + if (starred && sourceParse.success) { + track('app_starred_orca', { + source: sourceParse.data, + ...getCohortAtEmit() + }) + } + return starred + }) + + ipcMain.handle('gh:rateLimit', (_event, args?: { force?: boolean }) => + getRateLimit(args?.force ? { force: true } : undefined) + ) + + ipcMain.handle('gh:diagnoseAuth', (_event, args?: { host?: string }) => + diagnoseGhAuth(args?.host) + ) +} diff --git a/src/main/ipc/github-ipc-channel-parity.test.ts b/src/main/ipc/github-ipc-channel-parity.test.ts new file mode 100644 index 00000000000..a54b7cfbf19 --- /dev/null +++ b/src/main/ipc/github-ipc-channel-parity.test.ts @@ -0,0 +1,100 @@ +import { readFileSync } from 'node:fs' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = await vi.hoisted(async () => { + const { createGitHubIpcMocks } = await import('./github-ipc-module-mocks') + return createGitHubIpcMocks() +}) + +vi.mock('electron', () => mocks.electron) +vi.mock('../github/client', () => mocks.client) +vi.mock('../github/work-item-details', () => mocks.workItemDetails) +vi.mock('../github/pr-refresh-coordinator', () => mocks.prRefresh) +vi.mock('../telemetry/client', () => mocks.telemetry) +vi.mock('../telemetry/cohort-classifier', () => mocks.cohort) +vi.mock('./ui', () => mocks.ui) + +import * as github from './github' +import { createGitHubIpcHarness } from './github-ipc-test-harness' + +const EXPECTED_GITHUB_IPC_CHANNELS = [ + 'gh:prForBranch', + 'gh:refreshPRNow', + 'gh:enqueuePRRefresh', + 'gh:reportVisiblePRRefreshCandidates', + 'gh:issue', + 'gh:listIssues', + 'gh:createIssue', + 'gh:listWorkItems', + 'gh:countWorkItems', + 'gh:workItem', + 'gh:workItemByOwnerRepo', + 'gh:workItemDetails', + 'gh:notifyWorkItemMutated', + 'gh:prFileContents', + 'gh:repoSlug', + 'gh:repoUpstream', + 'gh:prChecks', + 'gh:prCheckDetails', + 'gh:prComments', + 'gh:setPRCommentReaction', + 'gh:resolveReviewThread', + 'gh:setPRFileViewed', + 'gh:addPRReviewCommentReply', + 'gh:addPRReviewComment', + 'gh:updatePRTitle', + 'gh:mergePR', + 'gh:setPRAutoMerge', + 'gh:updatePRState', + 'gh:rerunPRChecks', + 'gh:requestPRReviewers', + 'gh:removePRReviewers', + 'gh:updateIssue', + 'gh:addIssueComment', + 'gh:listLabels', + 'gh:listAssignableUsers', + 'gh:viewer', + 'gh:checkOrcaStarred', + 'gh:starOrca', + 'gh:rateLimit', + 'gh:diagnoseAuth', + 'gh:listAccessibleProjects', + 'gh:resolveProjectRef', + 'gh:listProjectViews', + 'gh:getProjectViewTable', + 'gh:projectWorkItemDetailsBySlug', + 'gh:updateProjectItemField', + 'gh:clearProjectItemField', + 'gh:updateIssueBySlug', + 'gh:updatePullRequestBySlug', + 'gh:addIssueCommentBySlug', + 'gh:updateIssueCommentBySlug', + 'gh:deleteIssueCommentBySlug', + 'gh:listLabelsBySlug', + 'gh:listAssignableUsersBySlug', + 'gh:listIssueTypesBySlug', + 'gh:updateIssueTypeBySlug' +] as const + +describe('GitHub IPC channel parity', () => { + const harness = createGitHubIpcHarness(mocks) + + beforeEach(harness.reset) + + it('preserves the facade and fixed IPC channel contract', () => { + github.registerGitHubHandlers(harness.store as never, harness.stats as never) + + const preloadSource = readFileSync(new URL('../../preload/index.ts', import.meta.url), 'utf8') + const exposedChannels = [...preloadSource.matchAll(/ipcRenderer\.invoke\('(gh:[^']+)'/g)].map( + (match) => match[1] + ) + const registeredChannels = mocks.electron.ipcMain.handle.mock.calls.map( + ([channel]) => channel as string + ) + + expect(Object.keys(github)).toEqual(['registerGitHubHandlers']) + expect(new Set(registeredChannels).size).toBe(registeredChannels.length) + expect(registeredChannels).toEqual(EXPECTED_GITHUB_IPC_CHANNELS) + expect(registeredChannels.toSorted()).toEqual([...new Set(exposedChannels)].toSorted()) + }) +}) diff --git a/src/main/ipc/github-issue-mutation-handlers.ts b/src/main/ipc/github-issue-mutation-handlers.ts new file mode 100644 index 00000000000..992f0c6e11f --- /dev/null +++ b/src/main/ipc/github-issue-mutation-handlers.ts @@ -0,0 +1,106 @@ +import { ipcMain } from 'electron' +import type { GitHubOwnerRepo } from '../../shared/github/pull-request-types' +import type { GitHubIssueUpdate } from '../../shared/issue-mutation-types' +import type { TaskSourceContext } from '../../shared/task-source-context' +import { addIssueComment, listAssignableUsers, listLabels, updateIssue } from '../github/client' +import type { Store } from '../persistence' +import { + assertRegisteredGitHubRepo, + getGitHubLocalGitOptionArgs, + getGitHubRepoConnectionId, + type GitHubRepoScopedArgs +} from './github-repo-routing' +import { broadcastGitHubWorkItemMutation } from './github-work-item-mutation-events' + +export function registerGitHubIssueMutationHandlers(store: Store): void { + ipcMain.handle( + 'gh:updateIssue', + async (event, args: GitHubRepoScopedArgs & { number: number; updates: GitHubIssueUpdate }) => { + const repo = assertRegisteredGitHubRepo(args, store) + if (typeof args.number !== 'number' || !Number.isInteger(args.number) || args.number < 1) { + return { ok: false, error: 'Invalid issue number' } + } + if (!args.updates || typeof args.updates !== 'object') { + return { ok: false, error: 'Updates object is required' } + } + const result = await updateIssue( + repo.path, + args.number, + args.updates, + getGitHubRepoConnectionId(repo), + ...getGitHubLocalGitOptionArgs(store, repo) + ) + if (result.ok) { + broadcastGitHubWorkItemMutation( + { repoPath: repo.path, repoId: repo.id, type: 'issue', number: args.number }, + event.sender.id + ) + } + return result + } + ) + + ipcMain.handle( + 'gh:addIssueComment', + async ( + event, + args: { + repoPath: string + repoId?: string + sourceContext?: TaskSourceContext | null + number: number + body: string + type?: 'issue' | 'pr' + prRepo?: GitHubOwnerRepo | null + } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + if (typeof args.number !== 'number' || !Number.isInteger(args.number) || args.number < 1) { + return { ok: false, error: 'Invalid issue number' } + } + if (!args.body?.trim()) { + return { ok: false, error: 'Comment body required' } + } + const result = await addIssueComment( + repo.path, + args.number, + args.body.trim(), + getGitHubRepoConnectionId(repo), + args.prRepo ?? null, + ...getGitHubLocalGitOptionArgs(store, repo) + ) + if (result.ok) { + broadcastGitHubWorkItemMutation( + { + repoPath: repo.path, + repoId: repo.id, + type: args.type ?? 'issue', + number: args.number + }, + event.sender.id + ) + } + return result + } + ) + + ipcMain.handle('gh:listLabels', (_event, args: GitHubRepoScopedArgs) => { + const repo = assertRegisteredGitHubRepo(args, store) + return listLabels( + repo.path, + repo.issueSourcePreference, + getGitHubRepoConnectionId(repo), + ...getGitHubLocalGitOptionArgs(store, repo) + ) + }) + + ipcMain.handle('gh:listAssignableUsers', (_event, args: GitHubRepoScopedArgs) => { + const repo = assertRegisteredGitHubRepo(args, store) + return listAssignableUsers( + repo.path, + repo.issueSourcePreference, + getGitHubRepoConnectionId(repo), + ...getGitHubLocalGitOptionArgs(store, repo) + ) + }) +} diff --git a/src/main/ipc/github-pr-mutation-handlers.ts b/src/main/ipc/github-pr-mutation-handlers.ts new file mode 100644 index 00000000000..1d3aecc38e3 --- /dev/null +++ b/src/main/ipc/github-pr-mutation-handlers.ts @@ -0,0 +1,212 @@ +import { ipcMain } from 'electron' +import type { GitHubOwnerRepo } from '../../shared/github/pull-request-types' +import type { GitHubPullRequestStateUpdate } from '../../shared/issue-mutation-types' +import { + mergePR, + removePRReviewers, + requestPRReviewers, + rerunPRChecks, + setPRAutoMerge, + updatePRState, + updatePRTitle +} from '../github/client' +import type { Store } from '../persistence' +import { + assertRegisteredGitHubRepo, + getGitHubLocalGitOptionArgs, + getGitHubRepoConnectionId, + type GitHubRepoScopedArgs +} from './github-repo-routing' +import { broadcastGitHubWorkItemMutation } from './github-work-item-mutation-events' + +function broadcastSuccessfulPRMutation( + ok: boolean, + repoPath: string, + repoId: string, + prNumber: number, + senderId: number +): void { + if (ok) { + broadcastGitHubWorkItemMutation({ repoPath, repoId, type: 'pr', number: prNumber }, senderId) + } +} + +export function registerGitHubPRMutationHandlers(store: Store): void { + ipcMain.handle( + 'gh:updatePRTitle', + async ( + event, + args: { repoPath: string; prNumber: number; title: string; prRepo?: GitHubOwnerRepo | null } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + const ok = await updatePRTitle( + repo.path, + args.prNumber, + args.title, + getGitHubRepoConnectionId(repo), + args.prRepo ?? null, + ...getGitHubLocalGitOptionArgs(store, repo) + ) + broadcastSuccessfulPRMutation(ok, repo.path, repo.id, args.prNumber, event.sender.id) + return ok + } + ) + + ipcMain.handle( + 'gh:mergePR', + async ( + event, + args: GitHubRepoScopedArgs & { + prNumber: number + method?: 'merge' | 'squash' | 'rebase' + prRepo?: GitHubOwnerRepo | null + } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + const result = await mergePR( + repo.path, + args.prNumber, + args.method, + getGitHubRepoConnectionId(repo), + args.prRepo ?? null, + ...getGitHubLocalGitOptionArgs(store, repo) + ) + broadcastSuccessfulPRMutation(result.ok, repo.path, repo.id, args.prNumber, event.sender.id) + return result + } + ) + + ipcMain.handle( + 'gh:setPRAutoMerge', + async ( + event, + args: GitHubRepoScopedArgs & { + prNumber: number + enabled: boolean + method?: 'merge' | 'squash' | 'rebase' + prRepo?: GitHubOwnerRepo | null + } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + const result = await setPRAutoMerge( + repo.path, + args.prNumber, + args.enabled, + args.method, + getGitHubRepoConnectionId(repo), + args.prRepo ?? null, + ...getGitHubLocalGitOptionArgs(store, repo) + ) + broadcastSuccessfulPRMutation(result.ok, repo.path, repo.id, args.prNumber, event.sender.id) + return result + } + ) + + ipcMain.handle( + 'gh:updatePRState', + async ( + event, + args: GitHubRepoScopedArgs & { + prNumber: number + updates: GitHubPullRequestStateUpdate + prRepo?: GitHubOwnerRepo | null + } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + if ( + typeof args.prNumber !== 'number' || + !Number.isInteger(args.prNumber) || + args.prNumber < 1 + ) { + return { ok: false, error: 'Invalid pull request number' } + } + const result = await updatePRState( + repo.path, + args.prNumber, + args.updates, + getGitHubRepoConnectionId(repo), + args.prRepo ?? null, + ...getGitHubLocalGitOptionArgs(store, repo) + ) + broadcastSuccessfulPRMutation(result.ok, repo.path, repo.id, args.prNumber, event.sender.id) + return result + } + ) + + ipcMain.handle( + 'gh:rerunPRChecks', + async ( + _event, + args: GitHubRepoScopedArgs & { + prNumber: number + headSha?: string + failedOnly?: boolean + prRepo?: GitHubOwnerRepo | null + } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + if ( + typeof args.prNumber !== 'number' || + !Number.isInteger(args.prNumber) || + args.prNumber < 1 + ) { + return { ok: false, error: 'Invalid pull request number' } + } + return rerunPRChecks( + repo.path, + args.prNumber, + { headSha: args.headSha, failedOnly: args.failedOnly, prRepo: args.prRepo ?? null }, + getGitHubRepoConnectionId(repo), + ...getGitHubLocalGitOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle( + 'gh:requestPRReviewers', + async ( + event, + args: GitHubRepoScopedArgs & { + prNumber: number + reviewers: string[] + prRepo?: GitHubOwnerRepo | null + } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + const result = await requestPRReviewers( + repo.path, + args.prNumber, + args.reviewers, + getGitHubRepoConnectionId(repo), + args.prRepo ?? null, + ...getGitHubLocalGitOptionArgs(store, repo) + ) + broadcastSuccessfulPRMutation(result.ok, repo.path, repo.id, args.prNumber, event.sender.id) + return result + } + ) + + ipcMain.handle( + 'gh:removePRReviewers', + async ( + event, + args: GitHubRepoScopedArgs & { + prNumber: number + reviewers: string[] + prRepo?: GitHubOwnerRepo | null + } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + const result = await removePRReviewers( + repo.path, + args.prNumber, + args.reviewers, + getGitHubRepoConnectionId(repo), + args.prRepo ?? null, + ...getGitHubLocalGitOptionArgs(store, repo) + ) + broadcastSuccessfulPRMutation(result.ok, repo.path, repo.id, args.prNumber, event.sender.id) + return result + } + ) +} diff --git a/src/main/ipc/github-pr-read-handlers.ts b/src/main/ipc/github-pr-read-handlers.ts new file mode 100644 index 00000000000..1c200218f91 --- /dev/null +++ b/src/main/ipc/github-pr-read-handlers.ts @@ -0,0 +1,178 @@ +import { ipcMain } from 'electron' +import type { GitHubReactionContent } from '../../shared/github/comment-types' +import type { GitHubOwnerRepo } from '../../shared/github/pull-request-types' +import type { TaskSourceContext } from '../../shared/task-source-context' +import { + getPRCheckDetails, + getPRChecks, + getPRComments, + getRepoSlug, + getRepoUpstream, + resolveReviewThread, + setPRCommentReaction +} from '../github/client' +import type { Store } from '../persistence' +import { + assertRegisteredGitHubRepo, + getGitHubLocalGitOptionArgs, + getGitHubRepoConnectionId +} from './github-repo-routing' + +export function registerGitHubPRReadHandlers(store: Store): void { + ipcMain.handle('gh:repoSlug', (_event, args: { repoPath: string }) => { + const repo = assertRegisteredGitHubRepo(args, store) + const localGitOptions = getGitHubLocalGitOptionArgs(store, repo)[0] + return localGitOptions + ? getRepoSlug(repo.path, getGitHubRepoConnectionId(repo), { + localGitExecOptions: localGitOptions + }) + : getRepoSlug(repo.path, getGitHubRepoConnectionId(repo)) + }) + + ipcMain.handle('gh:repoUpstream', (_event, args: { repoPath: string }) => { + const repo = assertRegisteredGitHubRepo(args, store) + const localGitOptions = getGitHubLocalGitOptionArgs(store, repo)[0] + return localGitOptions + ? getRepoUpstream(repo.path, getGitHubRepoConnectionId(repo), { + localGitExecOptions: localGitOptions + }) + : getRepoUpstream(repo.path, getGitHubRepoConnectionId(repo)) + }) + + ipcMain.handle( + 'gh:prChecks', + ( + _event, + args: { + repoPath: string + repoId?: string | null + sourceContext?: TaskSourceContext | null + prNumber: number + headSha?: string + prRepo?: GitHubOwnerRepo | null + noCache?: boolean + } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + return getPRChecks( + repo.path, + args.prNumber, + args.headSha, + args.prRepo ?? null, + { noCache: args.noCache }, + getGitHubRepoConnectionId(repo), + ...getGitHubLocalGitOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle( + 'gh:prCheckDetails', + ( + _event, + args: { + repoPath: string + repoId?: string | null + sourceContext?: TaskSourceContext | null + checkRunId?: number + workflowRunId?: number + checkName?: string + url?: string | null + prRepo?: GitHubOwnerRepo | null + } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + return getPRCheckDetails( + repo.path, + { + checkRunId: args.checkRunId, + workflowRunId: args.workflowRunId, + checkName: args.checkName, + url: args.url, + prRepo: args.prRepo ?? null + }, + getGitHubRepoConnectionId(repo), + ...getGitHubLocalGitOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle( + 'gh:prComments', + ( + _event, + args: { + repoPath: string + repoId?: string | null + sourceContext?: TaskSourceContext | null + prNumber: number + prRepo?: GitHubOwnerRepo | null + noCache?: boolean + } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + return getPRComments( + repo.path, + args.prNumber, + { noCache: args.noCache, prRepo: args.prRepo ?? null }, + getGitHubRepoConnectionId(repo), + ...getGitHubLocalGitOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle( + 'gh:setPRCommentReaction', + ( + _event, + args: { + repoPath: string + repoId?: string | null + sourceContext?: TaskSourceContext | null + reactionSubjectId: string + content: GitHubReactionContent + reacted: boolean + prRepo?: GitHubOwnerRepo | null + } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + if (!args.reactionSubjectId?.trim()) { + return false + } + return setPRCommentReaction( + repo.path, + args.reactionSubjectId.trim(), + args.content, + args.reacted, + getGitHubRepoConnectionId(repo), + args.prRepo ?? null, + ...getGitHubLocalGitOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle( + 'gh:resolveReviewThread', + async ( + _event, + args: { + repoPath: string + repoId?: string | null + sourceContext?: TaskSourceContext | null + threadId: string + resolve: boolean + prRepo?: GitHubOwnerRepo | null + } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + return resolveReviewThread( + repo.path, + args.threadId, + args.resolve, + getGitHubRepoConnectionId(repo), + args.prRepo ?? null, + ...getGitHubLocalGitOptionArgs(store, repo) + ) + } + ) +} diff --git a/src/main/ipc/github-pr-refresh-handlers.ts b/src/main/ipc/github-pr-refresh-handlers.ts new file mode 100644 index 00000000000..ab4dc74ec7d --- /dev/null +++ b/src/main/ipc/github-pr-refresh-handlers.ts @@ -0,0 +1,161 @@ +import { ipcMain } from 'electron' +import { resolve } from 'node:path' +import type { + GitHubPRRefreshCandidate, + GitHubPRRefreshEnqueueResult, + GitHubPRRefreshReason, + PRRefreshOutcome +} from '../../shared/github/pull-request-refresh-types' +import type { Repo } from '../../shared/repo-types' +import { getPRForBranch, type GitHubPRBranchLookupOptions } from '../github/client' +import { + clearVisiblePRRefreshWindow, + enqueuePRRefresh, + refreshPRNow, + reportVisiblePRRefreshCandidates, + setPRRefreshOutcomeObserver +} from '../github/pr-refresh-coordinator' +import type { Store } from '../persistence' +import type { StatsCollector } from '../stats/collector' +import { + applyRegisteredRepoToPRRefreshCandidate, + assertRegisteredGitHubRepo, + getGitHubLocalGitOptionArgs, + getGitHubRepoConnectionId, + validateAutomaticPRRefreshCandidate +} from './github-repo-routing' + +const visibilityCleanupRegistered = new Set() + +export function registerGitHubPRRefreshHandlers(store: Store, stats: StatsCollector): void { + function recordPRIfNeeded(repo: Repo, outcome: PRRefreshOutcome): void { + if (outcome.kind === 'found' && !stats.hasCountedPR(outcome.pr.url)) { + stats.record({ + type: 'pr_created', + at: Date.now(), + repoId: repo.id, + meta: { prNumber: outcome.pr.number, prUrl: outcome.pr.url } + }) + } + } + + setPRRefreshOutcomeObserver((candidate, outcome) => { + const repo = + store.getRepos().find((entry) => entry.id === candidate.repoId) ?? + store.getRepos().find((entry) => resolve(entry.path) === resolve(candidate.repoPath)) + if (repo) { + recordPRIfNeeded(repo, outcome) + } + }) + + ipcMain.handle( + 'gh:prForBranch', + async ( + _event, + args: { + repoPath: string + branch: string + linkedPRNumber?: number | null + fallbackPRNumber?: number | null + acceptMergedFallbackPR?: boolean + currentHeadOid?: string | null + } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + const localGitOptions = getGitHubLocalGitOptionArgs(store, repo)[0] + const hostedReviewOptions = localGitOptions + ? { localGitExecOptions: localGitOptions } + : undefined + const currentHeadOid = + typeof args.currentHeadOid === 'string' && args.currentHeadOid.trim().length > 0 + ? args.currentHeadOid.trim() + : null + const lookupOptions: GitHubPRBranchLookupOptions | undefined = hostedReviewOptions + ? { ...hostedReviewOptions } + : args.acceptMergedFallbackPR === true || currentHeadOid !== null + ? {} + : undefined + if (lookupOptions && args.acceptMergedFallbackPR === true) { + lookupOptions.acceptMergedFallbackPR = true + } + if (lookupOptions && currentHeadOid !== null) { + lookupOptions.currentHeadOid = currentHeadOid + } + const lookupOptionArgs: [] | [GitHubPRBranchLookupOptions] = lookupOptions + ? [lookupOptions] + : [] + const pr = await getPRForBranch( + repo.path, + args.branch, + args.linkedPRNumber ?? null, + getGitHubRepoConnectionId(repo), + args.linkedPRNumber == null ? (args.fallbackPRNumber ?? null) : null, + ...lookupOptionArgs + ) + if (pr && !stats.hasCountedPR(pr.url)) { + stats.record({ + type: 'pr_created', + at: Date.now(), + repoId: repo.id, + meta: { prNumber: pr.number, prUrl: pr.url } + }) + } + return pr + } + ) + + ipcMain.handle( + 'gh:refreshPRNow', + async (_event, args: { candidate: GitHubPRRefreshCandidate }) => { + const repo = assertRegisteredGitHubRepo(args.candidate, store) + const outcome = await refreshPRNow( + applyRegisteredRepoToPRRefreshCandidate(store, repo, args.candidate) + ) + recordPRIfNeeded(repo, outcome) + return outcome + } + ) + + ipcMain.handle( + 'gh:enqueuePRRefresh', + ( + event, + args: { + candidate: GitHubPRRefreshCandidate + reason: GitHubPRRefreshReason + priority?: number + } + ): GitHubPRRefreshEnqueueResult => { + const validation = validateAutomaticPRRefreshCandidate(args.candidate, store) + if (validation.kind === 'skipped') { + return validation.result + } + enqueuePRRefresh(validation.candidate, args.reason, args.priority ?? 0, event?.sender?.id) + return { kind: 'queued' } + } + ) + + ipcMain.handle( + 'gh:reportVisiblePRRefreshCandidates', + (event, args: { candidates: GitHubPRRefreshCandidate[]; generation: number }) => { + const senderId = event.sender.id + if (!visibilityCleanupRegistered.has(senderId)) { + visibilityCleanupRegistered.add(senderId) + event.sender.once('destroyed', () => { + visibilityCleanupRegistered.delete(senderId) + clearVisiblePRRefreshWindow(senderId) + }) + } + const candidates: GitHubPRRefreshCandidate[] = [] + const repos = store.getRepos() + for (const candidate of args.candidates) { + const validation = validateAutomaticPRRefreshCandidate(candidate, store, repos) + if (validation.kind === 'ok') { + candidates.push(validation.candidate) + } + } + reportVisiblePRRefreshCandidates(candidates, args.generation, senderId) + return true + } + ) +} diff --git a/src/main/ipc/github-pr-review-handlers.ts b/src/main/ipc/github-pr-review-handlers.ts new file mode 100644 index 00000000000..637eb74c1de --- /dev/null +++ b/src/main/ipc/github-pr-review-handlers.ts @@ -0,0 +1,180 @@ +import { ipcMain } from 'electron' +import type { GitHubOwnerRepo } from '../../shared/github/pull-request-types' +import type { TaskSourceContext } from '../../shared/task-source-context' +import { addPRReviewComment, addPRReviewCommentReply, setPRFileViewed } from '../github/client' +import type { Store } from '../persistence' +import { + assertRegisteredGitHubRepo, + getGitHubLocalGitOptionArgs, + getGitHubRepoConnectionId +} from './github-repo-routing' +import { broadcastGitHubWorkItemMutation } from './github-work-item-mutation-events' + +export function registerGitHubPRReviewHandlers(store: Store): void { + ipcMain.handle( + 'gh:setPRFileViewed', + async ( + event, + args: { + repoPath: string + repoId?: string + prNumber: number + prRepo?: GitHubOwnerRepo | null + pullRequestId: string + path: string + viewed: boolean + } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + if ( + typeof args.prNumber !== 'number' || + !Number.isInteger(args.prNumber) || + args.prNumber < 1 + ) { + return false + } + if (!args.pullRequestId?.trim() || !args.path?.trim()) { + return false + } + const ok = await setPRFileViewed({ + repoPath: repo.path, + connectionId: getGitHubRepoConnectionId(repo), + localGitOptions: getGitHubLocalGitOptionArgs(store, repo)[0], + prRepo: args.prRepo ?? null, + pullRequestId: args.pullRequestId.trim(), + path: args.path, + viewed: Boolean(args.viewed) + }) + if (ok) { + broadcastGitHubWorkItemMutation( + { repoPath: repo.path, repoId: repo.id, type: 'pr', number: args.prNumber }, + event.sender.id + ) + } + return ok + } + ) + + ipcMain.handle( + 'gh:addPRReviewCommentReply', + async ( + event, + args: { + repoPath: string + repoId?: string + sourceContext?: TaskSourceContext | null + prNumber: number + commentId: number + body: string + threadId?: string + path?: string + line?: number + prRepo?: GitHubOwnerRepo | null + } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + if ( + typeof args.prNumber !== 'number' || + !Number.isInteger(args.prNumber) || + args.prNumber < 1 + ) { + return { ok: false, error: 'Invalid PR number' } + } + if ( + typeof args.commentId !== 'number' || + !Number.isInteger(args.commentId) || + args.commentId < 1 + ) { + return { ok: false, error: 'Invalid comment ID' } + } + if (!args.body?.trim()) { + return { ok: false, error: 'Comment body required' } + } + const result = await addPRReviewCommentReply( + repo.path, + args.prNumber, + args.commentId, + args.body.trim(), + args.threadId, + args.path, + args.line, + getGitHubRepoConnectionId(repo), + args.prRepo ?? null, + ...getGitHubLocalGitOptionArgs(store, repo) + ) + if (result.ok) { + broadcastGitHubWorkItemMutation( + { repoPath: repo.path, repoId: repo.id, type: 'pr', number: args.prNumber }, + event.sender.id + ) + } + return result + } + ) + + ipcMain.handle( + 'gh:addPRReviewComment', + async ( + event, + args: { + repoPath: string + prNumber: number + prRepo?: GitHubOwnerRepo | null + commitId: string + path: string + line: number + startLine?: number + body: string + } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + if ( + typeof args.prNumber !== 'number' || + !Number.isInteger(args.prNumber) || + args.prNumber < 1 + ) { + return { ok: false, error: 'Invalid PR number' } + } + if (typeof args.line !== 'number' || !Number.isInteger(args.line) || args.line < 1) { + return { ok: false, error: 'Invalid line number' } + } + if ( + args.startLine !== undefined && + (typeof args.startLine !== 'number' || + !Number.isInteger(args.startLine) || + args.startLine < 1 || + args.startLine > args.line) + ) { + return { ok: false, error: 'Invalid start line' } + } + if (!args.commitId?.trim()) { + return { ok: false, error: 'Missing PR head SHA' } + } + if (!args.path?.trim()) { + return { ok: false, error: 'File path required' } + } + if (!args.body?.trim()) { + return { ok: false, error: 'Comment body required' } + } + const result = await addPRReviewComment({ + repoPath: repo.path, + prRepo: args.prRepo ?? null, + prNumber: args.prNumber, + commitId: args.commitId.trim(), + path: args.path, + line: args.line, + startLine: args.startLine, + body: args.body.trim(), + connectionId: getGitHubRepoConnectionId(repo), + localGitOptions: getGitHubLocalGitOptionArgs(store, repo)[0] + }) + if (result.ok) { + broadcastGitHubWorkItemMutation( + { repoPath: repo.path, repoId: repo.id, type: 'pr', number: args.prNumber }, + event.sender.id + ) + } + return result + } + ) +} diff --git a/src/main/ipc/github-project-view-handlers.ts b/src/main/ipc/github-project-view-handlers.ts new file mode 100644 index 00000000000..0106bd80bc7 --- /dev/null +++ b/src/main/ipc/github-project-view-handlers.ts @@ -0,0 +1,89 @@ +import { ipcMain } from 'electron' +import type { + AddIssueCommentBySlugArgs, + ClearProjectItemFieldArgs, + DeleteIssueCommentBySlugArgs, + GetProjectViewTableArgs, + ListAccessibleProjectsArgs, + ListAssignableUsersBySlugArgs, + ListIssueTypesBySlugArgs, + ListLabelsBySlugArgs, + ListProjectViewsArgs, + ProjectWorkItemDetailsBySlugArgs, + ResolveProjectRefArgs, + UpdateIssueBySlugArgs, + UpdateIssueCommentBySlugArgs, + UpdateIssueTypeBySlugArgs, + UpdateProjectItemFieldArgs, + UpdatePullRequestBySlugArgs +} from '../../shared/github/project-request-types' +import { + addIssueCommentBySlug, + clearProjectItemFieldValue, + deleteIssueCommentBySlug, + getProjectViewTable, + getWorkItemDetailsBySlug, + listAccessibleProjects, + listAssignableUsersBySlug, + listIssueTypesBySlug, + listLabelsBySlug, + listProjectViews, + resolveProjectRef, + updateIssueBySlug, + updateIssueCommentBySlug, + updateIssueTypeBySlug, + updateProjectItemFieldValue, + updatePullRequestBySlug +} from '../github/project-view' + +export function registerGitHubProjectViewHandlers(): void { + ipcMain.handle('gh:listAccessibleProjects', (_event, args?: ListAccessibleProjectsArgs) => + listAccessibleProjects(args) + ) + ipcMain.handle('gh:resolveProjectRef', (_event, args: ResolveProjectRefArgs) => + resolveProjectRef(args) + ) + ipcMain.handle('gh:listProjectViews', (_event, args: ListProjectViewsArgs) => + listProjectViews(args) + ) + ipcMain.handle('gh:getProjectViewTable', (_event, args: GetProjectViewTableArgs) => + getProjectViewTable(args) + ) + ipcMain.handle( + 'gh:projectWorkItemDetailsBySlug', + (_event, args: ProjectWorkItemDetailsBySlugArgs) => getWorkItemDetailsBySlug(args) + ) + ipcMain.handle('gh:updateProjectItemField', (_event, args: UpdateProjectItemFieldArgs) => + updateProjectItemFieldValue(args) + ) + ipcMain.handle('gh:clearProjectItemField', (_event, args: ClearProjectItemFieldArgs) => + clearProjectItemFieldValue(args) + ) + ipcMain.handle('gh:updateIssueBySlug', (_event, args: UpdateIssueBySlugArgs) => + updateIssueBySlug(args) + ) + ipcMain.handle('gh:updatePullRequestBySlug', (_event, args: UpdatePullRequestBySlugArgs) => + updatePullRequestBySlug(args) + ) + ipcMain.handle('gh:addIssueCommentBySlug', (_event, args: AddIssueCommentBySlugArgs) => + addIssueCommentBySlug(args) + ) + ipcMain.handle('gh:updateIssueCommentBySlug', (_event, args: UpdateIssueCommentBySlugArgs) => + updateIssueCommentBySlug(args) + ) + ipcMain.handle('gh:deleteIssueCommentBySlug', (_event, args: DeleteIssueCommentBySlugArgs) => + deleteIssueCommentBySlug(args) + ) + ipcMain.handle('gh:listLabelsBySlug', (_event, args: ListLabelsBySlugArgs) => + listLabelsBySlug(args) + ) + ipcMain.handle('gh:listAssignableUsersBySlug', (_event, args: ListAssignableUsersBySlugArgs) => + listAssignableUsersBySlug(args) + ) + ipcMain.handle('gh:listIssueTypesBySlug', (_event, args: ListIssueTypesBySlugArgs) => + listIssueTypesBySlug(args) + ) + ipcMain.handle('gh:updateIssueTypeBySlug', (_event, args: UpdateIssueTypeBySlugArgs) => + updateIssueTypeBySlug(args) + ) +} diff --git a/src/main/ipc/github-repo-routing.ts b/src/main/ipc/github-repo-routing.ts new file mode 100644 index 00000000000..a7f93c2b44f --- /dev/null +++ b/src/main/ipc/github-repo-routing.ts @@ -0,0 +1,130 @@ +import { resolve } from 'node:path' +import { getRepoExecutionHostId } from '../../shared/execution-host' +import type { + GitHubPRRefreshCandidate, + GitHubPRRefreshEnqueueResult +} from '../../shared/github/pull-request-refresh-types' +import type { Repo } from '../../shared/repo-types' +import type { TaskSourceContext } from '../../shared/task-source-context' +import { + notePRRefreshValidationDenial, + type PRRefreshValidationDenialReason +} from '../github/pr-refresh-validation-backoff' +import type { Store } from '../persistence' +import { getLocalProjectWorktreeGitOptions } from '../project-runtime-git-options' + +export type GitHubRepoScopedArgs = { + repoPath: string + repoId?: string | null + sourceContext?: TaskSourceContext | null +} + +type RegisteredRepoValidationResult = + | { kind: 'ok'; repo: Repo } + | { kind: 'denied'; reason: PRRefreshValidationDenialReason; message: string } + +function validateRegisteredRepo( + args: string | GitHubRepoScopedArgs, + store: Store, + repos = store.getRepos() +): RegisteredRepoValidationResult { + const repoPath = typeof args === 'string' ? args : args.repoPath + const repoId = typeof args === 'string' ? undefined : args.repoId + const resolvedRepoPath = resolve(repoPath) + const repo = repos.find((candidate) => + repoId ? candidate.id === repoId : resolve(candidate.path) === resolvedRepoPath + ) + if (!repo) { + return { + kind: 'denied', + reason: 'unknown-repo', + message: 'Access denied: unknown repository path' + } + } + if (repoId && resolve(repo.path) !== resolvedRepoPath) { + return { + kind: 'denied', + reason: 'repo-path-mismatch', + message: 'Access denied: repository path does not match repo id' + } + } + if ( + typeof args !== 'string' && + args.sourceContext?.provider === 'github' && + args.sourceContext.hostId !== getRepoExecutionHostId(repo) + ) { + return { + kind: 'denied', + reason: 'host-mismatch', + message: 'Access denied: GitHub source host does not match repository host' + } + } + return { kind: 'ok', repo } +} + +export function assertRegisteredGitHubRepo( + args: string | GitHubRepoScopedArgs, + store: Store +): Repo { + const result = validateRegisteredRepo(args, store) + if (result.kind === 'denied') { + throw new Error(result.message) + } + return result.repo +} + +export function getGitHubRepoConnectionId(repo: Repo): string | null { + return repo.connectionId ?? null +} + +export function getGitHubLocalGitOptionArgs( + store: Store, + repo: Repo +): [] | [{ wslDistro?: string }] { + const localGitOptions = getLocalProjectWorktreeGitOptions(store, repo) + return Object.keys(localGitOptions).length > 0 ? [localGitOptions] : [] +} + +export function applyRegisteredRepoToPRRefreshCandidate( + store: Store, + repo: Repo, + candidate: GitHubPRRefreshCandidate +): GitHubPRRefreshCandidate { + const localGitOptions = getGitHubLocalGitOptionArgs(store, repo)[0] + const appliedCandidate = { ...candidate } + delete appliedCandidate.localGitOptions + delete appliedCandidate.connectionId + delete appliedCandidate.executionHostId + delete appliedCandidate.connectionState + return { + ...appliedCandidate, + repoPath: repo.path, + repoId: repo.id, + ...(localGitOptions ? { localGitOptions } : {}), + connectionId: getGitHubRepoConnectionId(repo), + executionHostId: repo.executionHostId ?? null, + connectionState: repo.connectionId ? 'connected' : 'unknown' + } +} + +export function validateAutomaticPRRefreshCandidate( + candidate: GitHubPRRefreshCandidate, + store: Store, + repos = store.getRepos() +): + | { kind: 'ok'; candidate: GitHubPRRefreshCandidate } + | { kind: 'skipped'; result: Extract } { + const result = validateRegisteredRepo(candidate, store, repos) + if (result.kind === 'denied') { + const skippedReason = notePRRefreshValidationDenial({ + repoId: candidate.repoId, + repoPath: candidate.repoPath, + reason: result.reason + }) + return { kind: 'skipped', result: { kind: 'skipped', skippedReason } } + } + return { + kind: 'ok', + candidate: applyRegisteredRepoToPRRefreshCandidate(store, result.repo, candidate) + } +} diff --git a/src/main/ipc/github-work-item-handlers.ts b/src/main/ipc/github-work-item-handlers.ts new file mode 100644 index 00000000000..26c21b3db81 --- /dev/null +++ b/src/main/ipc/github-work-item-handlers.ts @@ -0,0 +1,214 @@ +import { ipcMain } from 'electron' +import type { GitHubOwnerRepo, GitHubPRFile } from '../../shared/github/pull-request-types' +import type { GitHubCreateIssueFields } from '../../shared/issue-mutation-types' +import type { TaskSourceContext } from '../../shared/task-source-context' +import { + countWorkItems, + createIssue, + getIssue, + getWorkItem, + getWorkItemByOwnerRepo, + listIssues, + listWorkItems +} from '../github/client' +import { getPRFileContents, getWorkItemDetails } from '../github/work-item-details' +import type { Store } from '../persistence' +import { dispatchWorkItem, type WorkItemArgs } from './github-work-item-args' +import { + assertRegisteredGitHubRepo, + getGitHubLocalGitOptionArgs, + getGitHubRepoConnectionId, + type GitHubRepoScopedArgs +} from './github-repo-routing' +import { broadcastGitHubWorkItemMutation } from './github-work-item-mutation-events' + +export function registerGitHubWorkItemHandlers(store: Store): void { + ipcMain.handle( + 'gh:issue', + ( + _event, + args: { + repoPath: string + repoId?: string | null + sourceContext?: TaskSourceContext | null + number: number + } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + return getIssue( + repo.path, + args.number, + getGitHubRepoConnectionId(repo), + ...getGitHubLocalGitOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle('gh:listIssues', (_event, args: { repoPath: string; limit?: number }) => { + const repo = assertRegisteredGitHubRepo(args, store) + return listIssues( + repo.path, + args.limit, + repo.issueSourcePreference, + getGitHubRepoConnectionId(repo), + ...getGitHubLocalGitOptionArgs(store, repo) + ).then((result) => result.items) + }) + + ipcMain.handle( + 'gh:createIssue', + ( + _event, + args: GitHubRepoScopedArgs & { title: string; body: string } & GitHubCreateIssueFields + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + const fields = + args.labels !== undefined || args.assignees !== undefined + ? { labels: args.labels, assignees: args.assignees } + : undefined + return createIssue( + repo.path, + args.title, + args.body, + repo.issueSourcePreference, + getGitHubRepoConnectionId(repo), + fields, + ...getGitHubLocalGitOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle( + 'gh:listWorkItems', + ( + _event, + args: { + repoPath: string + repoId?: string + limit?: number + query?: string + page?: number + noCache?: boolean + } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + return listWorkItems( + repo.path, + args.limit, + args.query, + args.page, + repo.issueSourcePreference, + getGitHubRepoConnectionId(repo), + args.noCache, + ...getGitHubLocalGitOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle('gh:countWorkItems', (_event, args: { repoPath: string; query?: string }) => { + const repo = assertRegisteredGitHubRepo(args, store) + return countWorkItems( + repo.path, + args.query, + repo.issueSourcePreference, + getGitHubRepoConnectionId(repo), + ...getGitHubLocalGitOptionArgs(store, repo) + ) + }) + + ipcMain.handle('gh:workItem', (_event, args: WorkItemArgs) => { + const repo = assertRegisteredGitHubRepo(args, store) + return dispatchWorkItem(args, repo, getWorkItem, getGitHubLocalGitOptionArgs(store, repo)[0]) + }) + + ipcMain.handle( + 'gh:workItemByOwnerRepo', + ( + _event, + args: { + repoPath: string + owner: string + repo: string + host?: string + number: number + type: 'issue' | 'pr' + } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + return getWorkItemByOwnerRepo( + repo.path, + { owner: args.owner, repo: args.repo, ...(args.host ? { host: args.host } : {}) }, + args.number, + args.type, + getGitHubRepoConnectionId(repo), + ...getGitHubLocalGitOptionArgs(store, repo) + ) + } + ) + + ipcMain.handle('gh:workItemDetails', (_event, args: WorkItemArgs) => { + const repo = assertRegisteredGitHubRepo(args, store) + return dispatchWorkItem( + args, + repo, + getWorkItemDetails, + getGitHubLocalGitOptionArgs(store, repo)[0] + ) + }) + + ipcMain.handle( + 'gh:notifyWorkItemMutated', + (event, args: { repoPath: string; repoId?: string; type: 'issue' | 'pr'; number: number }) => { + const repo = args.repoId + ? store.getRepos().find((candidate) => candidate.id === args.repoId) + : assertRegisteredGitHubRepo(args, store) + if (!repo) { + return false + } + if ( + (args.type !== 'issue' && args.type !== 'pr') || + typeof args.number !== 'number' || + !Number.isInteger(args.number) || + args.number < 1 + ) { + return false + } + broadcastGitHubWorkItemMutation( + { repoPath: repo.path, repoId: repo.id, type: args.type, number: args.number }, + event.sender.id + ) + return true + } + ) + + ipcMain.handle( + 'gh:prFileContents', + ( + _event, + args: { + repoPath: string + prNumber: number + prRepo?: GitHubOwnerRepo | null + path: string + oldPath?: string + status: GitHubPRFile['status'] + headSha: string + baseSha: string + } + ) => { + const repo = assertRegisteredGitHubRepo(args, store) + return getPRFileContents({ + repoPath: repo.path, + connectionId: getGitHubRepoConnectionId(repo), + localGitOptions: getGitHubLocalGitOptionArgs(store, repo)[0], + prRepo: args.prRepo ?? null, + prNumber: args.prNumber, + path: args.path, + oldPath: args.oldPath, + status: args.status, + headSha: args.headSha, + baseSha: args.baseSha + }) + } + ) +} diff --git a/src/main/ipc/github-work-item-mutation-events.ts b/src/main/ipc/github-work-item-mutation-events.ts new file mode 100644 index 00000000000..5466dd8bdc4 --- /dev/null +++ b/src/main/ipc/github-work-item-mutation-events.ts @@ -0,0 +1,15 @@ +import { sendToTrustedUIRenderer } from './ui' + +export type GitHubWorkItemMutation = { + repoPath: string + repoId?: string + type: 'issue' | 'pr' + number: number +} + +export function broadcastGitHubWorkItemMutation( + payload: GitHubWorkItemMutation, + senderId?: number +): void { + sendToTrustedUIRenderer('gh:workItemMutated', payload, senderId) +} diff --git a/src/main/ipc/github.ts b/src/main/ipc/github.ts index 7673aff06ce..668d9fdca79 100644 --- a/src/main/ipc/github.ts +++ b/src/main/ipc/github.ts @@ -1,1312 +1,21 @@ -/* eslint-disable max-lines -- Why: all GitHub IPC handlers stay co-located so -the repo-path validation, preference-threading, and stats wiring patterns are -reviewable as one surface. Splitting by feature area would risk drifting -validation/gate conventions across handler files. */ -import { ipcMain } from 'electron' -import { resolve } from 'node:path' -import type { GitHubReactionContent } from '../../shared/github/comment-types' -import type { - GitHubPRRefreshCandidate, - GitHubPRRefreshEnqueueResult, - GitHubPRRefreshReason, - PRRefreshOutcome -} from '../../shared/github/pull-request-refresh-types' -import type { GitHubOwnerRepo, GitHubPRFile } from '../../shared/github/pull-request-types' -import type { - GitHubCreateIssueFields, - GitHubIssueUpdate, - GitHubPullRequestStateUpdate -} from '../../shared/issue-mutation-types' -import type { Repo } from '../../shared/repo-types' -import { getRepoExecutionHostId } from '../../shared/execution-host' -import type { TaskSourceContext } from '../../shared/task-source-context' import type { Store } from '../persistence' import type { StatsCollector } from '../stats/collector' -import { - getPRForBranch, - getIssue, - getRepoSlug, - getRepoUpstream, - listIssues, - listWorkItems, - countWorkItems, - getWorkItem, - getWorkItemByOwnerRepo, - createIssue, - updateIssue, - addIssueComment, - listLabels, - listAssignableUsers, - getAuthenticatedViewer, - getPRChecks, - getPRCheckDetails, - getPRComments, - setPRCommentReaction, - resolveReviewThread, - setPRFileViewed, - addPRReviewComment, - addPRReviewCommentReply, - updatePRTitle, - mergePR, - setPRAutoMerge, - updatePRState, - rerunPRChecks, - requestPRReviewers, - removePRReviewers, - checkOrcaStarred, - starOrca -} from '../github/client' -import type { GitHubPRBranchLookupOptions } from '../github/client' -import { - clearVisiblePRRefreshWindow, - enqueuePRRefresh, - refreshPRNow, - reportVisiblePRRefreshCandidates, - setPRRefreshOutcomeObserver -} from '../github/pr-refresh-coordinator' -import { getWorkItemDetails, getPRFileContents } from '../github/work-item-details' -import { getRateLimit } from '../github/rate-limit' -import { diagnoseGhAuth } from '../github/auth-diagnose' -import { - notePRRefreshValidationDenial, - type PRRefreshValidationDenialReason -} from '../github/pr-refresh-validation-backoff' -import { getLocalProjectWorktreeGitOptions } from '../project-runtime-git-options' -import { dispatchWorkItem, type WorkItemArgs } from './github-work-item-args' -import { - getProjectViewTable, - listAccessibleProjects, - resolveProjectRef, - listProjectViews, - getWorkItemDetailsBySlug, - updateProjectItemFieldValue, - clearProjectItemFieldValue, - updateIssueBySlug, - updatePullRequestBySlug, - addIssueCommentBySlug, - updateIssueCommentBySlug, - deleteIssueCommentBySlug, - listLabelsBySlug, - listAssignableUsersBySlug, - listIssueTypesBySlug, - updateIssueTypeBySlug -} from '../github/project-view' -import type { - AddIssueCommentBySlugArgs, - ClearProjectItemFieldArgs, - DeleteIssueCommentBySlugArgs, - GetProjectViewTableArgs, - ListAccessibleProjectsArgs, - ListAssignableUsersBySlugArgs, - ListIssueTypesBySlugArgs, - ListLabelsBySlugArgs, - ListProjectViewsArgs, - ProjectWorkItemDetailsBySlugArgs, - ResolveProjectRefArgs, - UpdateIssueBySlugArgs, - UpdateIssueCommentBySlugArgs, - UpdateIssueTypeBySlugArgs, - UpdateProjectItemFieldArgs, - UpdatePullRequestBySlugArgs -} from '../../shared/github/project-request-types' -import { appStarSourceSchema } from '../../shared/gh-star-source' -import { track } from '../telemetry/client' -import { getCohortAtEmit } from '../telemetry/cohort-classifier' -import { sendToTrustedUIRenderer } from './ui' - -const prRefreshVisibilityCleanupRegistered = new Set() - -// Why: the app renderer owns the SWR cache; browser guests cannot consume this -// event. Skip the origin because it already updated its cache optimistically. -function broadcastWorkItemMutated( - payload: { - repoPath: string - repoId?: string - type: 'issue' | 'pr' - number: number - }, - senderId?: number -): void { - sendToTrustedUIRenderer('gh:workItemMutated', payload, senderId) -} - -// Why: returns the full Repo object instead of just the path string so that -// callers have access to repo.id for stat tracking and other context. -type RepoScopedArgs = { - repoPath: string - repoId?: string | null - sourceContext?: TaskSourceContext | null -} - -type RegisteredRepoValidationResult = - | { kind: 'ok'; repo: Repo } - | { kind: 'denied'; reason: PRRefreshValidationDenialReason; message: string } - -function validateRegisteredRepo( - args: string | RepoScopedArgs, - store: Store, - repos = store.getRepos() -): RegisteredRepoValidationResult { - const repoPath = typeof args === 'string' ? args : args.repoPath - const repoId = typeof args === 'string' ? undefined : args.repoId - const resolvedRepoPath = resolve(repoPath) - const repo = repos.find((r) => (repoId ? r.id === repoId : resolve(r.path) === resolvedRepoPath)) - if (!repo) { - return { - kind: 'denied', - reason: 'unknown-repo', - message: 'Access denied: unknown repository path' - } - } - if (repoId && resolve(repo.path) !== resolvedRepoPath) { - return { - kind: 'denied', - reason: 'repo-path-mismatch', - message: 'Access denied: repository path does not match repo id' - } - } - if ( - typeof args !== 'string' && - args.sourceContext?.provider === 'github' && - args.sourceContext.hostId !== getRepoExecutionHostId(repo) - ) { - return { - kind: 'denied', - reason: 'host-mismatch', - message: 'Access denied: GitHub source host does not match repository host' - } - } - return { kind: 'ok', repo } -} - -function assertRegisteredRepo(args: string | RepoScopedArgs, store: Store): Repo { - const result = validateRegisteredRepo(args, store) - if (result.kind === 'denied') { - throw new Error(result.message) - } - return result.repo -} - -function repoConnectionId(repo: Repo): string | null { - return repo.connectionId ?? null -} - -function localGitOptionArgs(store: Store, repo: Repo): [] | [{ wslDistro?: string }] { - const localGitOptions = getLocalProjectWorktreeGitOptions(store, repo) - return Object.keys(localGitOptions).length > 0 ? [localGitOptions] : [] -} - -function applyRepoToPRRefreshCandidate( - store: Store, - repo: Repo, - candidate: GitHubPRRefreshCandidate -): GitHubPRRefreshCandidate { - const localGitOptions = localGitOptionArgs(store, repo)[0] - const appliedCandidate = { ...candidate } - delete appliedCandidate.localGitOptions - delete appliedCandidate.connectionId - delete appliedCandidate.executionHostId - delete appliedCandidate.connectionState - return { - ...appliedCandidate, - repoPath: repo.path, - repoId: repo.id, - ...(localGitOptions ? { localGitOptions } : {}), - connectionId: repoConnectionId(repo), - executionHostId: repo.executionHostId ?? null, - connectionState: repo.connectionId ? 'connected' : 'unknown' - } -} - -function validateAutomaticPRRefreshCandidate( - candidate: GitHubPRRefreshCandidate, - store: Store, - repos = store.getRepos() -): - | { kind: 'ok'; candidate: GitHubPRRefreshCandidate } - | { - kind: 'skipped' - result: Extract - } { - const result = validateRegisteredRepo(candidate, store, repos) - if (result.kind === 'denied') { - const skippedReason = notePRRefreshValidationDenial({ - repoId: candidate.repoId, - repoPath: candidate.repoPath, - reason: result.reason - }) - return { kind: 'skipped', result: { kind: 'skipped', skippedReason } } - } - return { kind: 'ok', candidate: applyRepoToPRRefreshCandidate(store, result.repo, candidate) } -} +import { registerGitHubAccountHandlers } from './github-account-handlers' +import { registerGitHubIssueMutationHandlers } from './github-issue-mutation-handlers' +import { registerGitHubPRMutationHandlers } from './github-pr-mutation-handlers' +import { registerGitHubPRReadHandlers } from './github-pr-read-handlers' +import { registerGitHubPRRefreshHandlers } from './github-pr-refresh-handlers' +import { registerGitHubPRReviewHandlers } from './github-pr-review-handlers' +import { registerGitHubProjectViewHandlers } from './github-project-view-handlers' +import { registerGitHubWorkItemHandlers } from './github-work-item-handlers' export function registerGitHubHandlers(store: Store, stats: StatsCollector): void { - function recordPRIfNeeded(repo: Repo, outcome: PRRefreshOutcome): void { - if (outcome.kind === 'found' && !stats.hasCountedPR(outcome.pr.url)) { - stats.record({ - type: 'pr_created', - at: Date.now(), - repoId: repo.id, - meta: { prNumber: outcome.pr.number, prUrl: outcome.pr.url } - }) - } - } - - setPRRefreshOutcomeObserver((candidate, outcome) => { - const repo = - store.getRepos().find((r) => r.id === candidate.repoId) ?? - store.getRepos().find((r) => resolve(r.path) === resolve(candidate.repoPath)) - if (repo) { - recordPRIfNeeded(repo, outcome) - } - }) - - ipcMain.handle( - 'gh:prForBranch', - async ( - _event, - args: { - repoPath: string - branch: string - linkedPRNumber?: number | null - fallbackPRNumber?: number | null - acceptMergedFallbackPR?: boolean - currentHeadOid?: string | null - } - ) => { - const repo = assertRegisteredRepo(args, store) - const localGitOptions = localGitOptionArgs(store, repo)[0] - const hostedReviewOptionArgs: [] | [{ localGitExecOptions: { wslDistro?: string } }] = - localGitOptions ? [{ localGitExecOptions: localGitOptions }] : [] - const currentHeadOid = - typeof args.currentHeadOid === 'string' && args.currentHeadOid.trim().length > 0 - ? args.currentHeadOid.trim() - : null - const lookupOptions: GitHubPRBranchLookupOptions | undefined = hostedReviewOptionArgs[0] - ? { ...hostedReviewOptionArgs[0] } - : args.acceptMergedFallbackPR === true || currentHeadOid !== null - ? {} - : undefined - if (lookupOptions && args.acceptMergedFallbackPR === true) { - lookupOptions.acceptMergedFallbackPR = true - } - if (lookupOptions && currentHeadOid !== null) { - lookupOptions.currentHeadOid = currentHeadOid - } - const lookupOptionArgs: [] | [GitHubPRBranchLookupOptions] = lookupOptions - ? [lookupOptions] - : [] - const pr = await getPRForBranch( - repo.path, - args.branch, - args.linkedPRNumber ?? null, - repoConnectionId(repo), - args.linkedPRNumber == null ? (args.fallbackPRNumber ?? null) : null, - ...lookupOptionArgs - ) - // Emit pr_created when a PR is first detected for a branch. - // Why here: the renderer polls gh:prForBranch to check PR status per worktree. - // This captures PRs opened from any workflow (Orca UI, gh CLI, github.com). - if (pr && !stats.hasCountedPR(pr.url)) { - stats.record({ - type: 'pr_created', - at: Date.now(), - repoId: repo.id, - meta: { prNumber: pr.number, prUrl: pr.url } - }) - } - return pr - } - ) - - ipcMain.handle( - 'gh:refreshPRNow', - async (_event, args: { candidate: GitHubPRRefreshCandidate }) => { - const repo = assertRegisteredRepo(args.candidate, store) - const outcome = await refreshPRNow(applyRepoToPRRefreshCandidate(store, repo, args.candidate)) - recordPRIfNeeded(repo, outcome) - return outcome - } - ) - - ipcMain.handle( - 'gh:enqueuePRRefresh', - ( - event, - args: { - candidate: GitHubPRRefreshCandidate - reason: GitHubPRRefreshReason - priority?: number - } - ): GitHubPRRefreshEnqueueResult => { - const validation = validateAutomaticPRRefreshCandidate(args.candidate, store) - if (validation.kind === 'skipped') { - return validation.result - } - const senderWindowId = event?.sender?.id - enqueuePRRefresh(validation.candidate, args.reason, args.priority ?? 0, senderWindowId) - return { kind: 'queued' } - } - ) - - ipcMain.handle( - 'gh:reportVisiblePRRefreshCandidates', - (event, args: { candidates: GitHubPRRefreshCandidate[]; generation: number }) => { - const senderId = event.sender.id - if (!prRefreshVisibilityCleanupRegistered.has(senderId)) { - prRefreshVisibilityCleanupRegistered.add(senderId) - event.sender.once('destroyed', () => { - prRefreshVisibilityCleanupRegistered.delete(senderId) - clearVisiblePRRefreshWindow(senderId) - }) - } - const candidates: GitHubPRRefreshCandidate[] = [] - const repos = store.getRepos() - for (const candidate of args.candidates) { - const validation = validateAutomaticPRRefreshCandidate(candidate, store, repos) - if (validation.kind === 'ok') { - candidates.push(validation.candidate) - } - } - reportVisiblePRRefreshCandidates(candidates, args.generation, senderId) - return true - } - ) - - ipcMain.handle( - 'gh:issue', - ( - _event, - args: { - repoPath: string - repoId?: string | null - sourceContext?: TaskSourceContext | null - number: number - } - ) => { - const repo = assertRegisteredRepo(args, store) - return getIssue( - repo.path, - args.number, - repoConnectionId(repo), - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle('gh:listIssues', (_event, args: { repoPath: string; limit?: number }) => { - const repo = assertRegisteredRepo(args, store) - // Why: listIssues now returns { items, error? }. The IPC handler unwraps to - // the items array for the existing contract; feature 1's UI consumes the - // richer envelope through `gh:listWorkItems` instead. - return listIssues( - repo.path, - args.limit, - repo.issueSourcePreference, - repoConnectionId(repo), - ...localGitOptionArgs(store, repo) - ).then((r) => r.items) - }) - - ipcMain.handle( - 'gh:createIssue', - (_event, args: RepoScopedArgs & { title: string; body: string } & GitHubCreateIssueFields) => { - const repo = assertRegisteredRepo(args, store) - const fields = - args.labels !== undefined || args.assignees !== undefined - ? { labels: args.labels, assignees: args.assignees } - : undefined - return createIssue( - repo.path, - args.title, - args.body, - repo.issueSourcePreference, - repoConnectionId(repo), - fields, - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle( - 'gh:listWorkItems', - ( - _event, - args: { - repoPath: string - repoId?: string - limit?: number - query?: string - page?: number - noCache?: boolean - } - ) => { - const repo = assertRegisteredRepo(args, store) - return listWorkItems( - repo.path, - args.limit, - args.query, - args.page, - repo.issueSourcePreference, - repoConnectionId(repo), - args.noCache, - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle('gh:countWorkItems', (_event, args: { repoPath: string; query?: string }) => { - const repo = assertRegisteredRepo(args, store) - return countWorkItems( - repo.path, - args.query, - repo.issueSourcePreference, - repoConnectionId(repo), - ...localGitOptionArgs(store, repo) - ) - }) - - ipcMain.handle('gh:workItem', (_event, args: WorkItemArgs) => { - const repo = assertRegisteredRepo(args, store) - return dispatchWorkItem(args, repo, getWorkItem, localGitOptionArgs(store, repo)[0]) - }) - ipcMain.handle( - 'gh:workItemByOwnerRepo', - ( - _event, - args: { - repoPath: string - owner: string - repo: string - host?: string - number: number - type: 'issue' | 'pr' - } - ) => { - const repo = assertRegisteredRepo(args, store) - return getWorkItemByOwnerRepo( - repo.path, - // Why: Enterprise host identity must survive the IPC boundary or the - // lookup falls back to gh's default host for a same-named repo. - { owner: args.owner, repo: args.repo, ...(args.host ? { host: args.host } : {}) }, - args.number, - args.type, - repoConnectionId(repo), - ...localGitOptionArgs(store, repo) - ) - } - ) - ipcMain.handle('gh:workItemDetails', (_event, args: WorkItemArgs) => { - const repo = assertRegisteredRepo(args, store) - return dispatchWorkItem(args, repo, getWorkItemDetails, localGitOptionArgs(store, repo)[0]) - }) - - ipcMain.handle( - 'gh:notifyWorkItemMutated', - ( - event, - args: { - repoPath: string - repoId?: string - type: 'issue' | 'pr' - number: number - } - ) => { - const repo = args.repoId - ? store.getRepos().find((candidate) => candidate.id === args.repoId) - : assertRegisteredRepo(args, store) - if (!repo) { - return false - } - if ( - (args.type !== 'issue' && args.type !== 'pr') || - typeof args.number !== 'number' || - !Number.isInteger(args.number) || - args.number < 1 - ) { - return false - } - broadcastWorkItemMutated( - { - repoPath: repo.path, - repoId: repo.id, - type: args.type, - number: args.number - }, - event.sender.id - ) - return true - } - ) - - ipcMain.handle( - 'gh:prFileContents', - ( - _event, - args: { - repoPath: string - prNumber: number - prRepo?: GitHubOwnerRepo | null - path: string - oldPath?: string - status: GitHubPRFile['status'] - headSha: string - baseSha: string - } - ) => { - const repo = assertRegisteredRepo(args, store) - return getPRFileContents({ - repoPath: repo.path, - connectionId: repoConnectionId(repo), - localGitOptions: localGitOptionArgs(store, repo)[0], - prRepo: args.prRepo ?? null, - prNumber: args.prNumber, - path: args.path, - oldPath: args.oldPath, - status: args.status, - headSha: args.headSha, - baseSha: args.baseSha - }) - } - ) - - ipcMain.handle('gh:repoSlug', (_event, args: { repoPath: string }) => { - const repo = assertRegisteredRepo(args, store) - const localGitOptions = localGitOptionArgs(store, repo)[0] - return localGitOptions - ? getRepoSlug(repo.path, repoConnectionId(repo), { localGitExecOptions: localGitOptions }) - : getRepoSlug(repo.path, repoConnectionId(repo)) - }) - - ipcMain.handle('gh:repoUpstream', (_event, args: { repoPath: string }) => { - const repo = assertRegisteredRepo(args, store) - const localGitOptions = localGitOptionArgs(store, repo)[0] - return localGitOptions - ? getRepoUpstream(repo.path, repoConnectionId(repo), { localGitExecOptions: localGitOptions }) - : getRepoUpstream(repo.path, repoConnectionId(repo)) - }) - - ipcMain.handle( - 'gh:prChecks', - ( - _event, - args: { - repoPath: string - repoId?: string | null - sourceContext?: TaskSourceContext | null - prNumber: number - headSha?: string - prRepo?: GitHubOwnerRepo | null - noCache?: boolean - } - ) => { - const repo = assertRegisteredRepo(args, store) - return getPRChecks( - repo.path, - args.prNumber, - args.headSha, - args.prRepo ?? null, - { - noCache: args.noCache - }, - repoConnectionId(repo), - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle( - 'gh:prCheckDetails', - ( - _event, - args: { - repoPath: string - repoId?: string | null - sourceContext?: TaskSourceContext | null - checkRunId?: number - workflowRunId?: number - checkName?: string - url?: string | null - prRepo?: GitHubOwnerRepo | null - } - ) => { - const repo = assertRegisteredRepo(args, store) - return getPRCheckDetails( - repo.path, - { - checkRunId: args.checkRunId, - workflowRunId: args.workflowRunId, - checkName: args.checkName, - url: args.url, - prRepo: args.prRepo ?? null - }, - repoConnectionId(repo), - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle( - 'gh:prComments', - ( - _event, - args: { - repoPath: string - repoId?: string | null - sourceContext?: TaskSourceContext | null - prNumber: number - prRepo?: GitHubOwnerRepo | null - noCache?: boolean - } - ) => { - const repo = assertRegisteredRepo(args, store) - return getPRComments( - repo.path, - args.prNumber, - { noCache: args.noCache, prRepo: args.prRepo ?? null }, - repoConnectionId(repo), - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle( - 'gh:setPRCommentReaction', - ( - _event, - args: { - repoPath: string - repoId?: string | null - sourceContext?: TaskSourceContext | null - reactionSubjectId: string - content: GitHubReactionContent - reacted: boolean - prRepo?: GitHubOwnerRepo | null - } - ) => { - const repo = assertRegisteredRepo(args, store) - if (!args.reactionSubjectId?.trim()) { - return false - } - return setPRCommentReaction( - repo.path, - args.reactionSubjectId.trim(), - args.content, - args.reacted, - repoConnectionId(repo), - args.prRepo ?? null, - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle( - 'gh:resolveReviewThread', - async ( - _event, - args: { - repoPath: string - repoId?: string | null - sourceContext?: TaskSourceContext | null - threadId: string - resolve: boolean - prRepo?: GitHubOwnerRepo | null - } - ) => { - const repo = assertRegisteredRepo(args, store) - // Why: thread resolve doesn't carry the PR number, so we cannot target - // a specific cache entry. The renderer cache stores per-(repo, type, number) - // entries — emitting a path-wide invalidation here would require a new - // event shape; instead, the drawer's existing thread-resolve UI updates - // its local state immediately and the next reopen pays one fresh fetch. - return resolveReviewThread( - repo.path, - args.threadId, - args.resolve, - repoConnectionId(repo), - args.prRepo ?? null, - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle( - 'gh:setPRFileViewed', - async ( - event, - args: { - repoPath: string - repoId?: string - prNumber: number - prRepo?: GitHubOwnerRepo | null - pullRequestId: string - path: string - viewed: boolean - } - ) => { - const repo = assertRegisteredRepo(args, store) - if ( - typeof args.prNumber !== 'number' || - !Number.isInteger(args.prNumber) || - args.prNumber < 1 - ) { - return false - } - if (!args.pullRequestId?.trim() || !args.path?.trim()) { - return false - } - const ok = await setPRFileViewed({ - repoPath: repo.path, - connectionId: repoConnectionId(repo), - localGitOptions: localGitOptionArgs(store, repo)[0], - prRepo: args.prRepo ?? null, - pullRequestId: args.pullRequestId.trim(), - path: args.path, - viewed: Boolean(args.viewed) - }) - if (ok) { - broadcastWorkItemMutated( - { repoPath: repo.path, repoId: repo.id, type: 'pr', number: args.prNumber }, - event.sender.id - ) - } - return ok - } - ) - - ipcMain.handle( - 'gh:addPRReviewCommentReply', - async ( - event, - args: { - repoPath: string - repoId?: string - sourceContext?: TaskSourceContext | null - prNumber: number - commentId: number - body: string - threadId?: string - path?: string - line?: number - prRepo?: GitHubOwnerRepo | null - } - ) => { - const repo = assertRegisteredRepo(args, store) - if ( - typeof args.prNumber !== 'number' || - !Number.isInteger(args.prNumber) || - args.prNumber < 1 - ) { - return { ok: false, error: 'Invalid PR number' } - } - if ( - typeof args.commentId !== 'number' || - !Number.isInteger(args.commentId) || - args.commentId < 1 - ) { - return { ok: false, error: 'Invalid comment ID' } - } - if (!args.body?.trim()) { - return { ok: false, error: 'Comment body required' } - } - const result = await addPRReviewCommentReply( - repo.path, - args.prNumber, - args.commentId, - args.body.trim(), - args.threadId, - args.path, - args.line, - repoConnectionId(repo), - args.prRepo ?? null, - ...localGitOptionArgs(store, repo) - ) - if (result.ok) { - broadcastWorkItemMutated( - { repoPath: repo.path, repoId: repo.id, type: 'pr', number: args.prNumber }, - event.sender.id - ) - } - return result - } - ) - - ipcMain.handle( - 'gh:addPRReviewComment', - async ( - event, - args: { - repoPath: string - prNumber: number - prRepo?: GitHubOwnerRepo | null - commitId: string - path: string - line: number - startLine?: number - body: string - } - ) => { - const repo = assertRegisteredRepo(args, store) - if ( - typeof args.prNumber !== 'number' || - !Number.isInteger(args.prNumber) || - args.prNumber < 1 - ) { - return { ok: false, error: 'Invalid PR number' } - } - if (typeof args.line !== 'number' || !Number.isInteger(args.line) || args.line < 1) { - return { ok: false, error: 'Invalid line number' } - } - if ( - args.startLine !== undefined && - (typeof args.startLine !== 'number' || - !Number.isInteger(args.startLine) || - args.startLine < 1 || - args.startLine > args.line) - ) { - return { ok: false, error: 'Invalid start line' } - } - if (!args.commitId?.trim()) { - return { ok: false, error: 'Missing PR head SHA' } - } - if (!args.path?.trim()) { - return { ok: false, error: 'File path required' } - } - if (!args.body?.trim()) { - return { ok: false, error: 'Comment body required' } - } - const result = await addPRReviewComment({ - repoPath: repo.path, - prRepo: args.prRepo ?? null, - prNumber: args.prNumber, - commitId: args.commitId.trim(), - path: args.path, - line: args.line, - startLine: args.startLine, - body: args.body.trim(), - connectionId: repoConnectionId(repo), - localGitOptions: localGitOptionArgs(store, repo)[0] - }) - if (result.ok) { - broadcastWorkItemMutated( - { repoPath: repo.path, repoId: repo.id, type: 'pr', number: args.prNumber }, - event.sender.id - ) - } - return result - } - ) - - ipcMain.handle( - 'gh:updatePRTitle', - async ( - event, - args: { repoPath: string; prNumber: number; title: string; prRepo?: GitHubOwnerRepo | null } - ) => { - const repo = assertRegisteredRepo(args, store) - const ok = await updatePRTitle( - repo.path, - args.prNumber, - args.title, - repoConnectionId(repo), - args.prRepo ?? null, - ...localGitOptionArgs(store, repo) - ) - if (ok) { - broadcastWorkItemMutated( - { repoPath: repo.path, repoId: repo.id, type: 'pr', number: args.prNumber }, - event.sender.id - ) - } - return ok - } - ) - - ipcMain.handle( - 'gh:mergePR', - async ( - event, - args: { - repoPath: string - repoId?: string | null - sourceContext?: TaskSourceContext | null - prNumber: number - method?: 'merge' | 'squash' | 'rebase' - prRepo?: GitHubOwnerRepo | null - } - ) => { - const repo = assertRegisteredRepo(args, store) - const result = await mergePR( - repo.path, - args.prNumber, - args.method, - repoConnectionId(repo), - args.prRepo ?? null, - ...localGitOptionArgs(store, repo) - ) - if (result.ok) { - broadcastWorkItemMutated( - { repoPath: repo.path, repoId: repo.id, type: 'pr', number: args.prNumber }, - event.sender.id - ) - } - return result - } - ) - - ipcMain.handle( - 'gh:setPRAutoMerge', - async ( - event, - args: { - repoPath: string - repoId?: string | null - sourceContext?: TaskSourceContext | null - prNumber: number - enabled: boolean - method?: 'merge' | 'squash' | 'rebase' - prRepo?: GitHubOwnerRepo | null - } - ) => { - const repo = assertRegisteredRepo(args, store) - const result = await setPRAutoMerge( - repo.path, - args.prNumber, - args.enabled, - args.method, - repoConnectionId(repo), - args.prRepo ?? null, - ...localGitOptionArgs(store, repo) - ) - if (result.ok) { - broadcastWorkItemMutated( - { repoPath: repo.path, repoId: repo.id, type: 'pr', number: args.prNumber }, - event.sender.id - ) - } - return result - } - ) - - ipcMain.handle( - 'gh:updatePRState', - async ( - event, - args: RepoScopedArgs & { - prNumber: number - updates: GitHubPullRequestStateUpdate - prRepo?: GitHubOwnerRepo | null - } - ) => { - const repo = assertRegisteredRepo(args, store) - if ( - typeof args.prNumber !== 'number' || - !Number.isInteger(args.prNumber) || - args.prNumber < 1 - ) { - return { ok: false, error: 'Invalid pull request number' } - } - const result = await updatePRState( - repo.path, - args.prNumber, - args.updates, - repoConnectionId(repo), - args.prRepo ?? null, - ...localGitOptionArgs(store, repo) - ) - if (result.ok) { - broadcastWorkItemMutated( - { repoPath: repo.path, repoId: repo.id, type: 'pr', number: args.prNumber }, - event.sender.id - ) - } - return result - } - ) - - ipcMain.handle( - 'gh:rerunPRChecks', - async ( - _event, - args: RepoScopedArgs & { - prNumber: number - headSha?: string - failedOnly?: boolean - prRepo?: GitHubOwnerRepo | null - } - ) => { - const repo = assertRegisteredRepo(args, store) - if ( - typeof args.prNumber !== 'number' || - !Number.isInteger(args.prNumber) || - args.prNumber < 1 - ) { - return { ok: false, error: 'Invalid pull request number' } - } - return rerunPRChecks( - repo.path, - args.prNumber, - { headSha: args.headSha, failedOnly: args.failedOnly, prRepo: args.prRepo ?? null }, - repoConnectionId(repo), - ...localGitOptionArgs(store, repo) - ) - } - ) - - ipcMain.handle( - 'gh:requestPRReviewers', - async ( - event, - args: RepoScopedArgs & { - prNumber: number - reviewers: string[] - prRepo?: GitHubOwnerRepo | null - } - ) => { - const repo = assertRegisteredRepo(args, store) - const result = await requestPRReviewers( - repo.path, - args.prNumber, - args.reviewers, - repoConnectionId(repo), - args.prRepo ?? null, - ...localGitOptionArgs(store, repo) - ) - if (result.ok) { - broadcastWorkItemMutated( - { repoPath: repo.path, repoId: repo.id, type: 'pr', number: args.prNumber }, - event.sender.id - ) - } - return result - } - ) - - ipcMain.handle( - 'gh:removePRReviewers', - async ( - event, - args: RepoScopedArgs & { - prNumber: number - reviewers: string[] - prRepo?: GitHubOwnerRepo | null - } - ) => { - const repo = assertRegisteredRepo(args, store) - const result = await removePRReviewers( - repo.path, - args.prNumber, - args.reviewers, - repoConnectionId(repo), - args.prRepo ?? null, - ...localGitOptionArgs(store, repo) - ) - if (result.ok) { - broadcastWorkItemMutated( - { repoPath: repo.path, repoId: repo.id, type: 'pr', number: args.prNumber }, - event.sender.id - ) - } - return result - } - ) - - ipcMain.handle( - 'gh:updateIssue', - async (event, args: RepoScopedArgs & { number: number; updates: GitHubIssueUpdate }) => { - const repo = assertRegisteredRepo(args, store) - if (typeof args.number !== 'number' || !Number.isInteger(args.number) || args.number < 1) { - return { ok: false, error: 'Invalid issue number' } - } - if (!args.updates || typeof args.updates !== 'object') { - return { ok: false, error: 'Updates object is required' } - } - const result = await updateIssue( - repo.path, - args.number, - args.updates, - repoConnectionId(repo), - ...localGitOptionArgs(store, repo) - ) - if (result.ok) { - broadcastWorkItemMutated( - { repoPath: repo.path, repoId: repo.id, type: 'issue', number: args.number }, - event.sender.id - ) - } - return result - } - ) - - ipcMain.handle( - 'gh:addIssueComment', - async ( - event, - args: { - repoPath: string - repoId?: string - sourceContext?: TaskSourceContext | null - number: number - body: string - type?: 'issue' | 'pr' - prRepo?: GitHubOwnerRepo | null - } - ) => { - const repo = assertRegisteredRepo(args, store) - if (typeof args.number !== 'number' || !Number.isInteger(args.number) || args.number < 1) { - return { ok: false, error: 'Invalid issue number' } - } - if (!args.body?.trim()) { - return { ok: false, error: 'Comment body required' } - } - const result = await addIssueComment( - repo.path, - args.number, - args.body.trim(), - repoConnectionId(repo), - args.prRepo ?? null, - ...localGitOptionArgs(store, repo) - ) - if (result.ok) { - // Why: PR conversation comments hit `/issues/N/comments` too, but the - // drawer's cache key uses type='pr'. The caller passes through which - // drawer they're posting from so we only invalidate the matching key - // — broadcasting both would evict an unrelated PR/issue that happens - // to share the number. - broadcastWorkItemMutated( - { repoPath: repo.path, repoId: repo.id, type: args.type ?? 'issue', number: args.number }, - event.sender.id - ) - } - return result - } - ) - - ipcMain.handle('gh:listLabels', (_event, args: RepoScopedArgs) => { - const repo = assertRegisteredRepo(args, store) - return listLabels( - repo.path, - repo.issueSourcePreference, - repoConnectionId(repo), - ...localGitOptionArgs(store, repo) - ) - }) - - ipcMain.handle('gh:listAssignableUsers', (_event, args: RepoScopedArgs) => { - const repo = assertRegisteredRepo(args, store) - return listAssignableUsers( - repo.path, - repo.issueSourcePreference, - repoConnectionId(repo), - ...localGitOptionArgs(store, repo) - ) - }) - - // Star operations target the Orca repo itself — no repoPath validation needed - ipcMain.handle('gh:viewer', () => getAuthenticatedViewer()) - ipcMain.handle('gh:checkOrcaStarred', () => checkOrcaStarred()) - ipcMain.handle('gh:starOrca', async (_event, source: unknown) => { - const sourceParse = appStarSourceSchema.safeParse(source) - const starred = await starOrca() - if (starred && sourceParse.success) { - // Why: this main-owned event bypasses renderer telemetry IPC, so cohort - // context must be attached here on the successful star path. - track('app_starred_orca', { - source: sourceParse.data, - ...getCohortAtEmit() - }) - } - return starred - }) - - // Why: `rate_limit` is exempt from GitHub's rate-limit accounting, so - // polling is cheap. A 30s in-process cache still avoids the gh subprocess - // cost on every render — see getRateLimit for the ttl rationale. Force - // parameter lets the renderer bust the cache after a known-expensive op - // (e.g. post-ProjectPicker discovery) without waiting out the ttl. - ipcMain.handle('gh:rateLimit', (_event, args?: { force?: boolean }) => - getRateLimit(args?.force ? { force: true } : undefined) - ) - - ipcMain.handle('gh:diagnoseAuth', (_event, args?: { host?: string }) => - diagnoseGhAuth(args?.host) - ) - - // ── GitHub ProjectV2 view handlers ───────────────────────────────── - // Why: registered unconditionally so enabling the experimental flag at - // runtime takes effect without a restart. The renderer gates entry points. - // Handlers never throw across IPC — every failure mode resolves through the - // GitHubProjectViewError envelope. - - ipcMain.handle('gh:listAccessibleProjects', (_event, args?: ListAccessibleProjectsArgs) => - listAccessibleProjects(args) - ) - - ipcMain.handle('gh:resolveProjectRef', (_event, args: ResolveProjectRefArgs) => - resolveProjectRef(args) - ) - - ipcMain.handle('gh:listProjectViews', (_event, args: ListProjectViewsArgs) => - listProjectViews(args) - ) - - ipcMain.handle('gh:getProjectViewTable', (_event, args: GetProjectViewTableArgs) => - getProjectViewTable(args) - ) - - ipcMain.handle( - 'gh:projectWorkItemDetailsBySlug', - (_event, args: ProjectWorkItemDetailsBySlugArgs) => getWorkItemDetailsBySlug(args) - ) - - ipcMain.handle('gh:updateProjectItemField', (_event, args: UpdateProjectItemFieldArgs) => - updateProjectItemFieldValue(args) - ) - - ipcMain.handle('gh:clearProjectItemField', (_event, args: ClearProjectItemFieldArgs) => - clearProjectItemFieldValue(args) - ) - - ipcMain.handle('gh:updateIssueBySlug', (_event, args: UpdateIssueBySlugArgs) => - updateIssueBySlug(args) - ) - - ipcMain.handle('gh:updatePullRequestBySlug', (_event, args: UpdatePullRequestBySlugArgs) => - updatePullRequestBySlug(args) - ) - - ipcMain.handle('gh:addIssueCommentBySlug', (_event, args: AddIssueCommentBySlugArgs) => - addIssueCommentBySlug(args) - ) - - ipcMain.handle('gh:updateIssueCommentBySlug', (_event, args: UpdateIssueCommentBySlugArgs) => - updateIssueCommentBySlug(args) - ) - - ipcMain.handle('gh:deleteIssueCommentBySlug', (_event, args: DeleteIssueCommentBySlugArgs) => - deleteIssueCommentBySlug(args) - ) - - ipcMain.handle('gh:listLabelsBySlug', (_event, args: ListLabelsBySlugArgs) => - listLabelsBySlug(args) - ) - - ipcMain.handle('gh:listAssignableUsersBySlug', (_event, args: ListAssignableUsersBySlugArgs) => - listAssignableUsersBySlug(args) - ) - - ipcMain.handle('gh:listIssueTypesBySlug', (_event, args: ListIssueTypesBySlugArgs) => - listIssueTypesBySlug(args) - ) - - ipcMain.handle('gh:updateIssueTypeBySlug', (_event, args: UpdateIssueTypeBySlugArgs) => - updateIssueTypeBySlug(args) - ) - - // Why: issue-source preference writes go through the generic `repos:update` - // IPC (extended in this PR to accept `issueSourcePreference`). Routing - // through the same channel keeps a single write path, guarantees the - // `repos:changed` broadcast is emitted, and avoids two channels racing to - // persist the same field with different validation and eviction semantics. - // Reads piggyback on the `Repo` record already delivered by `repos:list`. + registerGitHubPRRefreshHandlers(store, stats) + registerGitHubWorkItemHandlers(store) + registerGitHubPRReadHandlers(store) + registerGitHubPRReviewHandlers(store) + registerGitHubPRMutationHandlers(store) + registerGitHubIssueMutationHandlers(store) + registerGitHubAccountHandlers() + registerGitHubProjectViewHandlers() }