From ed7fe7d5dc76c079752dccc7991f12b0293de990 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Wed, 28 Oct 2026 17:46:20 -0700 Subject: [PATCH] Combine exact PR fields and stack summary queries (#35295) * Read sidebar PR and stack metadata in one GraphQL query * Preserve direct PR error recovery without duplicate GraphQL reads * Keep combined sidebar queries out of merge preflight --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- .../github/client-stack-merge-guard.test.ts | 56 ++- .../github/client/lookup/pr-number-lookup.ts | 25 +- .../lookup/pull-request-sidebar-query.ts | 262 +++++++++++ .../github/default-branch-stale-pr.test.ts | 82 ++-- .../github/pull-request-sidebar-query.test.ts | 418 ++++++++++++++++++ 5 files changed, 793 insertions(+), 50 deletions(-) create mode 100644 src/main/github/client/lookup/pull-request-sidebar-query.ts create mode 100644 src/main/github/pull-request-sidebar-query.test.ts diff --git a/src/main/github/client-stack-merge-guard.test.ts b/src/main/github/client-stack-merge-guard.test.ts index 99d60be51ad..7aaac723470 100644 --- a/src/main/github/client-stack-merge-guard.test.ts +++ b/src/main/github/client-stack-merge-guard.test.ts @@ -43,32 +43,41 @@ describe('GitHub GraphQL rate-limit guard', () => { ghExecFileAsyncMock .mockResolvedValueOnce({ stdout: JSON.stringify({ - number: 202, - title: 'Stack API', - state: 'OPEN', - url: 'https://github.com/stablyai/orca/pull/202', - statusCheckRollup: [], - updatedAt: '2026-08-10T00:00:00Z', - isDraft: false, - mergeable: 'MERGEABLE', - baseRefName: 'stack/models', - headRefName: 'stack/api', - headRefOid: 'api-sha' + data: { + repository: { + pullRequest: { + number: 202, + title: 'Stack API', + state: 'OPEN', + url: 'https://github.com/stablyai/orca/pull/202', + updatedAt: '2026-08-10T00:00:00Z', + isDraft: false, + mergeable: 'MERGEABLE', + reviewDecision: null, + mergeStateStatus: 'CLEAN', + autoMergeRequest: null, + baseRefName: 'stack/models', + headRefName: 'stack/api', + baseRefOid: 'b'.repeat(40), + headRefOid: 'a'.repeat(40), + headRepositoryOwner: { login: 'stablyai' }, + headRepository: { name: 'orca' }, + commits: { nodes: [{ commit: { oid: 'a'.repeat(40), statusCheckRollup: null } }] }, + stack: { number: 51, size: 2, baseRefName: 'main' }, + stackEntry: { position: 2 } + } + } + } }) }) .mockResolvedValueOnce({ stdout: JSON.stringify({ - number: 202, - title: 'Stack API', - state: 'open', - html_url: 'https://github.com/stablyai/orca/pull/202', - head: { ref: 'stack/api', sha: 'api-sha' }, - base: { ref: 'stack/models', sha: 'models-sha' }, - stack: { - number: 51, - position: 2, - size: 2, - base: { ref: 'main', sha: 'main-sha' } + data: { + repository: { + autoMergeAllowed: true, + mergeQueue: null, + ref: { rules: { nodes: [] } } + } } }) }) @@ -139,6 +148,9 @@ describe('GitHub GraphQL rate-limit guard', () => { ] }) expect(pr?.mergeQueueRequired).toBe(true) + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(4) + expect(ghExecFileAsyncMock.mock.calls[0][0]).toContain('number=202') + expect(ghExecFileAsyncMock.mock.calls[1][0]).toContain('branch=stack/models') const mergeQueueMetadataCall = ghExecFileAsyncMock.mock.calls.find( ([args]) => args.includes('graphql') && args.includes('branch=main') ) diff --git a/src/main/github/client/lookup/pr-number-lookup.ts b/src/main/github/client/lookup/pr-number-lookup.ts index 2a4034d462c..0d64d9c25a4 100644 --- a/src/main/github/client/lookup/pr-number-lookup.ts +++ b/src/main/github/client/lookup/pr-number-lookup.ts @@ -18,6 +18,7 @@ import { hydratePullRequestLookupData } from './pull-request-lookup-hydration' import { isGitObjectId, isUsableRestStackMetadata } from './rest-stack-metadata-validation' import { revalidateGitHubRestJson } from '../../github-rest-revalidation' import { noteRepositoryRateLimitSpend } from '../../rate-limit' +import { readPullRequestSidebarQuery } from './pull-request-sidebar-query' export async function getRestPRByNumber( ownerRepo: GitHubApiRepository, number: number, @@ -56,9 +57,27 @@ export async function getPRByNumber( number: number, ghOptions: ReturnType, executionScope: string, - knownPullRequestData?: PullRequestLookupData | null + knownPullRequestData?: PullRequestLookupData | null, + options: { includeStackSummary?: boolean } = {} ): Promise { try { + const sidebarData = + options.includeStackSummary && knownPullRequestData === undefined + ? await readPullRequestSidebarQuery(ownerRepo, number, ghOptions) + : undefined + if (sidebarData === null) { + return null + } + if (sidebarData) { + const { stack, ...prData } = sidebarData + const hydrated = await hydratePullRequestLookupData( + ownerRepo, + prData, + ghOptions, + executionScope + ) + return { ...hydrated, ...(stack ? { stack } : {}) } + } noteRepositoryRateLimitSpend(ownerRepo, 'graphql', 1, ghOptions) const { stdout } = await ghExecFileAsync( [ @@ -124,7 +143,9 @@ export async function lookupPRByNumber(args: { candidate, args.number, args.ghOptions, - args.executionScope + args.executionScope, + undefined, + { includeStackSummary: true } ) if (!linkedData || (args.policy && !(await args.policy.accepts(linkedData, candidate)))) { continue diff --git a/src/main/github/client/lookup/pull-request-sidebar-query.ts b/src/main/github/client/lookup/pull-request-sidebar-query.ts new file mode 100644 index 00000000000..e0e9bdb2821 --- /dev/null +++ b/src/main/github/client/lookup/pull-request-sidebar-query.ts @@ -0,0 +1,262 @@ +import { z } from 'zod' +import { ghExecFileAsync } from '../../gh-utils' +import { extractExecError } from '../../../git/exec-error' +import { githubHostExecOptions, type GitHubApiRepository } from '../../github-api-repository' +import { noteRepositoryRateLimitSpend } from '../../rate-limit' +import type { GhExecOptions } from '../github-exec-scope' +import type { PullRequestLookupData } from './pull-request-lookup-data' + +const CHECKS_FIELDS = `commits(last: 1) { + nodes { + commit { + oid + statusCheckRollup { + contexts(first: 100, after: $checksCursor) { + nodes { + __typename + ... on CheckRun { status conclusion } + ... on StatusContext { state } + } + pageInfo { hasNextPage endCursor } + } + } + } + } +}` + +const SIDEBAR_QUERY = `query($owner: String!, $repo: String!, $number: Int!, $checksCursor: String) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $number) { + number title state url updatedAt isDraft mergeable reviewDecision mergeStateStatus + autoMergeRequest { enabledAt } + baseRefName headRefName baseRefOid headRefOid + headRepositoryOwner { login } + headRepository { name } + ${CHECKS_FIELDS} + stack { number size baseRefName } + stackEntry { position } + } + } +}` + +const CHECKS_QUERY = `query($owner: String!, $repo: String!, $number: Int!, $checksCursor: String) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $number) { number ${CHECKS_FIELDS} } + } +}` + +const commitsSchema = z.object({ + nodes: z + .array( + z.object({ + commit: z.object({ + oid: z.string(), + statusCheckRollup: z + .object({ + contexts: z.object({ + nodes: z.array(z.record(z.string(), z.unknown()).nullable()), + pageInfo: z.object({ hasNextPage: z.boolean(), endCursor: z.string().nullable() }) + }) + }) + .nullable() + }) + }) + ) + .max(1) +}) +const errorsSchema = z + .array( + z.object({ + type: z.string().optional(), + path: z.array(z.union([z.string(), z.number()])).optional() + }) + ) + .optional() + +const pullRequestSchema = z.object({ + number: z.number().int().positive(), + title: z.string(), + state: z.enum(['OPEN', 'CLOSED', 'MERGED']), + url: z.string(), + updatedAt: z.string(), + isDraft: z.boolean(), + mergeable: z.enum(['UNKNOWN', 'MERGEABLE', 'CONFLICTING']), + reviewDecision: z.enum(['APPROVED', 'CHANGES_REQUESTED', 'REVIEW_REQUIRED']).nullable(), + mergeStateStatus: z.string().nullable(), + autoMergeRequest: z.object({ enabledAt: z.string() }).nullable(), + baseRefName: z.string(), + headRefName: z.string(), + baseRefOid: z.string().nullable(), + headRefOid: z.string().nullable(), + headRepositoryOwner: z.object({ login: z.string() }).nullable(), + headRepository: z.object({ name: z.string() }).nullable(), + commits: commitsSchema, + stack: z.unknown().optional(), + stackEntry: z.unknown().optional() +}) +const stackSchema = z.object({ + number: z.number().int().positive(), + size: z.number().int().positive(), + baseRefName: z.string().trim().min(1) +}) +const stackEntrySchema = z.object({ position: z.number().int().positive() }) +const envelopeSchema = z.object({ + data: z.object({ repository: z.object({ pullRequest: pullRequestSchema }) }), + errors: errorsSchema +}) +const missingEnvelopeSchema = z.object({ + data: z.object({ repository: z.object({ pullRequest: z.null() }) }), + errors: errorsSchema +}) +const checksEnvelopeSchema = z.object({ + data: z.object({ + repository: z.object({ + pullRequest: z.object({ number: z.number(), commits: commitsSchema }) + }) + }), + errors: errorsSchema +}) + +async function readQuery( + repository: GitHubApiRepository, + number: number, + options: GhExecOptions, + query: string, + cursor?: string +): Promise<{ payload: unknown; failure?: { error: unknown } }> { + noteRepositoryRateLimitSpend(repository, 'graphql', 1, options) + const args = [ + 'api', + 'graphql', + '-f', + `query=${query}`, + '-f', + `owner=${repository.owner}`, + '-f', + `repo=${repository.repo}`, + '-F', + `number=${number}` + ] + if (cursor) { + args.push('-f', `checksCursor=${cursor}`) + } + let stdout: string + let failure: { error: unknown } | undefined + try { + stdout = (await ghExecFileAsync(args, { ...options, ...githubHostExecOptions(repository) })) + .stdout + } catch (error) { + failure = { error } + stdout = extractExecError(error).stdout + } + try { + return { payload: JSON.parse(stdout), failure } + } catch { + return { payload: undefined, failure } + } +} + +export async function readPullRequestSidebarQuery( + repository: GitHubApiRepository, + number: number, + options: GhExecOptions +): Promise { + if (repository.host?.toLowerCase() !== 'github.com') { + return undefined + } + const response = await readQuery(repository, number, options, SIDEBAR_QUERY) + const missing = missingEnvelopeSchema.safeParse(response.payload) + if ( + missing.success && + missing.data.errors?.length && + missing.data.errors.every( + ({ type, path }) => + type === 'NOT_FOUND' && + path?.length === 2 && + path[0] === 'repository' && + path[1] === 'pullRequest' + ) + ) { + return null + } + const parsed = envelopeSchema.safeParse(response.payload) + if (!parsed.success) { + if (response.failure) { + throw response.failure.error + } + return undefined + } + const envelope = parsed.data + const stackErrors = envelope.errors ?? [] + if ( + stackErrors.some( + ({ path }) => + path?.[0] !== 'repository' || + path[1] !== 'pullRequest' || + (path[2] !== 'stack' && path[2] !== 'stackEntry') + ) + ) { + if (response.failure) { + throw response.failure.error + } + return undefined + } + const pr = envelope.data.repository.pullRequest + if (pr.number !== number) { + return undefined + } + let contexts = pr.commits.nodes[0]?.commit.statusCheckRollup?.contexts + const statusCheckRollup = contexts?.nodes.flatMap((node) => (node ? [node] : [])) ?? [] + const cursors = new Set() + while (contexts?.pageInfo.hasNextPage) { + const cursor = contexts.pageInfo.endCursor + if (!cursor || cursors.has(cursor)) { + return undefined + } + cursors.add(cursor) + const page = await readQuery(repository, number, options, CHECKS_QUERY, cursor) + if (page.failure) { + throw page.failure.error + } + const parsedPage = checksEnvelopeSchema.safeParse(page.payload) + if (!parsedPage.success) { + return undefined + } + const next = parsedPage.data + if ( + next.errors?.length || + next.data.repository.pullRequest.number !== number || + next.data.repository.pullRequest.commits.nodes[0]?.commit.oid !== + pr.commits.nodes[0]?.commit.oid + ) { + return undefined + } + contexts = next.data.repository.pullRequest.commits.nodes[0]?.commit.statusCheckRollup?.contexts + if (!contexts) { + return undefined + } + statusCheckRollup.push(...contexts.nodes.flatMap((node) => (node ? [node] : []))) + } + const stack = stackSchema.safeParse(pr.stack) + const entry = stackEntrySchema.safeParse(pr.stackEntry) + const validStack = stack.success && entry.success && entry.data.position <= stack.data.size + const stackMetadataChecked = + stackErrors.length === 0 && ((pr.stack === null && pr.stackEntry === null) || validStack) + const { commits: _commits, stackEntry: _stackEntry, ...metadata } = pr + return { + ...metadata, + baseRefOid: pr.baseRefOid ?? undefined, + headRefOid: pr.headRefOid ?? undefined, + statusCheckRollup, + stackMetadataChecked, + stack: + stackMetadataChecked && validStack + ? { + number: stack.data.number, + size: stack.data.size, + position: entry.data.position, + baseRefName: stack.data.baseRefName + } + : undefined + } +} diff --git a/src/main/github/default-branch-stale-pr.test.ts b/src/main/github/default-branch-stale-pr.test.ts index b6021c8b931..c43a8f8562b 100644 --- a/src/main/github/default-branch-stale-pr.test.ts +++ b/src/main/github/default-branch-stale-pr.test.ts @@ -282,21 +282,36 @@ describe('issue #9171: default-branch checkout must not attach a stale non-open if (args[0] === 'api' && args[1]?.includes('pulls?head=')) { return { stdout: 'HTTP/2.0 200 OK\n\n[]' } } - if (args[0] === 'pr' && args[1] === 'view' && args[2] === '7') { + if (args[0] === 'api' && args[1] === 'graphql' && args.includes('number=7')) { return { stdout: JSON.stringify({ - number: 7, - title: 'Historical PR', - state: 'CLOSED', - url: 'https://github.com/acme/widgets/pull/7', - statusCheckRollup: [], - updatedAt: '2024-01-01T00:00:00Z', - isDraft: false, - mergeable: 'UNKNOWN', - baseRefName: 'old-release', - headRefName: 'master', - baseRefOid: 'base-oid', - headRefOid: 'stale-master-oid' + data: { + repository: { + pullRequest: { + number: 7, + title: 'Historical PR', + state: 'CLOSED', + url: 'https://github.com/acme/widgets/pull/7', + reviewDecision: null, + mergeStateStatus: 'UNKNOWN', + autoMergeRequest: null, + headRepositoryOwner: { login: 'acme' }, + headRepository: { name: 'widgets' }, + commits: { + nodes: [{ commit: { oid: 'stale-master-oid', statusCheckRollup: null } }] + }, + stack: null, + stackEntry: null, + updatedAt: '2024-01-01T00:00:00Z', + isDraft: false, + mergeable: 'UNKNOWN', + baseRefName: 'old-release', + headRefName: 'master', + baseRefOid: 'base-oid', + headRefOid: 'stale-master-oid' + } + } + } }) } } @@ -314,21 +329,36 @@ describe('issue #9171: default-branch checkout must not attach a stale non-open if (args[0] === 'api' && args[1]?.includes('pulls?head=')) { return { stdout: 'HTTP/2.0 200 OK\n\n[]' } } - if (args[0] === 'pr' && args[1] === 'view' && args[2] === '7') { + if (args[0] === 'api' && args[1] === 'graphql' && args.includes('number=7')) { return { stdout: JSON.stringify({ - number: 7, - title: 'Historical PR', - state: 'MERGED', - url: 'https://github.com/acme/widgets/pull/7', - statusCheckRollup: [], - updatedAt: '2024-01-01T00:00:00Z', - isDraft: false, - mergeable: 'UNKNOWN', - baseRefName: 'old-release', - headRefName: 'master', - baseRefOid: 'base-oid', - headRefOid: 'stale-master-oid' + data: { + repository: { + pullRequest: { + number: 7, + title: 'Historical PR', + state: 'MERGED', + url: 'https://github.com/acme/widgets/pull/7', + reviewDecision: null, + mergeStateStatus: 'UNKNOWN', + autoMergeRequest: null, + headRepositoryOwner: { login: 'acme' }, + headRepository: { name: 'widgets' }, + commits: { + nodes: [{ commit: { oid: 'stale-master-oid', statusCheckRollup: null } }] + }, + stack: null, + stackEntry: null, + updatedAt: '2024-01-01T00:00:00Z', + isDraft: false, + mergeable: 'UNKNOWN', + baseRefName: 'old-release', + headRefName: 'master', + baseRefOid: 'base-oid', + headRefOid: 'stale-master-oid' + } + } + } }) } } diff --git a/src/main/github/pull-request-sidebar-query.test.ts b/src/main/github/pull-request-sidebar-query.test.ts new file mode 100644 index 00000000000..194b649c4d5 --- /dev/null +++ b/src/main/github/pull-request-sidebar-query.test.ts @@ -0,0 +1,418 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { exec, spend } = vi.hoisted(() => ({ exec: vi.fn(), spend: vi.fn() })) +vi.mock('../git/runner', () => ({ ghExecFileAsync: exec, gitExecFileAsync: vi.fn() })) +vi.mock('./rate-limit', () => ({ + noteRepositoryRateLimitSpend: spend, + repositoryRateLimitGuard: () => ({ blocked: false }) +})) + +import { getPRByNumber, lookupPRByNumber } from './client/lookup/pr-number-lookup' +import { derivePRRefreshData } from './client/lookup/branch-lookup-derived-data' +import { _resetPRStackSummaryCacheForTests } from './client/lookup/pr-stack-summary-cache' +import { _resetMergeQueueCacheForTests } from './client/detect/repository-merge-metadata-cache' +import { readPullRequestSidebarQuery } from './client/lookup/pull-request-sidebar-query' +import { deriveCheckStatus } from './mappers' + +const repository = { owner: 'acme', repo: 'widgets', host: 'github.com' } +const options = { cwd: '/folder-workspace' } +const successCheck = { __typename: 'StatusContext', state: 'SUCCESS' } + +function commits(checks: unknown[] = [successCheck], endCursor?: string, oid = 'b'.repeat(40)) { + return { + nodes: [ + { + commit: { + oid, + statusCheckRollup: { + contexts: { + nodes: checks, + pageInfo: { hasNextPage: Boolean(endCursor), endCursor: endCursor ?? null } + } + } + } + } + ] + } +} +const fields = { + number: 42, + title: 'Keep sidebar metadata together', + state: 'CLOSED', + url: 'https://github.com/acme/widgets/pull/42', + updatedAt: '2026-10-28T00:00:00Z', + isDraft: false, + mergeable: 'UNKNOWN', + reviewDecision: null, + mergeStateStatus: 'UNKNOWN', + autoMergeRequest: null, + baseRefName: 'main', + headRefName: 'feature/sidebar', + baseRefOid: 'a'.repeat(40), + headRefOid: 'b'.repeat(40), + headRepositoryOwner: { login: 'contributor' }, + headRepository: { name: 'widgets' }, + commits: commits(), + stack: null, + stackEntry: null +} + +function response(pr: Record = fields, errors?: unknown[]) { + return { stdout: JSON.stringify({ data: { repository: { pullRequest: pr } }, errors }) } +} + +function restResponse() { + return { + stdout: `HTTP/2.0 200 OK\r\nContent-Type: application/json\r\n\r\n${JSON.stringify({ + number: 42, + title: fields.title, + state: 'closed', + html_url: fields.url, + head: { sha: fields.headRefOid }, + stack: null + })}` + } +} + +function legacyResponse() { + const { commits: _commits, stack: _stack, stackEntry: _entry, ...metadata } = fields + return { stdout: JSON.stringify({ ...metadata, statusCheckRollup: [successCheck] }) } +} + +beforeEach(() => { + vi.resetAllMocks() + _resetPRStackSummaryCacheForTests() + _resetMergeQueueCacheForTests() +}) + +describe('sidebar PR query', () => { + it.each(['HTTP 401: Bad credentials', 'HTTP 429: secondary rate limit', 'ETIMEDOUT'])( + 'uses REST recovery directly after %s without another GraphQL attempt', + async (message) => { + exec.mockImplementation(async (args: string[]) => { + if (args.includes('graphql') || args[0] === 'pr') { + throw new Error(message) + } + return restResponse() + }) + await expect( + getPRByNumber(repository, 42, options, 'local:host', undefined, { + includeStackSummary: true + }) + ).resolves.toMatchObject({ + number: 42 + }) + expect(exec).toHaveBeenCalledTimes(2) + expect(exec.mock.calls[1][0]).toContain('repos/acme/widgets/pulls/42') + expect(spend.mock.calls.filter(([, bucket]) => bucket === 'graphql')).toHaveLength(1) + } + ) + + it.each([true, false])( + 'accepts a scoped NOT_FOUND response without another request (nonzero exit: %s)', + async (failed) => { + const missing = { + stdout: JSON.stringify({ + data: { repository: { pullRequest: null } }, + errors: [{ type: 'NOT_FOUND', path: ['repository', 'pullRequest'] }] + }) + } + if (failed) { + exec.mockRejectedValueOnce({ code: 1, ...missing }) + } else { + exec.mockResolvedValueOnce(missing) + } + await expect( + getPRByNumber(repository, 42, options, 'local:host', undefined, { + includeStackSummary: true + }) + ).resolves.toBeNull() + expect(exec).toHaveBeenCalledTimes(1) + expect(spend).toHaveBeenCalledTimes(1) + } + ) + + it('uses REST recovery directly if a continuation request fails', async () => { + exec + .mockResolvedValueOnce(response({ ...fields, commits: commits([successCheck], 'next') })) + .mockRejectedValueOnce(new Error('ETIMEDOUT')) + .mockResolvedValueOnce(restResponse()) + await expect( + getPRByNumber(repository, 42, options, 'local:host', undefined, { includeStackSummary: true }) + ).resolves.toMatchObject({ + number: 42 + }) + expect(exec).toHaveBeenCalledTimes(3) + expect(exec.mock.calls[2][0]).toContain('repos/acme/widgets/pulls/42') + }) + + it('keeps capability hydration on the PR base before attaching its stack summary', async () => { + exec + .mockResolvedValueOnce( + response({ + ...fields, + state: 'OPEN', + stack: { number: 12, size: 3, baseRefName: 'release' }, + stackEntry: { position: 2 } + }) + ) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + data: { + repository: { autoMergeAllowed: true, mergeQueue: null, ref: { rules: { nodes: [] } } } + } + }) + }) + await expect( + getPRByNumber(repository, 42, options, 'local:host', undefined, { includeStackSummary: true }) + ).resolves.toMatchObject({ + autoMergeAllowed: true, + stack: { baseRefName: 'release', position: 2 } + }) + expect(exec).toHaveBeenCalledTimes(2) + expect(exec.mock.calls[1][0]).toContain('branch=main') + expect(exec.mock.calls[1][0]).not.toContain('branch=release') + }) + + it('reads linked PR metadata and confirms no stack with one request through the refresh pipeline', async () => { + exec.mockImplementation(async (args: string[]) => { + if (args[0] === 'api' && args[1] === 'graphql') { + return response() + } + if (args[0] === 'pr' && args[1] === 'view') { + return legacyResponse() + } + if (args[0] === 'api' && args[1] === 'repos/acme/widgets/pulls/42') { + return restResponse() + } + throw new Error(`Unexpected request: ${args.join(' ')}`) + }) + + const { data } = await lookupPRByNumber({ + candidates: [repository], + number: 42, + ghOptions: options, + executionScope: 'local:host' + }) + if (!data) { + throw new Error('Expected linked PR') + } + const derived = await derivePRRefreshData({ + data, + dataRepo: repository, + repoPath: options.cwd, + localGitOptions: {}, + ghOptions: options, + executionScope: 'local:host', + usedExactNumberLookup: true + }) + + expect(exec).toHaveBeenCalledTimes(1) + expect(spend).toHaveBeenCalledTimes(1) + expect(data).toMatchObject({ + title: fields.title, + state: 'CLOSED', + headRefOid: fields.headRefOid, + reviewDecision: null, + autoMergeEnabled: false, + stackMetadataChecked: true, + statusCheckRollup: [{ __typename: 'StatusContext', state: 'SUCCESS' }] + }) + expect(derived).toMatchObject({ stack: undefined, mergeable: 'UNKNOWN' }) + }) + + it('does not cache mutable PR fields or delay a later refresh', async () => { + exec + .mockResolvedValueOnce(response()) + .mockResolvedValueOnce(response({ ...fields, title: 'Updated' })) + await getPRByNumber(repository, 42, options, 'local:host', undefined, { + includeStackSummary: true + }) + await expect( + getPRByNumber(repository, 42, options, 'local:host', undefined, { includeStackSummary: true }) + ).resolves.toMatchObject({ + title: 'Updated' + }) + expect(exec).toHaveBeenCalledTimes(2) + }) + + it('maps stack position and the stack base independently of the PR base', async () => { + exec.mockResolvedValueOnce( + response({ + ...fields, + stack: { number: 12, size: 3, baseRefName: 'release' }, + stackEntry: { position: 2 } + }) + ) + await expect( + getPRByNumber(repository, 42, options, 'local:host', undefined, { includeStackSummary: true }) + ).resolves.toMatchObject({ + baseRefName: 'main', + stackMetadataChecked: true, + stack: { number: 12, size: 3, position: 2, baseRefName: 'release' } + }) + expect(exec).toHaveBeenCalledTimes(1) + }) + + it('keeps a failure beyond the first 100 checks and reuses the first page', async () => { + const failingCheck = { __typename: 'CheckRun', status: 'COMPLETED', conclusion: 'FAILURE' } + exec + .mockResolvedValueOnce( + response({ + ...fields, + commits: commits( + Array.from({ length: 100 }, () => successCheck), + 'next' + ) + }) + ) + .mockResolvedValueOnce(response({ number: 42, commits: commits([null, failingCheck]) })) + const data = await getPRByNumber(repository, 42, options, 'local:host', undefined, { + includeStackSummary: true + }) + expect(data?.statusCheckRollup).toHaveLength(101) + expect(deriveCheckStatus(data?.statusCheckRollup)).toBe('failure') + expect(exec).toHaveBeenCalledTimes(2) + expect(exec.mock.calls[1][0]).toContain('checksCursor=next') + expect(exec.mock.calls[1][0].join(' ')).not.toContain('stackEntry') + }) + + it.each([ + { number: 42, commits: commits([], 'next') }, + { number: 42, commits: commits([], undefined, 'c'.repeat(40)) } + ])( + 'recovers through the rich CLI if checks pagination stops advancing or the commit changes', + async (nextPage) => { + exec + .mockResolvedValueOnce(response({ ...fields, commits: commits([successCheck], 'next') })) + .mockResolvedValueOnce(response(nextPage)) + .mockResolvedValueOnce(legacyResponse()) + const data = await getPRByNumber(repository, 42, options, 'local:host', undefined, { + includeStackSummary: true + }) + expect(deriveCheckStatus(data?.statusCheckRollup)).toBe('success') + expect(exec).toHaveBeenCalledTimes(3) + expect(exec.mock.calls[2][0].slice(0, 2)).toEqual(['pr', 'view']) + } + ) + + it('preserves known branch discovery metadata without adding another query', async () => { + exec.mockResolvedValueOnce(legacyResponse()) + const known = { + ...fields, + statusCheckRollup: [], + headDefaultBranchName: 'trunk', + stackMetadataChecked: true, + stack: undefined + } + const data = await getPRByNumber(repository, 42, options, 'local:host', known) + expect(data).toMatchObject({ headDefaultBranchName: 'trunk', stackMetadataChecked: true }) + expect(exec).toHaveBeenCalledTimes(1) + expect(exec.mock.calls[0][0].slice(0, 2)).toEqual(['pr', 'view']) + }) + + it.each([ + { stack: undefined, stackEntry: undefined }, + { stack: { number: 12, size: 1, baseRefName: 'release' }, stackEntry: { position: 2 } } + ])( + 'recovers missing or invalid stack metadata through the existing reader', + async (stackFields) => { + exec + .mockResolvedValueOnce(response({ ...fields, ...stackFields })) + .mockResolvedValueOnce(restResponse()) + const data = await getPRByNumber(repository, 42, options, 'local:host', undefined, { + includeStackSummary: true + }) + if (!data) { + throw new Error('Expected linked PR') + } + await derivePRRefreshData({ + data, + dataRepo: repository, + repoPath: options.cwd, + localGitOptions: {}, + ghOptions: options, + executionScope: 'local:host', + usedExactNumberLookup: true + }) + expect(data.stackMetadataChecked).toBe(true) + expect(exec).toHaveBeenCalledTimes(2) + expect(exec.mock.calls[1][0]).toContain('repos/acme/widgets/pulls/42') + } + ) + + it('retains usable metadata from a stack field error on a nonzero gh exit', async () => { + exec.mockRejectedValueOnce( + Object.assign( + new Error('GraphQL partial error'), + response(fields, [{ path: ['repository', 'pullRequest', 'stack'], message: 'Unavailable' }]) + ) + ) + await expect( + getPRByNumber(repository, 42, options, 'local:host', undefined, { includeStackSummary: true }) + ).resolves.toMatchObject({ + title: fields.title, + stackMetadataChecked: false + }) + expect(exec).toHaveBeenCalledTimes(1) + }) + + it.each([ + { data: { repository: { pullRequest: { ...fields, number: 99 } } } }, + { + data: { repository: { pullRequest: fields } }, + errors: [{ path: ['repository', 'pullRequest', 'headRefOid'] }] + }, + { data: { repository: { pullRequest: fields } }, errors: [{ message: 'Unscoped failure' }] } + ])('falls back to the rich CLI read for an unusable response', async (envelope) => { + exec.mockResolvedValueOnce({ stdout: JSON.stringify(envelope) }).mockResolvedValueOnce({ + stdout: JSON.stringify({ ...fields, title: 'CLI fallback', statusCheckRollup: [] }) + }) + await expect( + getPRByNumber(repository, 42, options, 'local:host', undefined, { includeStackSummary: true }) + ).resolves.toMatchObject({ + title: 'CLI fallback' + }) + expect(exec).toHaveBeenCalledTimes(2) + expect(exec.mock.calls[1][0].slice(0, 2)).toEqual(['pr', 'view']) + }) + + it.each(['github.corp.example', undefined])( + 'keeps the existing read for host %s', + async (host) => { + exec.mockResolvedValueOnce({ stdout: JSON.stringify({ ...fields, statusCheckRollup: [] }) }) + await expect( + getPRByNumber({ ...repository, host }, 42, options, 'local:host', undefined, { + includeStackSummary: true + }) + ).resolves.toMatchObject({ number: 42 }) + expect(exec).toHaveBeenCalledTimes(1) + expect(exec.mock.calls[0][0].slice(0, 2)).toEqual(['pr', 'view']) + } + ) + + it.each([ + { ghAccount: { host: 'github.com', user: 'reviewer' }, env: { GH_TOKEN: 'fixture' } }, + { wslDistro: 'Ubuntu' }, + {} + ])('retains the caller execution and account options: %j', async (execution) => { + exec.mockResolvedValueOnce( + response({ + ...fields, + headRepository: null, + headRepositoryOwner: null, + headRefOid: null, + commits: { nodes: [{ commit: { oid: fields.headRefOid, statusCheckRollup: null } }] } + }) + ) + await expect( + readPullRequestSidebarQuery(repository, 42, { ...options, ...execution }) + ).resolves.toMatchObject({ + headRefOid: undefined, + headRepository: null, + statusCheckRollup: [], + stackMetadataChecked: true + }) + expect(exec.mock.calls[0][1]).toEqual({ ...options, ...execution, host: 'github.com' }) + }) +})