xiaoxue_admin
/
orca
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix: prepare branch before PR field generation (#2401)

This commit is contained in:
Jinjing 2026-05-19 22:20:56 -07:00 committed by GitHub
parent 9adb33c24a
commit dbc24be41d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 459 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
src/main/ipc/filesystem.ts
View File

@ -771,15 +771,24 @@ export function registerFilesystemHandlers(
error: SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE
}
}
const context = await getPullRequestDraftContext(
(argv) => provider.exec(argv, args.worktreePath),
{
base: args.base,
currentTitle: args.title,
currentBody: args.body,
currentDraft: args.draft
let context: Awaited<ReturnType<typeof getPullRequestDraftContext>>
try {
context = await getPullRequestDraftContext(
(argv) => provider.exec(argv, args.worktreePath),
{
base: args.base,
currentTitle: args.title,
currentBody: args.body,
currentDraft: args.draft
}
)
} catch (error) {
return {
success: false,
error:
error instanceof Error ? error.message : 'Failed to prepare branch for PR details.'
}
)
}
if (!context) {
return { success: false, error: 'No branch changes to summarize.' }
}
@ -793,15 +802,23 @@ export function registerFilesystemHandlers(
}
const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store)
const context = await getPullRequestDraftContext(
(argv, options) => gitExecFileAsync(argv, { cwd: worktreePath, ...options }),
{
base: args.base,
currentTitle: args.title,
currentBody: args.body,
currentDraft: args.draft
let context: Awaited<ReturnType<typeof getPullRequestDraftContext>>
try {
context = await getPullRequestDraftContext(
(argv, options) => gitExecFileAsync(argv, { cwd: worktreePath, ...options }),
{
base: args.base,
currentTitle: args.title,
currentBody: args.body,
currentDraft: args.draft
}
)
} catch (error) {
return {
success: false,
error: error instanceof Error ? error.message : 'Failed to prepare branch for PR details.'
}
)
}
if (!context) {
return { success: false, error: 'No branch changes to summarize.' }
}

32
src/main/runtime/orca-runtime-git.ts
View File

@ -443,22 +443,30 @@ export class RuntimeGitCommands {
error: SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE
}
}
const context = target.connectionId
? await getPullRequestDraftContext((argv) => provider!.exec(argv, target.worktree.path), {
base: input.base,
currentTitle: input.title,
currentBody: input.body,
currentDraft: input.draft
})
: await getPullRequestDraftContext(
(argv, options) => gitExecFileAsync(argv, { cwd: target.worktree.path, ...options }),
{
let context: Awaited<ReturnType<typeof getPullRequestDraftContext>>
try {
context = target.connectionId
? await getPullRequestDraftContext((argv) => provider!.exec(argv, target.worktree.path), {
base: input.base,
currentTitle: input.title,
currentBody: input.body,
currentDraft: input.draft
}
)
})
: await getPullRequestDraftContext(
(argv, options) => gitExecFileAsync(argv, { cwd: target.worktree.path, ...options }),
{
base: input.base,
currentTitle: input.title,
currentBody: input.body,
currentDraft: input.draft
}
)
} catch (error) {
return {
success: false,
error: error instanceof Error ? error.message : 'Failed to prepare branch for PR details.'
}
}
if (!context) {
return { success: false, error: 'No branch changes to summarize.' }
}

92
src/main/text-generation/commit-message-text-generation.test.ts
View File

@ -748,6 +748,7 @@ describe('generateCommitMessageFromContext', () => {
{
branch: 'feature/pr-fields',
base: 'main',
branchChangedByPreparation: false,
currentTitle: '',
currentBody: '',
currentDraft: false,
@ -797,6 +798,97 @@ describe('generateCommitMessageFromContext', () => {
expect(children[1]?.kill).not.toHaveBeenCalled()
})
it('reports branch changes when pull request field output cannot be parsed', async () => {
const listeners = new Map<string, (value: unknown) => void>()
spawnMock.mockReturnValue({
pid: 123,
kill: vi.fn(),
stdout: { on: vi.fn((event, callback) => listeners.set(`stdout:${event}`, callback)) },
stderr: { on: vi.fn((event, callback) => listeners.set(`stderr:${event}`, callback)) },
stdin: { end: vi.fn() },
on: vi.fn((event, callback) => listeners.set(event, callback))
} as never)
const pullRequest = generatePullRequestFieldsFromContext(
{
branch: 'feature/pr-fields',
base: 'main',
branchChangedByPreparation: true,
currentTitle: '',
currentBody: '',
currentDraft: false,
commitSummary: '- feat: update README',
changeSummary: 'M\tREADME.md',
patch: '+hello'
},
{
agentId: 'custom',
model: '',
customAgentCommand: 'agent'
},
{
kind: 'local',
cwd: '/repo'
}
)
listeners.get('stdout:data')?.(Buffer.from('not json'))
listeners.get('close')?.(0)
await expect(pullRequest).resolves.toEqual({
success: false,
error: 'Generated pull request details could not be parsed.',
branchChangedByPreparation: true
})
})
it('reports branch changes when pull request generation is canceled', async () => {
const listeners = new Map<string, (value: unknown) => void>()
const child = {
pid: 123,
kill: vi.fn(),
stdout: { on: vi.fn((event, callback) => listeners.set(`stdout:${event}`, callback)) },
stderr: { on: vi.fn((event, callback) => listeners.set(`stderr:${event}`, callback)) },
stdin: { end: vi.fn() },
on: vi.fn((event, callback) => listeners.set(event, callback))
}
spawnMock.mockReturnValue(child as never)
const pullRequest = generatePullRequestFieldsFromContext(
{
branch: 'feature/pr-fields',
base: 'main',
branchChangedByPreparation: true,
currentTitle: '',
currentBody: '',
currentDraft: false,
commitSummary: '- feat: update README',
changeSummary: 'M\tREADME.md',
patch: '+hello'
},
{
agentId: 'custom',
model: '',
customAgentCommand: 'agent'
},
{
kind: 'local',
cwd: '/repo'
}
)
cancelGeneratePullRequestFieldsLocal('/repo')
listeners.get('close')?.(null)
expect(child.kill).toHaveBeenCalledWith('SIGKILL')
await expect(pullRequest).resolves.toEqual({
success: false,
error: 'Generation canceled.',
canceled: true,
branchChangedByPreparation: true
})
})
it('routes Windows batch-script agent commands through cmd.exe', async () => {
const originalComSpec = process.env.ComSpec
process.env.ComSpec = 'C:\\Windows\\System32\\cmd.exe'

29
src/main/text-generation/commit-message-text-generation.ts
View File

@ -67,8 +67,13 @@ export type DiscoverCommitMessageModelsResult =
| { success: false; error: string }
export type GeneratePullRequestFieldsResult =
| { success: true; fields: GeneratedPullRequestFields; agentLabel?: string }
| { success: false; error: string; canceled?: boolean }
| {
success: true
fields: GeneratedPullRequestFields
agentLabel?: string
branchChangedByPreparation?: boolean
}
| { success: false; error: string; canceled?: boolean; branchChangedByPreparation?: boolean }
export type RemoteCommitMessageExecResult = {
stdout: string
@ -755,16 +760,24 @@ function formatPullRequestFieldsGenerationResult(
context: PullRequestDraftContext
): GeneratePullRequestFieldsResult {
if (!result.success) {
return result
return {
...result,
branchChangedByPreparation: context.branchChangedByPreparation
}
}
try {
return {
success: true,
fields: parseGeneratedPullRequestFields(result.rawOutput, context),
agentLabel: result.agentLabel
agentLabel: result.agentLabel,
branchChangedByPreparation: context.branchChangedByPreparation
}
} catch {
return { success: false, error: 'Generated pull request details could not be parsed.' }
return {
success: false,
error: 'Generated pull request details could not be parsed.',
branchChangedByPreparation: context.branchChangedByPreparation
}
}
}
@ -776,7 +789,11 @@ export async function generatePullRequestFieldsFromContext(
const prompt = buildPullRequestFieldsPrompt(context, params.customPrompt ?? '')
const planned = planCommitMessageGeneration(params, prompt)
if (!planned.ok) {
return { success: false, error: planned.error }
return {
success: false,
error: planned.error,
branchChangedByPreparation: context.branchChangedByPreparation
}
}
const internalResult =

167
src/main/text-generation/pull-request-context.test.ts Normal file
View File

@ -0,0 +1,167 @@
import { describe, expect, it, vi } from 'vitest'
import { getPullRequestDraftContext } from './pull-request-context'
type GitExec = Parameters<typeof getPullRequestDraftContext>[0]
function createContextInput(base = 'main') {
return {
base,
currentTitle: 'Existing title',
currentBody: 'Existing body',
currentDraft: false
}
}
describe('getPullRequestDraftContext', () => {
it('fetches and rebases onto the resolved remote base before collecting PR context', async () => {
const execGit = vi.fn<GitExec>(async (args) => {
if (args[0] === 'fetch') {
return { stdout: '', stderr: '' }
}
if (args[0] === 'for-each-ref') {
return { stdout: 'origin/HEAD\norigin/main\nupstream/main\n', stderr: '' }
}
if (args[0] === 'rebase') {
return { stdout: 'Current branch feature is up to date.\n', stderr: '' }
}
if (args[0] === 'rev-parse') {
return { stdout: 'unchanged-head\n', stderr: '' }
}
if (args[0] === 'branch') {
return { stdout: 'feature/pr-details\n', stderr: '' }
}
if (args[0] === 'merge-base') {
return { stdout: 'abc123\n', stderr: '' }
}
if (args[0] === 'log') {
return { stdout: '- feat: summarize branch\n', stderr: '' }
}
if (args[0] === 'diff' && args[1] === '--name-status') {
return { stdout: 'M\tsrc/file.ts\n', stderr: '' }
}
if (args[0] === 'diff') {
return { stdout: 'diff --git a/src/file.ts b/src/file.ts\n+change\n', stderr: '' }
}
throw new Error(`Unexpected git args: ${args.join(' ')}`)
})
const context = await getPullRequestDraftContext(execGit, createContextInput())
expect(context).toMatchObject({
branch: 'feature/pr-details',
base: 'main',
branchChangedByPreparation: false,
commitSummary: '- feat: summarize branch',
changeSummary: 'M\tsrc/file.ts'
})
expect(execGit).toHaveBeenCalledWith(['fetch', '--all', '--prune'], expect.any(Object))
expect(execGit).toHaveBeenCalledWith(['rebase', 'origin/main'], expect.any(Object))
expect(execGit).toHaveBeenCalledWith(['merge-base', 'origin/main', 'HEAD'], expect.any(Object))
const commandNames = execGit.mock.calls.map(([args]) => args[0])
expect(commandNames.indexOf('rebase')).toBeLessThan(commandNames.indexOf('merge-base'))
})
it('reports when preparation changes HEAD', async () => {
let revParseCount = 0
const execGit = vi.fn<GitExec>(async (args) => {
if (args[0] === 'fetch' || args[0] === 'rebase') {
return { stdout: '', stderr: '' }
}
if (args[0] === 'for-each-ref') {
return { stdout: 'origin/main\n', stderr: '' }
}
if (args[0] === 'rev-parse') {
revParseCount += 1
return { stdout: `${revParseCount === 1 ? 'old-head' : 'new-head'}\n`, stderr: '' }
}
if (args[0] === 'branch') {
return { stdout: 'feature\n', stderr: '' }
}
if (args[0] === 'merge-base') {
return { stdout: 'abc123\n', stderr: '' }
}
if (args[0] === 'log') {
return { stdout: '- feat: change\n', stderr: '' }
}
if (args[0] === 'diff') {
return { stdout: 'M\tREADME.md\n', stderr: '' }
}
throw new Error(`Unexpected git args: ${args.join(' ')}`)
})
const context = await getPullRequestDraftContext(execGit, createContextInput())
expect(context?.branchChangedByPreparation).toBe(true)
})
it('keeps a remote-qualified base when the selected base includes the remote', async () => {
const execGit = vi.fn<GitExec>(async (args) => {
if (args[0] === 'fetch' || args[0] === 'rebase') {
return { stdout: '', stderr: '' }
}
if (args[0] === 'for-each-ref') {
return { stdout: 'origin/main\nupstream/main\n', stderr: '' }
}
if (args[0] === 'branch') {
return { stdout: 'feature\n', stderr: '' }
}
if (args[0] === 'rev-parse') {
return { stdout: 'abc123\n', stderr: '' }
}
if (args[0] === 'merge-base') {
return { stdout: 'abc123\n', stderr: '' }
}
if (args[0] === 'log') {
return { stdout: '- feat: change\n', stderr: '' }
}
if (args[0] === 'diff') {
return { stdout: 'M\tREADME.md\n', stderr: '' }
}
return { stdout: '', stderr: '' }
})
await getPullRequestDraftContext(execGit, createContextInput('upstream/main'))
expect(execGit).toHaveBeenCalledWith(['rebase', 'upstream/main'], expect.any(Object))
expect(execGit).toHaveBeenCalledWith(
['merge-base', 'upstream/main', 'HEAD'],
expect.any(Object)
)
})
it('stops generation when the rebase fails', async () => {
const execGit = vi.fn<GitExec>(async (args) => {
if (args[0] === 'fetch') {
return { stdout: '', stderr: '' }
}
if (args[0] === 'for-each-ref') {
return { stdout: 'origin/main\n', stderr: '' }
}
if (args[0] === 'rev-parse') {
return { stdout: 'abc123\n', stderr: '' }
}
if (args[0] === 'rebase') {
throw new Error('Command failed: git rebase origin/main\nCONFLICT (content): README.md')
}
throw new Error(`Unexpected git args: ${args.join(' ')}`)
})
await expect(getPullRequestDraftContext(execGit, createContextInput())).rejects.toThrow(
'Rebase before generating PR details failed: CONFLICT (content): README.md'
)
expect(execGit).not.toHaveBeenCalledWith(
['merge-base', 'origin/main', 'HEAD'],
expect.anything()
)
})
it('returns null without running git when the base is invalid', async () => {
const execGit = vi.fn<GitExec>()
await expect(getPullRequestDraftContext(execGit, createContextInput('--main'))).resolves.toBe(
null
)
expect(execGit).not.toHaveBeenCalled()
})
})

77
src/main/text-generation/pull-request-context.ts
View File

@ -23,6 +23,79 @@ async function safeExec(execGit: GitExec, args: string[]): Promise<string> {
}
}
function summarizeGitError(error: unknown): string {
if (!(error instanceof Error)) {
return 'Git command failed.'
}
const lines = error.message
.split('\n')
.map((line) => line.trim())
.filter(Boolean)
return lines.at(-1) ?? error.message
}
async function requiredExec(execGit: GitExec, args: string[], label: string): Promise<string> {
try {
const { stdout } = await execGit(args, { maxBuffer: MAX_PULL_REQUEST_CONTEXT_BYTES })
return stdout.trim()
} catch (error) {
throw new Error(`${label}: ${summarizeGitError(error)}`)
}
}
async function resolveComparisonBase(execGit: GitExec, base: string): Promise<string> {
const refs = (
await safeExec(execGit, ['for-each-ref', '--format=%(refname:short)', 'refs/remotes'])
)
.split('\n')
.map((line) => line.trim())
.filter((line) => line && !line.endsWith('/HEAD'))
if (refs.includes(base)) {
return base
}
const preferredRemoteRefs = [`origin/${base}`, `upstream/${base}`]
for (const ref of preferredRemoteRefs) {
if (refs.includes(ref)) {
return ref
}
}
return refs.find((ref) => ref.endsWith(`/${base}`)) ?? base
}
type PullRequestBranchPreparation = {
comparisonBase: string
branchChanged: boolean
}
async function preparePullRequestBranch(
execGit: GitExec,
base: string
): Promise<PullRequestBranchPreparation> {
await requiredExec(
execGit,
['fetch', '--all', '--prune'],
'Fetch before generating PR details failed'
)
const comparisonBase = await resolveComparisonBase(execGit, base)
const headBeforeRebase = await safeExec(execGit, ['rev-parse', 'HEAD'])
// Why: GitHub PR diffs are three-dot based; rebasing first keeps already-landed
// branch changes from bleeding into the generated description.
await requiredExec(
execGit,
['rebase', comparisonBase],
'Rebase before generating PR details failed'
)
const headAfterRebase = await safeExec(execGit, ['rev-parse', 'HEAD'])
return {
comparisonBase,
branchChanged:
Boolean(headBeforeRebase) && Boolean(headAfterRebase) && headBeforeRebase !== headAfterRebase
}
}
export async function getPullRequestDraftContext(
execGit: GitExec,
input: PullRequestContextInput
@ -32,9 +105,10 @@ export async function getPullRequestDraftContext(
return null
}
const { comparisonBase, branchChanged } = await preparePullRequestBranch(execGit, base)
const [branch, mergeBase] = await Promise.all([
safeExec(execGit, ['branch', '--show-current']),
safeExec(execGit, ['merge-base', base, 'HEAD'])
safeExec(execGit, ['merge-base', comparisonBase, 'HEAD'])
])
if (!mergeBase) {
return null
@ -54,6 +128,7 @@ export async function getPullRequestDraftContext(
return {
branch: branch || null,
base,
branchChangedByPreparation: branchChanged,
currentTitle: input.currentTitle,
currentBody: input.currentBody,
currentDraft: input.currentDraft,

12
src/renderer/src/components/right-sidebar/ChecksPanel.tsx
View File

@ -747,6 +747,17 @@ export default function ChecksPanel(): React.JSX.Element {
}
}, [activeWorktree, activeWorktreeId, fetchUpstreamStatus, pushBranch])
const handleBranchChangedByPullRequestGeneration = useCallback(async (): Promise<void> => {
if (!activeWorktreeId || !activeWorktree?.path) {
return
}
// Why: AI PR detail generation rebases before summarizing; if HEAD moved,
// the dialog must push before creating from the refreshed branch state.
setCreatePrPushFirst(true)
const connectionId = getConnectionId(activeWorktreeId) ?? undefined
await fetchUpstreamStatus(activeWorktreeId, activeWorktree.path, connectionId)
}, [activeWorktree?.path, activeWorktreeId, fetchUpstreamStatus])
const handlePullRequestCreated = useCallback(
async (result: { number: number; url: string }): Promise<void> => {
if (!repo || !branch) {
@ -940,6 +951,7 @@ export default function ChecksPanel(): React.JSX.Element {
pushBeforeCreate={createPrPushFirst}
onOpenChange={setCreatePrDialogOpen}
onPushBeforeCreate={pushBeforeCreatePullRequest}
onBranchChangedByGeneration={handleBranchChangedByPullRequestGeneration}
onCreated={handlePullRequestCreated}
/>
)}

5
src/renderer/src/components/right-sidebar/CreatePullRequestDialog.tsx
View File

@ -33,6 +33,7 @@ type CreatePullRequestDialogProps = {
pushBeforeCreate: boolean
onOpenChange: (open: boolean) => void
onPushBeforeCreate: () => Promise<boolean>
onBranchChangedByGeneration: () => Promise<void>
onCreated: (result: { number: number; url: string }) => Promise<void>
}
@ -57,6 +58,7 @@ export function CreatePullRequestDialog({
pushBeforeCreate,
onOpenChange,
onPushBeforeCreate,
onBranchChangedByGeneration,
onCreated
}: CreatePullRequestDialogProps): React.JSX.Element {
const settings = useAppStore((s) => s.settings)
@ -93,7 +95,8 @@ export function CreatePullRequestDialog({
branch,
eligibility,
settings,
submitting
submitting,
onBranchChangedByGeneration
})
useEffect(() => {

8
src/renderer/src/components/right-sidebar/SourceControl.tsx
View File

@ -1575,6 +1575,13 @@ function SourceControlInner(): React.JSX.Element {
worktreePath
])
const handleBranchChangedByPullRequestGeneration = useCallback(async (): Promise<void> => {
// Why: AI PR detail generation rebases before summarizing; if HEAD moved,
// the dialog must not create a PR from stale push/create eligibility.
setCreatePrPushFirst(true)
await refreshActiveGitStatusAfterMutation()
}, [refreshActiveGitStatusAfterMutation])
const handlePullRequestCreated = useCallback(
async (result: { number: number; url: string }): Promise<void> => {
if (!activeRepo || !branchName) {
@ -2748,6 +2755,7 @@ function SourceControlInner(): React.JSX.Element {
pushBeforeCreate={createPrPushFirst}
onOpenChange={setCreatePrDialogOpen}
onPushBeforeCreate={pushBeforeCreatePullRequest}
onBranchChangedByGeneration={handleBranchChangedByPullRequestGeneration}
onCreated={handlePullRequestCreated}
/>
<div ref={sourceControlRef} className="relative flex h-full flex-col overflow-hidden">

23
src/renderer/src/components/right-sidebar/useCreatePullRequestDialogFields.ts
View File

@ -1,3 +1,5 @@
/* eslint-disable max-lines -- Why: field state, base search, AI generation,
and cancellation share request guards that need to stay in one hook. */
import { useCallback, useEffect, useRef, useState } from 'react'
import { getConnectionId } from '@/lib/connection-context'
import { useAppStore, type AppState } from '@/store'
@ -25,6 +27,7 @@ type UseCreatePullRequestDialogFieldsOptions = {
eligibility: HostedReviewCreationEligibility | null
settings: AppState['settings']
submitting: boolean
onBranchChangedByGeneration?: () => Promise<void>
}
type GenerationSeed = {
@ -47,7 +50,8 @@ export function useCreatePullRequestDialogFields({
branch,
eligibility,
settings,
submitting
submitting,
onBranchChangedByGeneration
}: UseCreatePullRequestDialogFieldsOptions) {
const commitMessageAi = settings?.commitMessageAi
const effectiveCommitMessageAgentId = resolveCommitMessageAgentChoice(
@ -208,7 +212,11 @@ export function useCreatePullRequestDialogFields({
draft
}
)
if (generationRequestIdRef.current !== requestId) {
if (result.branchChangedByPreparation) {
await onBranchChangedByGeneration?.()
}
const isCurrentRequest = generationRequestIdRef.current === requestId
if (!isCurrentRequest) {
return
}
if (!result.success) {
@ -254,7 +262,16 @@ export function useCreatePullRequestDialogFields({
setGenerating(false)
}
}
}, [base, body, draft, generateDisabled, title, worktreeId, worktreePath])
}, [
base,
body,
draft,
generateDisabled,
onBranchChangedByGeneration,
title,
worktreeId,
worktreePath
])
const handleCancelGenerate = useCallback((): void => {
if (!worktreePath || !generateInFlightRef.current) {

3
src/renderer/src/runtime/runtime-git-client.ts
View File

@ -28,8 +28,9 @@ export type RuntimeGeneratePullRequestFieldsResult =
success: true
fields: { base: string; title: string; body: string; draft: boolean }
agentLabel?: string
branchChangedByPreparation?: boolean
}
| { success: false; error: string; canceled?: boolean }
| { success: false; error: string; canceled?: boolean; branchChangedByPreparation?: boolean }
type RuntimeGitSettings = Pick<GlobalSettings, 'activeRuntimeEnvironmentId'> &
Partial<Pick<GlobalSettings, 'commitMessageAi' | 'agentCmdOverrides' | 'enableGitHubAttribution'>>

1
src/shared/pull-request-generation.test.ts
View File

@ -8,6 +8,7 @@ import {
const context: PullRequestDraftContext = {
branch: 'feature/pr-details',
base: 'main',
branchChangedByPreparation: false,
currentTitle: 'Feature pr details',
currentBody: '- Add form',
currentDraft: false,

1
src/shared/pull-request-generation.ts
View File

@ -3,6 +3,7 @@ import { truncateDiffForPrompt } from './commit-message-prompt'
export type PullRequestDraftContext = {
branch: string | null
base: string
branchChangedByPreparation: boolean
currentTitle: string
currentBody: string
currentDraft: boolean