diff --git a/src/main/github/github-stack-api-responses.ts b/src/main/github/github-stack-api-responses.ts new file mode 100644 index 00000000000..727d72331ef --- /dev/null +++ b/src/main/github/github-stack-api-responses.ts @@ -0,0 +1,52 @@ +import type { HostedReviewSummary } from '../../shared/hosted-review' + +export type NumberedHostedReviewSummary = Omit & { number: number } + +export type GitHubStackPullRequest = NumberedHostedReviewSummary & { + headRefName: string + baseRefName: string +} + +export type GitHubStack = { + number: number + open: boolean + pull_requests: { number: number }[] +} + +export function parseGitHubStackPullRequests(stdout: string): GitHubStackPullRequest[] { + const pullRequests = JSON.parse(stdout) as { + number?: unknown + html_url?: unknown + head?: { ref?: unknown } + base?: { ref?: unknown } + }[] + return pullRequests.flatMap((pullRequest) => { + const number = Number(pullRequest.number) + const url = typeof pullRequest.html_url === 'string' ? pullRequest.html_url : '' + const headRefName = typeof pullRequest.head?.ref === 'string' ? pullRequest.head.ref : '' + const baseRefName = typeof pullRequest.base?.ref === 'string' ? pullRequest.base.ref : '' + return Number.isInteger(number) && number > 0 && url && headRefName && baseRefName + ? [{ number, url, headRefName, baseRefName }] + : [] + }) +} + +export function parseGitHubStacks(stdout: string): GitHubStack[] { + const stacks = JSON.parse(stdout) as { + number?: unknown + open?: unknown + pull_requests?: { number?: unknown }[] + }[] + return stacks.flatMap((stack) => { + const number = Number(stack.number) + const pullRequests = (stack.pull_requests ?? []).flatMap((pullRequest) => { + const pullRequestNumber = Number(pullRequest.number) + return Number.isInteger(pullRequestNumber) && pullRequestNumber > 0 + ? [{ number: pullRequestNumber }] + : [] + }) + return Number.isInteger(number) && number > 0 + ? [{ number, open: stack.open === true, pull_requests: pullRequests }] + : [] + }) +} diff --git a/src/main/github/stacked-pr-creation.test.ts b/src/main/github/stacked-pr-creation.test.ts new file mode 100644 index 00000000000..e97ea14f2a4 --- /dev/null +++ b/src/main/github/stacked-pr-creation.test.ts @@ -0,0 +1,273 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { ghExecFileAsyncMock, repositoryMock } = vi.hoisted(() => ({ + ghExecFileAsyncMock: vi.fn(), + repositoryMock: vi.fn() +})) + +vi.mock('./gh-utils', () => ({ + acquire: vi.fn(), + release: vi.fn(), + ghExecFileAsync: ghExecFileAsyncMock, + ghRepoExecOptions: (context: { repoPath: string; connectionId?: string | null }) => + context.connectionId ? {} : { cwd: context.repoPath }, + githubRepoContext: ( + repoPath: string, + connectionId?: string | null, + localGitOptions?: Record + ) => ({ repoPath, connectionId, localGitOptions }) +})) + +vi.mock('./github-api-repository', () => ({ + getOriginGitHubApiRepository: repositoryMock, + githubHostExecOptions: (repository: { host?: string }) => ({ host: repository.host }) +})) + +import { + prepareGitHubStackedPullRequest, + registerGitHubStackedPullRequest +} from './stacked-pr-creation' + +const repository = { owner: 'acme', repo: 'orca', host: 'github.com' } +const parentReview = { number: 41, url: 'https://github.com/acme/orca/pull/41' } +const currentReview = { number: 42, url: 'https://github.com/acme/orca/pull/42' } + +function pullRequest(number: number, head: string, base: string) { + return { + number, + html_url: `https://github.com/acme/orca/pull/${number}`, + head: { ref: head }, + base: { ref: base } + } +} + +function stack(number: number, pullRequests: number[]) { + return { + number, + open: true, + pull_requests: pullRequests.map((pullRequestNumber) => ({ number: pullRequestNumber })) + } +} + +beforeEach(() => { + ghExecFileAsyncMock.mockReset() + repositoryMock.mockReset() + repositoryMock.mockResolvedValue(repository) +}) + +describe('prepareGitHubStackedPullRequest', () => { + it('resolves an open parent PR and an existing current PR', async () => { + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: JSON.stringify([pullRequest(41, 'stack/parent', 'main')]) }) + .mockResolvedValueOnce({ + stdout: JSON.stringify([pullRequest(42, 'stack/child', 'stack/parent')]) + }) + .mockResolvedValueOnce({ stdout: JSON.stringify([stack(50, [40, 41])]) }) + .mockResolvedValueOnce({ stdout: '[]' }) + + const result = await prepareGitHubStackedPullRequest('/repo', { + provider: 'github', + base: 'origin/stack/parent', + head: 'refs/heads/stack/child', + title: 'Child' + }) + + expect(result).toMatchObject({ + ok: true, + parentReview: { number: 41 }, + currentReview: { number: 42 } + }) + expect(ghExecFileAsyncMock.mock.calls[0][0]).toEqual([ + 'api', + 'repos/acme/orca/pulls?head=acme%3Astack%2Fparent&state=open&per_page=2' + ]) + expect(ghExecFileAsyncMock.mock.calls[1][0]).toEqual([ + 'api', + 'repos/acme/orca/pulls?head=acme%3Astack%2Fchild&base=stack%2Fparent&state=open&per_page=2' + ]) + }) + + it('allows an idempotent retry after the child was already registered', async () => { + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: JSON.stringify([pullRequest(41, 'stack/parent', 'main')]) }) + .mockResolvedValueOnce({ + stdout: JSON.stringify([pullRequest(42, 'stack/child', 'stack/parent')]) + }) + .mockResolvedValueOnce({ stdout: JSON.stringify([stack(50, [41, 42])]) }) + .mockResolvedValueOnce({ stdout: JSON.stringify([stack(50, [41, 42])]) }) + + const result = await prepareGitHubStackedPullRequest('/repo', { + provider: 'github', + base: 'stack/parent', + head: 'stack/child', + title: 'Child' + }) + + expect(result).toMatchObject({ ok: true, currentReview: { number: 42 } }) + }) + + it('requires an open PR for the selected parent branch', async () => { + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: '[]' }) + .mockResolvedValueOnce({ stdout: '[]' }) + + const result = await prepareGitHubStackedPullRequest('/repo', { + provider: 'github', + base: 'feature/parent', + head: 'feature/child', + title: 'Child' + }) + + expect(result).toMatchObject({ ok: false, code: 'validation' }) + if (!result.ok) { + expect(result.error).toContain('does not have an open pull request') + } + }) + + it('rejects a parent that is not the top of its stack', async () => { + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: JSON.stringify([pullRequest(41, 'stack/parent', 'main')]) }) + .mockResolvedValueOnce({ stdout: '[]' }) + .mockResolvedValueOnce({ stdout: JSON.stringify([stack(50, [41, 45])]) }) + + const result = await prepareGitHubStackedPullRequest('/repo', { + provider: 'github', + base: 'stack/parent', + head: 'stack/child', + title: 'Child' + }) + + expect(result).toMatchObject({ ok: false, code: 'validation' }) + if (!result.ok) { + expect(result.error).toContain('top pull request') + } + }) + + it('does not offer stacks on GitHub Enterprise Server', async () => { + repositoryMock.mockResolvedValue({ + owner: 'acme', + repo: 'orca', + host: 'github.acme.test' + }) + + const result = await prepareGitHubStackedPullRequest('/repo', { + provider: 'github', + base: 'stack/parent', + head: 'stack/child', + title: 'Child' + }) + + expect(result).toMatchObject({ ok: false, code: 'validation' }) + expect(ghExecFileAsyncMock).not.toHaveBeenCalled() + }) +}) + +describe('registerGitHubStackedPullRequest', () => { + it('creates a new stack with the parent and current PR', async () => { + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: '[]' }) + .mockResolvedValueOnce({ stdout: '[]' }) + .mockResolvedValueOnce({ stdout: JSON.stringify({ number: 50 }) }) + + const result = await registerGitHubStackedPullRequest({ + repoPath: '/repo', + repository, + parentReview, + currentReview + }) + + expect(result).toMatchObject({ ok: true, number: 42, stackNumber: 50 }) + expect(ghExecFileAsyncMock.mock.calls[2][0]).toEqual([ + 'api', + '-X', + 'POST', + 'repos/acme/orca/stacks', + '-F', + 'pull_requests[]=41', + '-F', + 'pull_requests[]=42' + ]) + }) + + it('appends the current PR when the parent is the existing top', async () => { + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: JSON.stringify([stack(50, [40, 41])]) }) + .mockResolvedValueOnce({ stdout: '[]' }) + .mockResolvedValueOnce({ stdout: JSON.stringify({ number: 50 }) }) + + const result = await registerGitHubStackedPullRequest({ + repoPath: '/repo', + repository, + parentReview, + currentReview, + connectionId: 'ssh-1' + }) + + expect(result).toMatchObject({ ok: true, stackNumber: 50 }) + expect(ghExecFileAsyncMock.mock.calls[2][0]).toEqual([ + 'api', + '-X', + 'POST', + 'repos/acme/orca/stacks/50/add', + '-F', + 'pull_requests[]=42' + ]) + expect(ghExecFileAsyncMock.mock.calls[2][1]).not.toHaveProperty('cwd') + }) + + it('treats an already registered parent-child pair as success', async () => { + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: JSON.stringify([stack(50, [41, 42])]) }) + .mockResolvedValueOnce({ stdout: JSON.stringify([stack(50, [41, 42])]) }) + + const result = await registerGitHubStackedPullRequest({ + repoPath: '/repo', + repository, + parentReview, + currentReview + }) + + expect(result).toMatchObject({ ok: true, stackNumber: 50 }) + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(2) + }) + + it('does not claim registration when the stack no longer holds the parent', async () => { + // A concurrent stack edit can drop the parent while the child sits at index 0. + // Reading index 0 off a findIndex miss would report that pair as registered. + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: JSON.stringify([stack(50, [42])]) }) + .mockResolvedValueOnce({ stdout: JSON.stringify([stack(50, [42])]) }) + + const result = await registerGitHubStackedPullRequest({ + repoPath: '/repo', + repository, + parentReview, + currentReview + }) + + expect(result).toMatchObject({ + ok: false, + error: 'The pull request already belongs to a different GitHub stack.', + createdReview: currentReview + }) + }) + + it('preserves the created PR when registration fails', async () => { + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: '[]' }) + .mockResolvedValueOnce({ stdout: '[]' }) + .mockRejectedValueOnce(new Error('HTTP 422')) + + const result = await registerGitHubStackedPullRequest({ + repoPath: '/repo', + repository, + parentReview, + currentReview + }) + + expect(result).toMatchObject({ + ok: false, + createdReview: currentReview + }) + }) +}) diff --git a/src/main/github/stacked-pr-creation.ts b/src/main/github/stacked-pr-creation.ts new file mode 100644 index 00000000000..357f78bcbc6 --- /dev/null +++ b/src/main/github/stacked-pr-creation.ts @@ -0,0 +1,310 @@ +import type { + CreateStackedHostedReviewInput, + CreateStackedHostedReviewResult +} from '../../shared/hosted-review' +import { isDefaultGitHubHost } from '../../shared/github-repository-identity-key' +import { + normalizeHostedReviewBaseRef, + normalizeHostedReviewHeadRef +} from '../../shared/hosted-review-refs' +import { acquire, ghExecFileAsync, ghRepoExecOptions, githubRepoContext, release } from './gh-utils' +import { + getOriginGitHubApiRepository, + githubHostExecOptions, + type GitHubApiRepository +} from './github-api-repository' +import { + getHostedReviewLocalGitOptions, + type HostedReviewExecutionOptions +} from '../source-control/hosted-review-git-options' +import { + parseGitHubStackPullRequests, + parseGitHubStacks, + type GitHubStack, + type GitHubStackPullRequest, + type NumberedHostedReviewSummary +} from './github-stack-api-responses' + +type StackedPullRequestPlan = + | { + ok: true + repository: GitHubApiRepository + parentReview: GitHubStackPullRequest + currentReview: GitHubStackPullRequest | null + } + | Extract + +function creationError(error: string): Extract { + return { ok: false, code: 'validation', error } +} + +function isStacksUnavailableError(error: unknown): boolean { + const message = error instanceof Error ? error.message.toLowerCase() : String(error).toLowerCase() + return message.includes('http 404') || message.includes('feature not supported') +} + +function ghOptions( + repoPath: string, + repository: GitHubApiRepository, + connectionId?: string | null, + options: HostedReviewExecutionOptions = {} +) { + return { + ...ghRepoExecOptions( + githubRepoContext(repoPath, connectionId, getHostedReviewLocalGitOptions(options)) + ), + ...githubHostExecOptions(repository), + timeout: 60_000 + } +} + +async function findOpenPullRequestsForBranch( + repoPath: string, + repository: GitHubApiRepository, + branch: string, + connectionId?: string | null, + options: HostedReviewExecutionOptions = {}, + base?: string +): Promise { + const head = encodeURIComponent(`${repository.owner}:${branch}`) + const baseQuery = base ? `&base=${encodeURIComponent(base)}` : '' + const endpoint = `repos/${repository.owner}/${repository.repo}/pulls?head=${head}${baseQuery}&state=open&per_page=2` + const { stdout } = await ghExecFileAsync( + ['api', endpoint], + ghOptions(repoPath, repository, connectionId, options) + ) + return parseGitHubStackPullRequests(stdout) +} + +async function getStacksForPullRequest( + repoPath: string, + repository: GitHubApiRepository, + pullRequestNumber: number, + connectionId?: string | null, + options: HostedReviewExecutionOptions = {} +): Promise { + const endpoint = `repos/${repository.owner}/${repository.repo}/stacks?pull_request=${pullRequestNumber}` + const { stdout } = await ghExecFileAsync( + ['api', endpoint], + ghOptions(repoPath, repository, connectionId, options) + ) + return parseGitHubStacks(stdout) +} + +function validateParentStack( + parentReview: NumberedHostedReviewSummary, + stacks: GitHubStack[] +): Extract | null { + if (stacks.length > 1) { + return creationError('The selected parent pull request belongs to multiple stacks.') + } + const stack = stacks[0] + if (!stack) { + return null + } + if (!stack.open) { + return creationError('The selected parent belongs to a closed stack.') + } + if (stack.pull_requests.at(-1)?.number !== parentReview.number) { + return creationError( + 'Choose the top pull request in the stack as the base branch before adding another layer.' + ) + } + return null +} + +export async function prepareGitHubStackedPullRequest( + repoPath: string, + input: CreateStackedHostedReviewInput, + connectionId?: string | null, + options: HostedReviewExecutionOptions = {} +): Promise { + if (input.provider !== 'github') { + return creationError('Stacked pull request creation is available only for GitHub repositories.') + } + const repository = await getOriginGitHubApiRepository( + repoPath, + connectionId, + getHostedReviewLocalGitOptions(options) + ) + if (!repository || !isDefaultGitHubHost(repository.host)) { + return creationError('GitHub stacked pull requests are available only on GitHub.com.') + } + const base = normalizeHostedReviewBaseRef(input.base).trim() + const head = input.head ? normalizeHostedReviewHeadRef(input.head).trim() : '' + if (!base || !head || base.toLowerCase() === head.toLowerCase()) { + return creationError('Choose a different parent branch before creating a stacked pull request.') + } + + await acquire() + try { + const [parentPullRequests, currentPullRequests] = await Promise.all([ + findOpenPullRequestsForBranch(repoPath, repository, base, connectionId, options), + findOpenPullRequestsForBranch(repoPath, repository, head, connectionId, options, base) + ]) + if (parentPullRequests.length === 0) { + return creationError( + `The parent branch ${base} does not have an open pull request. Create that pull request first.` + ) + } + if (parentPullRequests.length !== 1) { + return creationError(`Orca found multiple open pull requests for the parent branch ${base}.`) + } + if (currentPullRequests.length > 1) { + return creationError(`Orca found multiple open pull requests for the current branch ${head}.`) + } + const parentReview = parentPullRequests[0] + const currentReview = currentPullRequests[0] ?? null + const [parentStacks, currentStacks] = await Promise.all([ + getStacksForPullRequest(repoPath, repository, parentReview.number, connectionId, options), + currentReview + ? getStacksForPullRequest(repoPath, repository, currentReview.number, connectionId, options) + : Promise.resolve([]) + ]) + if ( + currentReview && + registeredStackNumber(parentReview, currentReview, parentStacks, currentStacks) + ) { + return { ok: true, repository, parentReview, currentReview } + } + if (currentStacks.length > 0) { + return creationError('The pull request already belongs to a different GitHub stack.') + } + const parentError = validateParentStack(parentReview, parentStacks) + return ( + parentError ?? { + ok: true, + repository, + parentReview, + currentReview + } + ) + } catch (error) { + console.warn('GitHub stack creation preflight failed:', error) + return { + ok: false, + code: isStacksUnavailableError(error) ? 'validation' : 'unknown', + error: isStacksUnavailableError(error) + ? 'GitHub stacked pull requests are not available for this repository.' + : 'Orca could not verify the parent pull request. Retry in a moment.' + } + } finally { + release() + } +} + +function registeredStackNumber( + parentReview: NumberedHostedReviewSummary, + currentReview: NumberedHostedReviewSummary, + parentStacks: GitHubStack[], + currentStacks: GitHubStack[] +): number | null { + const parentStack = parentStacks[0] + const currentStack = currentStacks[0] + if (!parentStack || !currentStack || parentStack.number !== currentStack.number) { + return null + } + const parentPosition = parentStack.pull_requests.findIndex( + (pullRequest) => pullRequest.number === parentReview.number + ) + // Why: a miss is -1, and -1 + 1 reads the first entry — which reports "already + // registered" whenever the current PR heads a stack the parent has left. + if (parentPosition < 0) { + return null + } + return parentStack.pull_requests[parentPosition + 1]?.number === currentReview.number + ? parentStack.number + : null +} + +export async function registerGitHubStackedPullRequest(args: { + repoPath: string + repository: GitHubApiRepository + parentReview: NumberedHostedReviewSummary + currentReview: NumberedHostedReviewSummary + connectionId?: string | null + options?: HostedReviewExecutionOptions +}): Promise { + const options = args.options ?? {} + await acquire() + try { + const [parentStacks, currentStacks] = await Promise.all([ + getStacksForPullRequest( + args.repoPath, + args.repository, + args.parentReview.number, + args.connectionId, + options + ), + getStacksForPullRequest( + args.repoPath, + args.repository, + args.currentReview.number, + args.connectionId, + options + ) + ]) + const existingStackNumber = registeredStackNumber( + args.parentReview, + args.currentReview, + parentStacks, + currentStacks + ) + if (existingStackNumber) { + return { + ok: true, + ...args.currentReview, + stackNumber: existingStackNumber, + parentReview: args.parentReview + } + } + if (currentStacks.length > 0) { + return { + ...creationError('The pull request already belongs to a different GitHub stack.'), + createdReview: args.currentReview + } + } + const parentError = validateParentStack(args.parentReview, parentStacks) + if (parentError) { + return { ...parentError, createdReview: args.currentReview } + } + + const parentStack = parentStacks[0] + const endpoint = parentStack + ? `repos/${args.repository.owner}/${args.repository.repo}/stacks/${parentStack.number}/add` + : `repos/${args.repository.owner}/${args.repository.repo}/stacks` + const pullRequests = parentStack + ? [args.currentReview.number] + : [args.parentReview.number, args.currentReview.number] + const command = ['api', '-X', 'POST', endpoint] + for (const pullRequest of pullRequests) { + command.push('-F', `pull_requests[]=${pullRequest}`) + } + const { stdout } = await ghExecFileAsync(command, { + ...ghOptions(args.repoPath, args.repository, args.connectionId, options), + idempotent: false + }) + const stackNumber = Number((JSON.parse(stdout) as { number?: unknown }).number) + if (!Number.isInteger(stackNumber) || stackNumber <= 0) { + throw new Error('GitHub returned an invalid stack response.') + } + return { + ok: true, + ...args.currentReview, + stackNumber, + parentReview: args.parentReview + } + } catch (error) { + console.warn('GitHub stack registration failed:', error) + return { + ok: false, + code: isStacksUnavailableError(error) ? 'validation' : 'unknown', + error: isStacksUnavailableError(error) + ? 'The pull request was created, but GitHub stacks are not available for this repository.' + : 'The pull request was created, but GitHub could not add it to the stack. Retry to finish stack registration.', + createdReview: args.currentReview + } + } finally { + release() + } +} diff --git a/src/main/ipc/hosted-review.test.ts b/src/main/ipc/hosted-review.test.ts index cef5d7f1d66..0399446e462 100644 --- a/src/main/ipc/hosted-review.test.ts +++ b/src/main/ipc/hosted-review.test.ts @@ -13,6 +13,7 @@ function setPlatform(platform: NodeJS.Platform): void { const { handleMock, createHostedReviewMock, + createStackedHostedReviewMock, getHostedReviewCreationEligibilityMock, getHostedReviewForBranchMock, resolveRegisteredWorktreePathMock, @@ -20,6 +21,7 @@ const { } = vi.hoisted(() => ({ handleMock: vi.fn(), createHostedReviewMock: vi.fn(), + createStackedHostedReviewMock: vi.fn(), getHostedReviewCreationEligibilityMock: vi.fn(), getHostedReviewForBranchMock: vi.fn(), resolveRegisteredWorktreePathMock: vi.fn(), @@ -37,6 +39,10 @@ vi.mock('../source-control/hosted-review-creation', () => ({ getHostedReviewCreationEligibility: getHostedReviewCreationEligibilityMock })) +vi.mock('../source-control/stacked-hosted-review-creation', () => ({ + createStackedHostedReview: createStackedHostedReviewMock +})) + vi.mock('../source-control/hosted-review', () => ({ getHostedReviewForBranch: getHostedReviewForBranchMock })) @@ -87,6 +93,7 @@ describe('registerHostedReviewHandlers', () => { setPlatform(ORIGINAL_PLATFORM) handleMock.mockReset() createHostedReviewMock.mockReset() + createStackedHostedReviewMock.mockReset() getHostedReviewCreationEligibilityMock.mockReset() getHostedReviewForBranchMock.mockReset() resolveRegisteredWorktreePathMock.mockReset() @@ -384,6 +391,35 @@ describe('registerHostedReviewHandlers', () => { ) }) + it('routes stacked creation through its dedicated SSH-safe handler', async () => { + createStackedHostedReviewMock.mockResolvedValueOnce({ + ok: true, + number: 43, + url: 'https://github.com/acme/orca/pull/43', + stackNumber: 50, + parentReview: { number: 42, url: 'https://github.com/acme/orca/pull/42' } + }) + registerHostedReviewHandlers(store as never, stats as never) + + await handlers['hostedReview:createStacked'](null, { + repoPath, + repoId: repo.id, + worktreePath, + provider: 'github', + base: 'stack/parent', + head: 'stack/child', + title: 'Child' + }) + + expect(createStackedHostedReviewMock).toHaveBeenCalledWith( + worktreePath, + expect.objectContaining({ base: 'stack/parent', head: 'stack/child' }), + 'ssh-1', + {} + ) + expect(createHostedReviewMock).not.toHaveBeenCalled() + }) + it('rejects creation when repoId and repoPath point at different registered repos', async () => { store.getRepo.mockImplementation((repoId: string) => repoId === repo.id ? { ...repo, path: '/other/repo' } : null diff --git a/src/main/ipc/hosted-review.ts b/src/main/ipc/hosted-review.ts index 670064d3e54..736668e1f0d 100644 --- a/src/main/ipc/hosted-review.ts +++ b/src/main/ipc/hosted-review.ts @@ -2,6 +2,7 @@ import { ipcMain } from 'electron' import { posix, resolve } from 'node:path' import type { CreateHostedReviewArgs, + CreateStackedHostedReviewArgs, HostedReviewCreationEligibilityArgs, HostedReviewForBranchArgs } from '../../shared/hosted-review' @@ -12,6 +13,7 @@ import { createHostedReview, getHostedReviewCreationEligibility } from '../source-control/hosted-review-creation' +import { createStackedHostedReview } from '../source-control/stacked-hosted-review-creation' import { getHostedReviewForBranch } from '../source-control/hosted-review' import { resolveRegisteredWorktreePath } from './filesystem-auth' import { listRepoWorktrees } from '../repo-worktrees' @@ -160,4 +162,44 @@ export function registerHostedReviewHandlers(store: Store, stats: StatsCollector } return result }) + + ipcMain.handle( + 'hostedReview:createStacked', + async (_event, args: CreateStackedHostedReviewArgs) => { + const repo = assertRegisteredRepo(args.repoPath, store, args.repoId) + const worktreePath = await resolveHostedReviewWorktreePath(repo, store, args.worktreePath) + const localGitOptions = getLocalProjectWorktreeGitOptions(store, repo) + const sharedLinkPaths = repo.connectionId ? [] : getWorktreeSharedLinkPaths(repo) + const executionOptions = { + ...(Object.keys(localGitOptions).length > 0 + ? { localGitExecOptions: localGitOptions } + : {}), + ...(sharedLinkPaths.length > 0 ? { sharedLinkPaths } : {}) + } + const input = { + provider: args.provider, + base: args.base, + head: args.head, + title: args.title, + body: args.body, + draft: args.draft, + ...(args.useTemplate !== undefined ? { useTemplate: args.useTemplate } : {}) + } + const result = await createStackedHostedReview( + worktreePath, + input, + repo.connectionId ?? null, + executionOptions + ) + if (result.ok && !stats.hasCountedPR(result.url)) { + stats.record({ + type: 'pr_created', + at: Date.now(), + repoId: repo.id, + meta: { prNumber: result.number, prUrl: result.url } + }) + } + return result + } + ) } diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index aa40c7ee64b..e5bacc5eb9e 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -234,6 +234,7 @@ const { invalidateAuthorizedRootsCacheMock, prepareLocalWorktreeRootForRepoMock, createHostedReviewMock, + createStackedHostedReviewMock, getHostedReviewCreationEligibilityMock, getHostedReviewForBranchMock, getPRForBranchMock, @@ -341,6 +342,7 @@ const { invalidateAuthorizedRootsCacheMock: vi.fn(), prepareLocalWorktreeRootForRepoMock: vi.fn(), createHostedReviewMock: vi.fn(), + createStackedHostedReviewMock: vi.fn(), getHostedReviewCreationEligibilityMock: vi.fn(), getHostedReviewForBranchMock: vi.fn(), getPRForBranchMock: vi.fn().mockResolvedValue(null), @@ -513,6 +515,10 @@ vi.mock('../source-control/hosted-review-creation', () => ({ getHostedReviewCreationEligibility: getHostedReviewCreationEligibilityMock })) +vi.mock('../source-control/stacked-hosted-review-creation', () => ({ + createStackedHostedReview: createStackedHostedReviewMock +})) + vi.mock('../source-control/hosted-review', () => ({ getHostedReviewForBranch: getHostedReviewForBranchMock })) @@ -732,6 +738,14 @@ function resetRuntimeTestMocks(): void { number: 1, url: 'https://example.com/pull/1' }) + createStackedHostedReviewMock.mockReset() + createStackedHostedReviewMock.mockResolvedValue({ + ok: true, + number: 2, + url: 'https://example.com/pull/2', + stackNumber: 10, + parentReview: { number: 1, url: 'https://example.com/pull/1' } + }) getHostedReviewCreationEligibilityMock.mockReset() getHostedReviewCreationEligibilityMock.mockResolvedValue({ provider: 'github', @@ -7005,6 +7019,15 @@ describe('OrcaRuntimeService', () => { body: '', draft: false }) + await runtime.createStackedHostedReview({ + repoSelector: `id:${TEST_REPO_ID}`, + provider: 'github', + base: 'stack/parent', + head: 'feature/ssh', + title: 'Feature SSH', + body: '', + draft: false + }) expect(getHostedReviewCreationEligibilityMock).toHaveBeenCalledWith( expect.objectContaining({ @@ -7022,6 +7045,16 @@ describe('OrcaRuntimeService', () => { }), 'ssh-1' ) + expect(createStackedHostedReviewMock).toHaveBeenCalledWith( + '/remote/repo', + expect.objectContaining({ + provider: 'github', + base: 'stack/parent', + head: 'feature/ssh' + }), + 'ssh-1', + {} + ) }) it('routes local WSL project hosted review flows through runtime git options', async () => { @@ -7084,6 +7117,15 @@ describe('OrcaRuntimeService', () => { body: '', draft: false }) + await runtime.createStackedHostedReview({ + repoSelector: `id:${TEST_REPO_ID}`, + provider: 'github', + base: 'stack/parent', + head: 'feature/wsl', + title: 'Feature WSL', + body: '', + draft: false + }) expect(getHostedReviewCreationEligibilityMock).toHaveBeenCalledWith( expect.objectContaining({ @@ -7112,6 +7154,16 @@ describe('OrcaRuntimeService', () => { null, { localGitExecOptions: { wslDistro: 'Ubuntu' } } ) + expect(createStackedHostedReviewMock).toHaveBeenCalledWith( + TEST_REPO_PATH, + expect.objectContaining({ + provider: 'github', + base: 'stack/parent', + head: 'feature/wsl' + }), + null, + { localGitExecOptions: { wslDistro: 'Ubuntu' } } + ) }) it('treats SSH worktree drift as unknown without local git probes', async () => { diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 691486d6dce..0b234b24160 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -676,6 +676,8 @@ import { inspectSetupScriptImportCandidates } from '../../shared/setup-script-im import type { CreateHostedReviewInput, CreateHostedReviewResult, + CreateStackedHostedReviewInput, + CreateStackedHostedReviewResult, HostedReviewCreationEligibility, HostedReviewCreationEligibilityArgs, HostedReviewInfo @@ -685,6 +687,7 @@ import { createHostedReview as createHostedReviewFromRepo, getHostedReviewCreationEligibility as getHostedReviewCreationEligibilityFromRepo } from '../source-control/hosted-review-creation' +import { createStackedHostedReview as createStackedHostedReviewFromRepo } from '../source-control/stacked-hosted-review-creation' import { getLocalProjectGitExecOptions, getLocalProjectWorktreeGitOptions, @@ -19688,6 +19691,36 @@ export class OrcaRuntimeService { return result } + async createStackedHostedReview( + args: CreateStackedHostedReviewInput & { repoSelector: string; worktreeSelector?: string } + ): Promise { + const { repo, repoPath } = await this.resolveHostedReviewTarget(args) + const executionOptions = this.getHostedReviewExecutionOptions(repo) + const result = await createStackedHostedReviewFromRepo( + repoPath, + { + provider: args.provider, + base: args.base, + head: args.head, + title: args.title, + body: args.body, + draft: args.draft, + ...(args.useTemplate !== undefined ? { useTemplate: args.useTemplate } : {}) + }, + repo.connectionId ?? null, + executionOptions ?? {} + ) + if (result.ok && this.stats && !this.stats.hasCountedPR(result.url)) { + this.stats.record({ + type: 'pr_created', + at: Date.now(), + repoId: repo.id, + meta: { prNumber: result.number, prUrl: result.url } + }) + } + return result + } + async listGitLabRepoWorkItems( repoSelector: string, state?: MRListState, diff --git a/src/main/runtime/rpc/methods/hosted-review.test.ts b/src/main/runtime/rpc/methods/hosted-review.test.ts index 3d132e82e41..ba4b05790b7 100644 --- a/src/main/runtime/rpc/methods/hosted-review.test.ts +++ b/src/main/runtime/rpc/methods/hosted-review.test.ts @@ -162,4 +162,39 @@ describe('hosted review RPC methods', () => { result: { ok: true, number: 51 } }) }) + + it('dispatches stacked creation through a distinct runtime method', async () => { + const runtime = { + getRuntimeId: () => 'test-runtime', + createStackedHostedReview: vi.fn().mockResolvedValue({ + ok: true, + number: 52, + url: 'https://github.com/acme/orca/pull/52', + stackNumber: 60, + parentReview: { number: 51, url: 'https://github.com/acme/orca/pull/51' } + }) + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: HOSTED_REVIEW_METHODS }) + + const response = await dispatcher.dispatch( + makeRequest('hostedReview.createStacked', { + repo: 'repo-1', + worktree: 'path:/worktrees/child', + provider: 'github', + base: 'stack/parent', + head: 'stack/child', + title: 'Child' + }) + ) + + expect(runtime.createStackedHostedReview).toHaveBeenCalledWith({ + repoSelector: 'repo-1', + worktreeSelector: 'path:/worktrees/child', + provider: 'github', + base: 'stack/parent', + head: 'stack/child', + title: 'Child' + }) + expect(response).toMatchObject({ ok: true, result: { ok: true, stackNumber: 60 } }) + }) }) diff --git a/src/main/runtime/rpc/methods/hosted-review.ts b/src/main/runtime/rpc/methods/hosted-review.ts index b658c4c74cb..0d48b02fb19 100644 --- a/src/main/runtime/rpc/methods/hosted-review.ts +++ b/src/main/runtime/rpc/methods/hosted-review.ts @@ -105,5 +105,21 @@ export const HOSTED_REVIEW_METHODS: RpcMethod[] = [ draft: params.draft, useTemplate: params.useTemplate }) + }), + defineMethod({ + name: 'hostedReview.createStacked', + params: HostedReviewCreate, + handler: async (params, { runtime }) => + runtime.createStackedHostedReview({ + repoSelector: params.repo, + worktreeSelector: params.worktree, + provider: params.provider, + base: params.base, + head: params.head, + title: params.title, + body: params.body, + draft: params.draft, + useTemplate: params.useTemplate + }) }) ] diff --git a/src/main/runtime/runtime-rpc.ts b/src/main/runtime/runtime-rpc.ts index 8b5a24f15d2..b783382c544 100644 --- a/src/main/runtime/runtime-rpc.ts +++ b/src/main/runtime/runtime-rpc.ts @@ -316,6 +316,7 @@ const MOBILE_RPC_METHOD_ALLOWLIST = new Set([ 'host.wsl.isAvailable', 'host.wsl.listDistros', 'hostedReview.create', + 'hostedReview.createStacked', 'hostedReview.forBranch', 'hostedReview.getCreationEligibility', 'linear.getCustomView', diff --git a/src/main/source-control/hosted-review-creation-eligibility.test.ts b/src/main/source-control/hosted-review-creation-eligibility.test.ts index f2fa711ee54..2cf262e8cca 100644 --- a/src/main/source-control/hosted-review-creation-eligibility.test.ts +++ b/src/main/source-control/hosted-review-creation-eligibility.test.ts @@ -472,29 +472,31 @@ describe('getHostedReviewCreationEligibility', () => { blockedReason: null, nextAction: null, defaultBaseRef: 'origin/main', - head: 'feature/create-pr' + head: 'feature/create-pr', + stackedCreationSupported: true }) }) it('detects a GitHub Enterprise Server branch as the GitHub provider (#8312)', async () => { mockGitHubEnterpriseProvider() - await expect( - getHostedReviewCreationEligibility({ - repoPath: '/repo', - branch: 'feature/create-pr', - base: 'origin/main', - hasUncommittedChanges: false, - hasUpstream: true, - ahead: 0, - behind: 0 - }) - ).resolves.toMatchObject({ + const result = await getHostedReviewCreationEligibility({ + repoPath: '/repo', + branch: 'feature/create-pr', + base: 'origin/main', + hasUncommittedChanges: false, + hasUpstream: true, + ahead: 0, + behind: 0 + }) + + expect(result).toMatchObject({ provider: 'github', canCreate: true, blockedReason: null, nextAction: null }) + expect(result).not.toHaveProperty('stackedCreationSupported') // Enterprise auth was already confirmed during detection; the gate must not // fire a redundant gh probe. diff --git a/src/main/source-control/hosted-review-creation.ts b/src/main/source-control/hosted-review-creation.ts index 58136542917..0fa50ef3a32 100644 --- a/src/main/source-control/hosted-review-creation.ts +++ b/src/main/source-control/hosted-review-creation.ts @@ -20,6 +20,8 @@ import { isAzureDevOpsReviewCreationAuthenticated } from '../azure-devops/pull-r import { isGiteaReviewCreationAuthenticated } from '../gitea/pull-request-creation' import { isBitbucketReviewCreationAuthenticated } from '../bitbucket/pull-request-creation' import { getEnterpriseGitHubRepoSlug } from '../github/github-enterprise-repository' +import { getRepoSlug } from '../github/client' +import { isDefaultGitHubHost } from '../../shared/github-repository-identity-key' import { acquire, ghExecFileAsync, gitExecFileAsync, release } from '../github/gh-utils' import { isNoUpstreamError, normalizeGitErrorMessage } from '../../shared/git-remote-error' import type { GitUpstreamStatus } from '../../shared/types' @@ -535,12 +537,19 @@ export async function getHostedReviewCreationEligibility( : lookupFailed ? 'unavailable' : 'not_found' + const githubRepository = + provider === 'github' + ? await getRepoSlug(args.repoPath, args.connectionId, args).catch(() => null) + : null const baseResult = { provider, review: review ? { number: review.number, url: review.url } : null, reviewLookupOutcome, defaultBaseRef, - head: branch || null + head: branch || null, + ...(githubRepository && isDefaultGitHubHost(githubRepository.host) + ? { stackedCreationSupported: true } + : {}) } if (!branch || branch === 'HEAD') { diff --git a/src/main/source-control/stacked-hosted-review-creation.test.ts b/src/main/source-control/stacked-hosted-review-creation.test.ts new file mode 100644 index 00000000000..7814e66e6c8 --- /dev/null +++ b/src/main/source-control/stacked-hosted-review-creation.test.ts @@ -0,0 +1,92 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { prepareMock, registerMock, createMock } = vi.hoisted(() => ({ + prepareMock: vi.fn(), + registerMock: vi.fn(), + createMock: vi.fn() +})) + +vi.mock('../github/stacked-pr-creation', () => ({ + prepareGitHubStackedPullRequest: prepareMock, + registerGitHubStackedPullRequest: registerMock +})) + +vi.mock('./hosted-review-creation', () => ({ createHostedReview: createMock })) + +import { createStackedHostedReview } from './stacked-hosted-review-creation' + +const input = { + provider: 'github' as const, + base: 'stack/parent', + head: 'stack/child', + title: 'Child' +} +const repository = { owner: 'acme', repo: 'orca', host: 'github.com' } +const parentReview = { number: 41, url: 'https://github.com/acme/orca/pull/41' } +const currentReview = { number: 42, url: 'https://github.com/acme/orca/pull/42' } + +beforeEach(() => { + prepareMock.mockReset() + registerMock.mockReset() + createMock.mockReset() +}) + +describe('createStackedHostedReview', () => { + it('creates the current PR before registering the stack', async () => { + prepareMock.mockResolvedValue({ + ok: true, + repository, + parentReview, + currentReview: null + }) + createMock.mockResolvedValue({ ok: true, ...currentReview }) + registerMock.mockResolvedValue({ + ok: true, + ...currentReview, + parentReview, + stackNumber: 50 + }) + + const result = await createStackedHostedReview('/repo', input, 'ssh-1') + + expect(result).toMatchObject({ ok: true, stackNumber: 50 }) + expect(createMock).toHaveBeenCalledWith('/repo', input, 'ssh-1', {}) + expect(registerMock).toHaveBeenCalledWith( + expect.objectContaining({ parentReview, currentReview, connectionId: 'ssh-1' }) + ) + }) + + it('retries registration without creating a duplicate PR', async () => { + prepareMock.mockResolvedValue({ + ok: true, + repository, + parentReview, + currentReview + }) + registerMock.mockResolvedValue({ + ok: true, + ...currentReview, + parentReview, + stackNumber: 50 + }) + + await createStackedHostedReview('/repo', input) + + expect(createMock).not.toHaveBeenCalled() + expect(registerMock).toHaveBeenCalledOnce() + }) + + it('does not create a PR when the parent topology is invalid', async () => { + prepareMock.mockResolvedValue({ + ok: false, + code: 'validation', + error: 'Choose the top pull request.' + }) + + const result = await createStackedHostedReview('/repo', input) + + expect(result).toMatchObject({ ok: false, code: 'validation' }) + expect(createMock).not.toHaveBeenCalled() + expect(registerMock).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/source-control/stacked-hosted-review-creation.ts b/src/main/source-control/stacked-hosted-review-creation.ts new file mode 100644 index 00000000000..0a36588f493 --- /dev/null +++ b/src/main/source-control/stacked-hosted-review-creation.ts @@ -0,0 +1,61 @@ +import type { + CreateStackedHostedReviewInput, + CreateStackedHostedReviewResult, + HostedReviewSummary +} from '../../shared/hosted-review' +import { + prepareGitHubStackedPullRequest, + registerGitHubStackedPullRequest +} from '../github/stacked-pr-creation' +import { createHostedReview } from './hosted-review-creation' +import type { HostedReviewExecutionOptions } from './hosted-review-git-options' + +export async function createStackedHostedReview( + repoPath: string, + input: CreateStackedHostedReviewInput, + connectionId?: string | null, + options: HostedReviewExecutionOptions = {} +): Promise { + const plan = await prepareGitHubStackedPullRequest(repoPath, input, connectionId, options) + if (!plan.ok) { + return plan + } + + let currentReview: (HostedReviewSummary & { number: number }) | null = plan.currentReview + if (!currentReview) { + const created = await createHostedReview(repoPath, input, connectionId, options) + if (!created.ok) { + if (!created.existingReview?.number) { + return created + } + return { + ok: false, + code: 'validation', + error: + 'An open pull request already exists for this branch but does not target the selected parent branch.', + createdReview: { + number: created.existingReview.number, + url: created.existingReview.url + } + } + } else { + currentReview = { number: created.number, url: created.url } + } + } + if (!currentReview) { + return { + ok: false, + code: 'unknown_completion', + error: 'Pull request creation may have completed. Retry to finish stack registration.' + } + } + + return registerGitHubStackedPullRequest({ + repoPath, + repository: plan.repository, + parentReview: plan.parentReview, + currentReview, + connectionId, + options + }) +} diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 17c64db922c..cf59c18719c 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -2,6 +2,8 @@ import type { CreateHostedReviewArgs, CreateHostedReviewResult, + CreateStackedHostedReviewArgs, + CreateStackedHostedReviewResult, HostedReviewCreationEligibility, HostedReviewCreationEligibilityArgs, HostedReviewForBranchArgs, @@ -1986,6 +1988,7 @@ export type PreloadApi = { args: HostedReviewCreationEligibilityArgs ) => Promise create: (args: CreateHostedReviewArgs) => Promise + createStacked: (args: CreateStackedHostedReviewArgs) => Promise } // ── GitLab — parallel to gh, MR/issue surface only in v1 ──────── // Shapes mirror gh.* except where GitLab's API differs (MR states, host-qualified project path, `glab api -i` paging). diff --git a/src/preload/index.ts b/src/preload/index.ts index 1254203b969..70ce60249b1 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -1715,7 +1715,9 @@ const api = { ipcRenderer.invoke('hostedReview:forBranch', args), getCreationEligibility: (args: unknown): Promise => ipcRenderer.invoke('hostedReview:getCreationEligibility', args), - create: (args: unknown): Promise => ipcRenderer.invoke('hostedReview:create', args) + create: (args: unknown): Promise => ipcRenderer.invoke('hostedReview:create', args), + createStacked: (args: unknown): Promise => + ipcRenderer.invoke('hostedReview:createStacked', args) }, // Why: GitLab bindings live in `./gitlab` so `gl.*` changes don't conflict on every upstream sync of this central file. diff --git a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx index de63d4ffd0e..c109c46b1a3 100644 --- a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx +++ b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx @@ -189,6 +189,7 @@ import { resolveSourceControlLaunchPlatform } from '@/lib/source-control-launch- import { getLocalProjectExecutionRuntimeContext } from '@/lib/local-preflight-context' import { getRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner' import { CreateHostedReviewComposer } from './CreateHostedReviewComposer' +import { useHostedReviewStackParent } from './useHostedReviewStackParent' import { resolveCreatedHostedReviewLink } from './source-control-created-review-link' import { formatCreateError } from './create-pull-request-review-copy' import { stripBaseRef, useCreatePullRequestDialogFields } from './useCreatePullRequestDialogFields' @@ -476,6 +477,7 @@ export default function ChecksPanel(): React.JSX.Element { (s) => s.getHostedReviewCreationEligibility ) const createHostedReview = useAppStore((s) => s.createHostedReview) + const createStackedHostedReview = useAppStore((s) => s.createStackedHostedReview) const enqueueGitHubPRRefresh = useAppStore((s) => s.enqueueGitHubPRRefresh) const conflictOperation = useAppStore((s) => activeWorktreeId ? (s.gitConflictOperationByWorktree[activeWorktreeId] ?? 'unknown') : 'unknown' @@ -1320,10 +1322,13 @@ export default function ChecksPanel(): React.JSX.Element { setBody: setPrBody, draft: prDraft, setDraft: setPrDraft, + stackedCreationSupported: prStackedCreationSupported, + repoDefaultBaseRef: prRepoDefaultBaseRef, baseQuery: prBaseQuery, setBaseQuery: setPrBaseQuery, baseResults: prBaseResults, setBaseResults: setPrBaseResults, + baseSearchPending: prBaseSearchPending, baseSearchError: prBaseSearchError, generating: prGenerating, generateError: prGenerateError, @@ -1360,6 +1365,17 @@ export default function ChecksPanel(): React.JSX.Element { onCancelGenerate: handleCancelGeneratePullRequestFieldsForActive } }) + const stackParentReview = useHostedReviewStackParent({ + enabled: hostedReviewCreateProvider === 'github' && prStackedCreationSupported, + repoPath: repo?.path ?? '', + repoId: repo?.id ?? null, + base: prBase, + // Why: the repo default, not eligibility's defaultBaseRef — that one resolves to + // the worktree's own base, which is exactly the branch a stacked PR targets. + repoDefaultBase: prRepoDefaultBaseRef, + head: branch, + fetchHostedReviewForBranch + }) useEffect(() => { // Why: PR generation can finish while this composer is hidden by a worktree switch; hydrate once the original composer is visible again. if ( @@ -3970,121 +3986,86 @@ export default function ChecksPanel(): React.JSX.Element { ] ) - const handleCreatePullRequest = useCallback(async (): Promise => { - if (!repo || !branch || !createComposerOpen || prGenerating || createPrInFlightRef.current) { - return - } + const handleCreatePullRequest = useCallback( + async (stacked = false): Promise => { + if (!repo || !branch || !createComposerOpen || prGenerating || createPrInFlightRef.current) { + return + } - const requestContextKey = panelContextKey - const isCurrentCreateRequest = (): boolean => - panelContextKeyRef.current === requestContextKey && - createPrInFlightRef.current === requestContextKey - const base = stripBaseRef(prBase).trim() - const title = prTitle.trim() - const worktreePath = activeWorktreePath ?? repo.path - if (!title) { - setCreatePrError( - translate( - 'auto.components.right.sidebar.SourceControl.f3a8b2c1d0e5', - 'Enter a {{value0}} title.', - { - value0: hostedReviewCreateCopy.reviewLabel + const requestContextKey = panelContextKey + const isCurrentCreateRequest = (): boolean => + panelContextKeyRef.current === requestContextKey && + createPrInFlightRef.current === requestContextKey + const base = stripBaseRef(prBase).trim() + const title = prTitle.trim() + const worktreePath = activeWorktreePath ?? repo.path + if (!title) { + setCreatePrError( + translate( + 'auto.components.right.sidebar.SourceControl.f3a8b2c1d0e5', + 'Enter a {{value0}} title.', + { + value0: hostedReviewCreateCopy.reviewLabel + } + ) + ) + return + } + if (!base || stripBaseRef(base).toLowerCase() === stripBaseRef(branch).toLowerCase()) { + setCreatePrError( + translate( + 'auto.components.right.sidebar.SourceControl.ae743199cd', + 'Choose a different base branch before creating a {{value0}}.', + { value0: hostedReviewCreateCopy.reviewLabel } + ) + ) + return + } + + createPrInFlightRef.current = requestContextKey + setIsCreatingPr(true) + setCreatePrError(null) + let pushed = false + try { + const shouldPushBeforeCreate = + createPrPushFirst || hostedReviewCreation?.blockedReason === 'needs_push' + if (shouldPushBeforeCreate) { + const ok = await pushBeforeCreatePullRequest() + if (!isCurrentCreateRequest()) { + return } - ) - ) - return - } - if (!base || stripBaseRef(base).toLowerCase() === stripBaseRef(branch).toLowerCase()) { - setCreatePrError( - translate( - 'auto.components.right.sidebar.SourceControl.ae743199cd', - 'Choose a different base branch before creating a {{value0}}.', - { value0: hostedReviewCreateCopy.reviewLabel } - ) - ) - return - } - - createPrInFlightRef.current = requestContextKey - setIsCreatingPr(true) - setCreatePrError(null) - let pushed = false - try { - const shouldPushBeforeCreate = - createPrPushFirst || hostedReviewCreation?.blockedReason === 'needs_push' - if (shouldPushBeforeCreate) { - const ok = await pushBeforeCreatePullRequest() + if (!ok) { + setCreatePrError('Push failed. Resolve the push error, then try again.') + return + } + pushed = true + } + const createInput = { + repoId: repo.id, + provider: hostedReviewCreateProvider, + base, + head: normalizeHostedReviewHeadRef(branch), + title, + body: prBody, + draft: prDraft && hostedReviewProviderSupportsDraft(hostedReviewCreateProvider), + worktreePath, + useTemplate: prCreationDefaults.useTemplate + } + const result = stacked + ? await createStackedHostedReview(repo.path, createInput) + : await createHostedReview(repo.path, createInput) if (!isCurrentCreateRequest()) { return } - if (!ok) { - setCreatePrError('Push failed. Resolve the push error, then try again.') - return - } - pushed = true - } - const result = await createHostedReview(repo.path, { - repoId: repo.id, - provider: hostedReviewCreateProvider, - base, - head: normalizeHostedReviewHeadRef(branch), - title, - body: prBody, - draft: prDraft && hostedReviewProviderSupportsDraft(hostedReviewCreateProvider), - worktreePath, - useTemplate: prCreationDefaults.useTemplate - }) - if (!isCurrentCreateRequest()) { - return - } - if (result.ok) { - await handlePullRequestCreated({ - provider: hostedReviewCreateProvider, - number: result.number, - url: result.url - }) - if (prCreationDefaults.openAfterCreate) { - openHttpLink(result.url, { worktreeId: activeWorktreeId }) - } - if (activePullRequestGenerationKey) { - updatePullRequestGenerationRecord( - activePullRequestGenerationKey, - clearPullRequestGenerationRequiresPushBeforeCreate - ) - } - return - } - if (result.existingReview?.url) { - const number = result.existingReview.number - toast.success( - number - ? translate( - 'auto.components.right.sidebar.ChecksPanel.b6ce28da5b', - '{{value0}} #{{value1}} is already open', - { value0: hostedReviewCreateCopy.titleLabel, value1: number } - ) - : translate( - 'auto.components.right.sidebar.ChecksPanel.cf9e69f3be', - '{{value0}} is already open', - { value0: hostedReviewCreateCopy.titleLabel } - ), - { - action: { - label: translate( - 'auto.components.right.sidebar.ChecksPanel.192e686e57', - 'Open on {{value0}}', - { value0: hostedReviewCreateCopy.providerName } - ), - onClick: () => window.api.shell.openUrl(result.existingReview!.url) - } - } - ) - if (number) { + if (result.ok) { await handlePullRequestCreated({ provider: hostedReviewCreateProvider, - number, - url: result.existingReview.url + number: result.number, + url: result.url }) + if (prCreationDefaults.openAfterCreate) { + openHttpLink(result.url, { worktreeId: activeWorktreeId }) + } if (activePullRequestGenerationKey) { updatePullRequestGenerationRecord( activePullRequestGenerationKey, @@ -4093,55 +4074,110 @@ export default function ChecksPanel(): React.JSX.Element { } return } + if ('existingReview' in result && result.existingReview?.url) { + const number = result.existingReview.number + toast.success( + number + ? translate( + 'auto.components.right.sidebar.ChecksPanel.b6ce28da5b', + '{{value0}} #{{value1}} is already open', + { value0: hostedReviewCreateCopy.titleLabel, value1: number } + ) + : translate( + 'auto.components.right.sidebar.ChecksPanel.cf9e69f3be', + '{{value0}} is already open', + { value0: hostedReviewCreateCopy.titleLabel } + ), + { + action: { + label: translate( + 'auto.components.right.sidebar.ChecksPanel.192e686e57', + 'Open on {{value0}}', + { value0: hostedReviewCreateCopy.providerName } + ), + onClick: () => window.api.shell.openUrl(result.existingReview!.url) + } + } + ) + if (number) { + await handlePullRequestCreated({ + provider: hostedReviewCreateProvider, + number, + url: result.existingReview.url + }) + if (activePullRequestGenerationKey) { + updatePullRequestGenerationRecord( + activePullRequestGenerationKey, + clearPullRequestGenerationRequiresPushBeforeCreate + ) + } + return + } + } + // Why: stacked creation can create the pull request and still fail to register + // the stack. Link the review that exists before surfacing the stack failure, or + // the workspace stays unaware of a PR the user can already see on GitHub. + if ('createdReview' in result && result.createdReview?.url) { + const { number, url } = result.createdReview + if (number) { + await handlePullRequestCreated({ + provider: hostedReviewCreateProvider, + number, + url + }) + } + } + setCreatePrError(formatCreateError(result, pushed, hostedReviewCreateCopy.shortLabel)) + } catch (error) { + if (!isCurrentCreateRequest()) { + return + } + setCreatePrError( + error instanceof Error + ? error.message + : translate( + 'auto.components.right.sidebar.SourceControl.e2b7a1c0d9f4', + 'Failed to create {{value0}}', + { value0: hostedReviewCreateCopy.reviewLabel } + ) + ) + } finally { + if (createPrInFlightRef.current === requestContextKey) { + createPrInFlightRef.current = null + setIsCreatingPr(false) + setGitStatusRefreshNonce((value) => value + 1) + } } - setCreatePrError(formatCreateError(result, pushed, hostedReviewCreateCopy.shortLabel)) - } catch (error) { - if (!isCurrentCreateRequest()) { - return - } - setCreatePrError( - error instanceof Error - ? error.message - : translate( - 'auto.components.right.sidebar.SourceControl.e2b7a1c0d9f4', - 'Failed to create {{value0}}', - { value0: hostedReviewCreateCopy.reviewLabel } - ) - ) - } finally { - if (createPrInFlightRef.current === requestContextKey) { - createPrInFlightRef.current = null - setIsCreatingPr(false) - setGitStatusRefreshNonce((value) => value + 1) - } - } - }, [ - activeWorktreePath, - activeWorktreeId, - activePullRequestGenerationKey, - branch, - createComposerOpen, - createHostedReview, - createPrPushFirst, - handlePullRequestCreated, - hostedReviewCreateCopy.providerName, - hostedReviewCreateCopy.reviewLabel, - hostedReviewCreateCopy.shortLabel, - hostedReviewCreateCopy.titleLabel, - hostedReviewCreateProvider, - hostedReviewCreation?.blockedReason, - panelContextKey, - prBase, - prBody, - prCreationDefaults.openAfterCreate, - prCreationDefaults.useTemplate, - prDraft, - prGenerating, - prTitle, - pushBeforeCreatePullRequest, - repo, - updatePullRequestGenerationRecord - ]) + }, + [ + activeWorktreePath, + activeWorktreeId, + activePullRequestGenerationKey, + branch, + createComposerOpen, + createHostedReview, + createStackedHostedReview, + createPrPushFirst, + handlePullRequestCreated, + hostedReviewCreateCopy.providerName, + hostedReviewCreateCopy.reviewLabel, + hostedReviewCreateCopy.shortLabel, + hostedReviewCreateCopy.titleLabel, + hostedReviewCreateProvider, + hostedReviewCreation?.blockedReason, + panelContextKey, + prBase, + prBody, + prCreationDefaults.openAfterCreate, + prCreationDefaults.useTemplate, + prDraft, + prGenerating, + prTitle, + pushBeforeCreatePullRequest, + repo, + updatePullRequestGenerationRecord + ] + ) // ── Empty state ── if (!activeWorktree) { @@ -4283,10 +4319,12 @@ export default function ChecksPanel(): React.JSX.Element { {!operationInProgress && createComposerOpen ? (
void handleGeneratePullRequestFields()} onCancelGenerate={handleCancelGeneratePullRequestFields} - onPrimaryAction={() => void handleCreatePullRequest()} + onPrimaryAction={(stacked) => void handleCreatePullRequest(stacked)} />
) : null} diff --git a/src/renderer/src/components/right-sidebar/CreateHostedReviewBasePicker.tsx b/src/renderer/src/components/right-sidebar/CreateHostedReviewBasePicker.tsx new file mode 100644 index 00000000000..3f1cc1d37a2 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/CreateHostedReviewBasePicker.tsx @@ -0,0 +1,292 @@ +import { Check, ChevronDown } from 'lucide-react' +import { useId, useRef, useState } from 'react' +import { Input } from '@/components/ui/input' +import { Label } from '@/components/ui/label' +import { cn } from '@/lib/utils' +import { translate } from '@/i18n/i18n' +import type { LocalizedHostedReviewCopy } from '@/i18n/hosted-review-localized-copy' +import { COMPOSER_FIELD_CLASS } from './create-hosted-review-composer-field-class' +import { CreateHostedReviewComposerMessage } from './CreateHostedReviewComposerMessage' +import { stripBaseRef } from './useCreatePullRequestDialogFields' + +type CreateHostedReviewBasePickerProps = { + copy: LocalizedHostedReviewCopy + base: string + setBase: (value: string) => void + /** The repo's default branch, where an emptied field lands. Null until resolved. */ + repoDefaultBase: string | null + editing: boolean + setEditing: (value: boolean) => void + baseQuery: string + setBaseQuery: (value: string) => void + baseResults: string[] + setBaseResults: (value: string[]) => void + baseSearchPending: boolean + baseSearchError: string | null + fieldsLocked: boolean + strippedBranch: string + baseSameAsBranch: boolean +} + +/** + * The composer's merge target: a labelled full-width combobox whose results stay + * attached to it, plus the base-scoped errors. + */ +export function CreateHostedReviewBasePicker({ + copy, + base, + setBase, + repoDefaultBase, + editing, + setEditing, + baseQuery, + setBaseQuery, + baseResults, + setBaseResults, + baseSearchPending, + baseSearchError, + fieldsLocked, + strippedBranch, + baseSameAsBranch +}: CreateHostedReviewBasePickerProps): React.JSX.Element { + const [activeResult, setActiveResult] = useState(-1) + const inputRef = useRef(null) + // Why: marks a blur that an explicit Enter/Escape already resolved. + const settledRef = useRef(false) + const fieldId = useId() + const resultsId = useId() + + const trimmedQuery = baseQuery.trim() + const trimmedRepoDefault = repoDefaultBase?.trim() ?? '' + const showResults = editing && baseResults.length > 0 + // Why: emptying the field is how you say "not this branch"; name where it lands so + // the reset isn't an invisible behaviour. + const showRepoDefaultHint = editing && trimmedQuery.length === 0 && trimmedRepoDefault.length > 0 + // Why: only claim "no branches match" once a search has actually settled, so the + // debounce window can't report an absence the app hasn't observed yet. + const showNoResults = + editing && + baseResults.length === 0 && + trimmedQuery.length >= 2 && + !baseSearchPending && + !baseSearchError + + const closeSearch = (): void => { + setEditing(false) + setBaseQuery('') + setBaseResults([]) + setActiveResult(-1) + } + + const commitSearch = (value: string): void => { + // Why: an emptied field commits the repo default rather than silently restoring + // the branch the user just cleared. With no default resolved yet there is nothing + // honest to fall back to, so the committed base stands. + const nextBase = value.trim() || trimmedRepoDefault + if (nextBase) { + setBase(nextBase) + } + settledRef.current = true + closeSearch() + inputRef.current?.blur() + } + + const cancelSearch = (): void => { + settledRef.current = true + closeSearch() + inputRef.current?.blur() + } + + const handleBlur = (): void => { + // Why: Enter and Escape blur the input themselves; without this they would + // re-enter the commit path below with the pre-close query still in scope. + if (settledRef.current) { + settledRef.current = false + closeSearch() + return + } + // Why: clicking away from an emptied field means what pressing Enter on it + // means — land on the repo default instead of restoring what was cleared. + // A partial query still cancels; only an empty one is an instruction. + if (trimmedQuery.length === 0 && trimmedRepoDefault) { + setBase(trimmedRepoDefault) + } + closeSearch() + } + + const moveActiveResult = (delta: number): void => { + if (baseResults.length === 0) { + return + } + setActiveResult((current) => { + const next = current + delta + if (next < 0) { + return baseResults.length - 1 + } + return next >= baseResults.length ? 0 : next + }) + } + + const handleKeyDown = (event: React.KeyboardEvent): void => { + if (event.key === 'ArrowDown' || event.key === 'ArrowUp') { + event.preventDefault() + moveActiveResult(event.key === 'ArrowDown' ? 1 : -1) + return + } + if (event.key === 'Enter') { + event.preventDefault() + commitSearch(baseResults[activeResult] ?? baseQuery) + return + } + if (event.key === 'Escape') { + event.preventDefault() + cancelSearch() + } + } + + return ( + // Why: the base owns the full sidebar width — branch names are long — and the + // head branch rides the label row instead of costing another line. +
+
+ {/* Why: the label holds its line and the head branch absorbs the squeeze — + a wrapping two-word label next to a one-line ref reads as broken. */} + + + {translate( + 'auto.components.right.sidebar.CreateHostedReviewBasePicker.bb4b41d563', + 'from {{value0}}', + { value0: strippedBranch } + )} + +
+ +
+ = 0 ? `${resultsId}-${activeResult}` : undefined + } + aria-invalid={baseSameAsBranch || undefined} + // Why: an input can't ellipsize, and base refs routinely overflow the sidebar. + title={editing ? undefined : base} + value={editing ? baseQuery : base} + disabled={fieldsLocked} + onFocus={(event) => { + // Why: a programmatic blur that never fired would otherwise leave the + // flag set and swallow the next real one. + settledRef.current = false + setEditing(true) + setBaseQuery(event.currentTarget.value) + }} + onBlur={handleBlur} + onChange={(event) => { + setBaseQuery(event.target.value) + setActiveResult(-1) + }} + onKeyDown={handleKeyDown} + // Why: the placeholder is where an emptied field lands, so it has to be the + // repo's real default — a hardcoded "main" lies on a trunk-named repo. + placeholder={ + trimmedRepoDefault || + translate('auto.components.right.sidebar.SourceControl.e64a632456', 'main') + } + className={cn(COMPOSER_FIELD_CLASS, 'pl-2 pr-7')} + /> +
+ + {showResults ? ( +
+ {baseResults.map((ref, index) => { + const selected = stripBaseRef(base) === ref + return ( + + ) + })} +
+ ) : null} + + {showRepoDefaultHint ? ( +

+ {translate( + 'auto.components.right.sidebar.CreateHostedReviewBasePicker.da4d57c9c2', + 'Leave empty to use {{value0}}.', + { value0: trimmedRepoDefault } + )} +

+ ) : null} + + {showNoResults ? ( +

+ {translate( + 'auto.components.right.sidebar.CreateHostedReviewBasePicker.5a9315b61a', + 'No branches match “{{value0}}”. Press Enter to use it anyway.', + { value0: trimmedQuery } + )} +

+ ) : null} + + {/* Why: base problems belong to the base field, not to the block of + operation errors above the submit button. */} + {baseSameAsBranch ? ( + + {translate( + 'auto.components.right.sidebar.SourceControl.ae743199cd', + 'Choose a different base branch before creating a {{value0}}.', + { value0: copy.reviewLabel } + )} + + ) : null} + {baseSearchError ? ( + {baseSearchError} + ) : null} +
+ ) +} diff --git a/src/renderer/src/components/right-sidebar/CreateHostedReviewComposer.tsx b/src/renderer/src/components/right-sidebar/CreateHostedReviewComposer.tsx index 63c66b01ddc..7876e8dca25 100644 --- a/src/renderer/src/components/right-sidebar/CreateHostedReviewComposer.tsx +++ b/src/renderer/src/components/right-sidebar/CreateHostedReviewComposer.tsx @@ -1,3 +1,4 @@ +import { useState } from 'react' import { ChevronDown, GitMerge, @@ -26,8 +27,10 @@ import { type HostedReviewProvider } from '../../../../shared/hosted-review' import { stripBaseRef } from './useCreatePullRequestDialogFields' +import type { HostedReviewStackParent } from './useHostedReviewStackParent' import type { DropdownActionKind, DropdownEntry } from './source-control-dropdown-items' import { CreateHostedReviewComposerFields } from './CreateHostedReviewComposerFields' +import { getCreateButtonLabel } from './create-hosted-review-button-label' import { RIGHT_SIDEBAR_MORPHING_PRIMARY_BUTTON_CLASS, RIGHT_SIDEBAR_PRIMARY_BUTTON_LABEL_CLASS, @@ -47,16 +50,20 @@ export type CreateHostedReviewComposerProps = { branch: string base: string setBase: (value: string) => void + repoDefaultBase: string | null title: string setTitle: (value: string) => void body: string setBody: (value: string) => void draft: boolean setDraft: (value: boolean) => void + stackedCreationSupported: boolean + stackParentReview: HostedReviewStackParent | null baseQuery: string setBaseQuery: (value: string) => void baseResults: string[] setBaseResults: (value: string[]) => void + baseSearchPending: boolean baseSearchError: string | null aiGenerationEnabled: boolean generating: boolean @@ -70,7 +77,7 @@ export type CreateHostedReviewComposerProps = { dropdownItems?: DropdownEntry[] onGenerate: () => void onCancelGenerate: () => void - onPrimaryAction: () => void + onPrimaryAction: (stacked: boolean) => void onDropdownAction?: (kind: DropdownActionKind) => void } @@ -80,16 +87,20 @@ export function CreateHostedReviewComposer({ branch, base, setBase, + repoDefaultBase, title, setTitle, body, setBody, draft, setDraft, + stackedCreationSupported, + stackParentReview, baseQuery, setBaseQuery, baseResults, setBaseResults, + baseSearchPending, baseSearchError, aiGenerationEnabled, generating, @@ -111,7 +122,20 @@ export function CreateHostedReviewComposer({ const supportsDraft = hostedReviewProviderSupportsDraft(provider) const effectiveDraft = supportsDraft && draft const ReviewIcon = provider === 'gitlab' ? GitMerge : GitPullRequestArrow + const stackedModeAvailable = provider === 'github' && stackedCreationSupported const normalizedBase = stripBaseRef(base) + const stackSelectionKey = stackParentReview + ? `${normalizedBase}:${stackParentReview.number}` + : null + const [stackSelection, setStackSelection] = useState({ key: '', enabled: false }) + const effectiveStacked = + stackedModeAvailable && + stackSelectionKey !== null && + stackSelection.key === stackSelectionKey && + stackSelection.enabled + const setStacked = (enabled: boolean): void => { + setStackSelection({ key: stackSelectionKey ?? '', enabled }) + } const strippedBranch = stripBaseRef(branch) const baseSameAsBranch = normalizedBase.toLowerCase() === strippedBranch.toLowerCase() const createDisabled = @@ -197,7 +221,9 @@ export function CreateHostedReviewComposer({ return (
-
+ {/* Why: one gap between groups, tighter gaps inside them — the form reads as + content → merge target → options → action instead of a stack of boxes. */} +