diff --git a/apps/web/src/lib/integrations/platforms/github/adapter.ts b/apps/web/src/lib/integrations/platforms/github/adapter.ts index e016e584f1..87e1ae93f8 100644 --- a/apps/web/src/lib/integrations/platforms/github/adapter.ts +++ b/apps/web/src/lib/integrations/platforms/github/adapter.ts @@ -1,7 +1,7 @@ import { Octokit } from '@octokit/rest'; import { createAppAuth } from '@octokit/auth-app'; import { exchangeWebFlowCode } from '@octokit/oauth-methods'; -import { logExceptInTest } from '@/lib/utils.server'; +import { logExceptInTest, warnExceptInTest } from '@/lib/utils.server'; import crypto from 'crypto'; import type { InstallationToken } from '@/lib/integrations/core/types'; @@ -647,6 +647,146 @@ function isHttpError(error: unknown): error is { status: number; message: string ); } +export type AssociatedPullRequest = { + number: number; + htmlUrl: string; + state: 'open' | 'closed' | 'merged'; + title: string; + headSha: string; + updatedAt: string; // ISO +}; + +/** + * Thrown when GitHub returns a rate-limit response. The caller can surface + * `resetAt` to the user so they know when to retry. + */ +export class GitHubRateLimitError extends Error { + public readonly resetAt: Date; + constructor(resetAt: Date) { + super(`GitHub rate limited until ${resetAt.toISOString()}`); + this.name = 'GitHubRateLimitError'; + this.resetAt = resetAt; + } +} + +function getResponseHeader(error: unknown, name: string): string | undefined { + if (typeof error !== 'object' || error === null) return undefined; + const response = (error as { response?: { headers?: Record } }).response; + const headers = response?.headers; + if (!headers) return undefined; + const value = headers[name] ?? headers[name.toLowerCase()]; + return typeof value === 'string' ? value : undefined; +} + +function parseRateLimitResetAt(error: unknown): Date { + const resetHeader = getResponseHeader(error, 'x-ratelimit-reset'); + const resetSeconds = resetHeader ? Number(resetHeader) : NaN; + if (Number.isFinite(resetSeconds) && resetSeconds > 0) { + return new Date(resetSeconds * 1000); + } + // Fall back to "retry in 60s" if the header is missing/invalid, so callers + // always have a usable Date to show. + return new Date(Date.now() + 60_000); +} + +function getErrorMessage(error: unknown): string { + if (typeof error !== 'object' || error === null) return ''; + const message = (error as { message?: unknown }).message; + return typeof message === 'string' ? message : ''; +} + +function isRateLimitError(error: unknown): boolean { + if (!isHttpError(error)) return false; + // 429 is unambiguously rate limiting. + if (error.status === 429) return true; + // `x-ratelimit-remaining: 0` signals the primary rate limit is exhausted + // regardless of status. + const remaining = getResponseHeader(error, 'x-ratelimit-remaining'); + if (remaining === '0') return true; + // 403 is overloaded: it can mean rate/abuse limiting OR a plain permission + // denial (e.g. installation lacks pull request access). Only treat 403 as + // rate-limited when the message indicates so, so that genuine permission + // failures are surfaced to the caller. + if (error.status === 403) { + const message = getErrorMessage(error).toLowerCase(); + return ( + message.includes('rate limit') || + message.includes('secondary rate limit') || + message.includes('abuse') + ); + } + return false; +} + +/** + * Look up the pull request associated with a `(repo, branch)` pair using an + * installation token. Returns the most recently updated PR whose head ref + * matches `branch`, preferring `open` PRs when multiple exist. + * + * This helper is only invoked from the manual "Refresh PR info" mutation; the + * webhook path updates the DB directly. Intentionally no caching or dedup — + * the mutation is throttled server-side to once per 10s per session. + * + * @returns The associated PR, or `null` if no PR matches (or the repo is no + * longer accessible to this installation). + * @throws {GitHubRateLimitError} when GitHub rate-limits the request. + */ +export async function fetchPullRequestForBranch(params: { + installationId: number; + owner: string; + repo: string; + branch: string; + appType: GitHubAppType; +}): Promise { + const { installationId, owner, repo, branch, appType } = params; + + const tokenData = await generateGitHubInstallationToken(String(installationId), appType); + const octokit = new Octokit({ auth: tokenData.token }); + + try { + const { data: prs } = await octokit.pulls.list({ + owner, + repo, + head: `${owner}:${branch}`, + state: 'all', + per_page: 10, + sort: 'updated', + direction: 'desc', + }); + + if (prs.length === 0) { + return null; + } + + const chosen = prs.find(pr => pr.state === 'open') ?? prs[0]; + + const state: AssociatedPullRequest['state'] = + chosen.merged_at != null ? 'merged' : chosen.state === 'open' ? 'open' : 'closed'; + + return { + number: chosen.number, + htmlUrl: chosen.html_url, + state, + title: chosen.title, + headSha: chosen.head.sha, + updatedAt: chosen.updated_at, + }; + } catch (error) { + if (isRateLimitError(error)) { + throw new GitHubRateLimitError(parseRateLimitResetAt(error)); + } + if (isHttpError(error) && error.status === 404) { + warnExceptInTest('[fetchPullRequestForBranch] Repo not accessible or deleted', { + owner, + repo, + branch, + }); + return null; + } + throw error; + } +} + /** * Get repository details including whether it's empty. * Used to validate target repo before migration. diff --git a/apps/web/src/lib/integrations/platforms/github/fetch-pull-request-for-branch.test.ts b/apps/web/src/lib/integrations/platforms/github/fetch-pull-request-for-branch.test.ts new file mode 100644 index 0000000000..4fce84ca75 --- /dev/null +++ b/apps/web/src/lib/integrations/platforms/github/fetch-pull-request-for-branch.test.ts @@ -0,0 +1,271 @@ +/** + * Tests for `fetchPullRequestForBranch` in `adapter.ts`. + * + * The adapter module is globally mocked via the jest `moduleNameMapper` entry + * for `@/lib/integrations/platforms/github/adapter`. We bypass that by importing + * via a relative path, then mock `@octokit/rest` and the installation-token + * generation so we never hit real GitHub. + */ + +process.env.GITHUB_APP_ID = 'test-app-id'; +process.env.GITHUB_APP_PRIVATE_KEY = 'test-private-key'; +process.env.GITHUB_LITE_APP_ID = 'test-lite-app-id'; +process.env.GITHUB_LITE_APP_PRIVATE_KEY = 'test-lite-private-key'; + +const mockPullsList = jest.fn(); + +jest.mock('@octokit/rest', () => ({ + Octokit: jest.fn().mockImplementation(() => ({ + pulls: { list: mockPullsList }, + })), +})); + +jest.mock('@octokit/auth-app', () => ({ + createAppAuth: jest.fn(() => async () => ({ token: 'mock-token', expiresAt: '2099-01-01' })), +})); + +import { fetchPullRequestForBranch, GitHubRateLimitError } from './adapter'; + +type ListResponse = { + data: Array<{ + number: number; + html_url: string; + state: 'open' | 'closed'; + title: string; + updated_at: string; + merged_at: string | null; + head: { sha: string }; + }>; +}; + +function okResponse(data: ListResponse['data']): ListResponse { + return { data }; +} + +function httpError( + status: number, + headers: Record = {}, + message?: string +) { + return Object.assign(new Error(message ?? `HTTP ${status}`), { + status, + response: { headers }, + }); +} + +beforeEach(() => { + mockPullsList.mockReset(); +}); + +describe('fetchPullRequestForBranch', () => { + const params = { + installationId: 42, + owner: 'acme', + repo: 'widgets', + branch: 'feature/login', + appType: 'standard' as const, + }; + + it('returns the single open PR when the API returns one result', async () => { + mockPullsList.mockResolvedValueOnce( + okResponse([ + { + number: 7, + html_url: 'https://github.com/acme/widgets/pull/7', + state: 'open', + title: 'Add login page', + updated_at: '2026-04-20T10:00:00Z', + merged_at: null, + head: { sha: 'abc123' }, + }, + ]) + ); + + const result = await fetchPullRequestForBranch(params); + + expect(result).toEqual({ + number: 7, + htmlUrl: 'https://github.com/acme/widgets/pull/7', + state: 'open', + title: 'Add login page', + headSha: 'abc123', + updatedAt: '2026-04-20T10:00:00Z', + }); + + expect(mockPullsList).toHaveBeenCalledWith({ + owner: 'acme', + repo: 'widgets', + head: 'acme:feature/login', + state: 'all', + per_page: 10, + sort: 'updated', + direction: 'desc', + }); + }); + + it('prefers an open PR over a more-recently-updated closed one', async () => { + mockPullsList.mockResolvedValueOnce( + okResponse([ + { + number: 9, + html_url: 'https://github.com/acme/widgets/pull/9', + state: 'closed', + title: 'Old attempt', + updated_at: '2026-04-22T10:00:00Z', + merged_at: null, + head: { sha: 'closedsha' }, + }, + { + number: 7, + html_url: 'https://github.com/acme/widgets/pull/7', + state: 'open', + title: 'Current attempt', + updated_at: '2026-04-20T10:00:00Z', + merged_at: null, + head: { sha: 'opensha' }, + }, + ]) + ); + + const result = await fetchPullRequestForBranch(params); + + expect(result).toMatchObject({ number: 7, state: 'open', headSha: 'opensha' }); + }); + + it('returns merged state when merged_at is set', async () => { + mockPullsList.mockResolvedValueOnce( + okResponse([ + { + number: 11, + html_url: 'https://github.com/acme/widgets/pull/11', + state: 'closed', + title: 'Merged work', + updated_at: '2026-04-22T10:00:00Z', + merged_at: '2026-04-22T09:59:00Z', + head: { sha: 'mergedsha' }, + }, + ]) + ); + + const result = await fetchPullRequestForBranch(params); + + expect(result?.state).toBe('merged'); + }); + + it('falls back to the first result when no open PR exists', async () => { + mockPullsList.mockResolvedValueOnce( + okResponse([ + { + number: 2, + html_url: 'https://github.com/acme/widgets/pull/2', + state: 'closed', + title: 'Later closed', + updated_at: '2026-04-22T10:00:00Z', + merged_at: null, + head: { sha: 'sha-2' }, + }, + { + number: 1, + html_url: 'https://github.com/acme/widgets/pull/1', + state: 'closed', + title: 'Earlier closed', + updated_at: '2026-04-20T10:00:00Z', + merged_at: null, + head: { sha: 'sha-1' }, + }, + ]) + ); + + const result = await fetchPullRequestForBranch(params); + + expect(result?.number).toBe(2); + expect(result?.state).toBe('closed'); + }); + + it('returns null when no PRs match', async () => { + mockPullsList.mockResolvedValueOnce(okResponse([])); + + const result = await fetchPullRequestForBranch(params); + + expect(result).toBeNull(); + }); + + it('returns null on 404 (repo no longer accessible)', async () => { + mockPullsList.mockRejectedValueOnce(httpError(404)); + + const result = await fetchPullRequestForBranch(params); + + expect(result).toBeNull(); + }); + + it('throws GitHubRateLimitError on 403 with rate-limit reset header', async () => { + const resetEpochSeconds = 1_800_000_000; + mockPullsList.mockRejectedValueOnce( + httpError(403, { + 'x-ratelimit-remaining': '0', + 'x-ratelimit-reset': String(resetEpochSeconds), + }) + ); + + await expect(fetchPullRequestForBranch(params)).rejects.toMatchObject({ + name: 'GitHubRateLimitError', + resetAt: new Date(resetEpochSeconds * 1000), + }); + }); + + it('throws GitHubRateLimitError on 429', async () => { + const resetEpochSeconds = 1_800_000_100; + mockPullsList.mockRejectedValueOnce( + httpError(429, { 'x-ratelimit-reset': String(resetEpochSeconds) }) + ); + + const error = await fetchPullRequestForBranch(params).catch(e => e); + expect(error).toBeInstanceOf(GitHubRateLimitError); + if (error instanceof GitHubRateLimitError) { + expect(error.resetAt).toEqual(new Date(resetEpochSeconds * 1000)); + } + }); + + it('throws GitHubRateLimitError when x-ratelimit-remaining is 0 even without 403/429', async () => { + const resetEpochSeconds = 1_800_000_200; + // Status 200 is nonsensical with this error shape, but the helper should + // still recognise the rate-limit signal from the header. + mockPullsList.mockRejectedValueOnce( + httpError(500, { + 'x-ratelimit-remaining': '0', + 'x-ratelimit-reset': String(resetEpochSeconds), + }) + ); + + await expect(fetchPullRequestForBranch(params)).rejects.toBeInstanceOf(GitHubRateLimitError); + }); + + it('throws GitHubRateLimitError on 403 with "secondary rate limit" message', async () => { + mockPullsList.mockRejectedValueOnce( + httpError(403, {}, 'You have exceeded a secondary rate limit.') + ); + + await expect(fetchPullRequestForBranch(params)).rejects.toBeInstanceOf(GitHubRateLimitError); + }); + + it('re-throws non-rate-limit 403 (permission denied) unchanged', async () => { + // GitHub returns 403 when an installation lacks the required permission + // (e.g. pull request read access). We must NOT wrap this as a rate limit, + // or callers will incorrectly tell users to retry. + const permissionError = httpError( + 403, + {}, + 'Resource not accessible by integration' + ); + mockPullsList.mockRejectedValueOnce(permissionError); + + await expect(fetchPullRequestForBranch(params)).rejects.toBe(permissionError); + }); + + it('re-throws unexpected errors unchanged', async () => { + const boom = new Error('network exploded'); + mockPullsList.mockRejectedValueOnce(boom); + + await expect(fetchPullRequestForBranch(params)).rejects.toBe(boom); + }); +}); diff --git a/apps/web/src/tests/setup/__mocks__/lib/integrations/platforms/github/adapter.ts b/apps/web/src/tests/setup/__mocks__/lib/integrations/platforms/github/adapter.ts index 1535a4eddb..6fc9ba576a 100644 --- a/apps/web/src/tests/setup/__mocks__/lib/integrations/platforms/github/adapter.ts +++ b/apps/web/src/tests/setup/__mocks__/lib/integrations/platforms/github/adapter.ts @@ -68,3 +68,31 @@ export async function updateCheckRun( ): Promise { return; } + +export type AssociatedPullRequest = { + number: number; + htmlUrl: string; + state: 'open' | 'closed' | 'merged'; + title: string; + headSha: string; + updatedAt: string; +}; + +export class GitHubRateLimitError extends Error { + public readonly resetAt: Date; + constructor(resetAt: Date) { + super(`GitHub rate limited until ${resetAt.toISOString()}`); + this.name = 'GitHubRateLimitError'; + this.resetAt = resetAt; + } +} + +export async function fetchPullRequestForBranch(_params: { + installationId: number; + owner: string; + repo: string; + branch: string; + appType: GitHubAppType; +}): Promise { + return null; +}