Create PRs from Source Control (#2478)
* Create PRs directly from Source Control - Replace the modal flow with an inline PR composer in the sidebar - Keep PR creation state and validation scoped per worktree - Rename the recovery action to clarify it only pushes before creating PRs * Clean up fork PR remotes after worktree deletion - Track Orca-created push target remotes in worktree metadata - Reuse ownership markers when later worktrees share the same fork remote - Fetch only the selected PR base instead of every remote before drafting PRs - Mirror local branch cleanup for SSH worktree deletion * Stabilize pull request creation flow - Keep PR actions and composer fields locked while generation or creation is in flight - Refresh git status, branch comparison, and history after remote actions settle - Disable push-only actions on diverged branches so users sync first * Make PR context generation read-only - Stop rebasing or probing HEAD before collecting PR draft context - Allow git operations on known repo roots without refreshing worktree cache Co-authored-by: Orca <help@stably.ai> * fix: address review findings --------- Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinjing
GitHub
parent
c99633e612
commit
aeaf0dece4
|
|
@ -70,6 +70,21 @@ describe('git remote operations', () => {
|
|||
])
|
||||
})
|
||||
|
||||
it('passes --force-with-lease when requested', async () => {
|
||||
gitExecFileAsyncMock
|
||||
.mockResolvedValueOnce({ stdout: 'feature\n', stderr: '' })
|
||||
.mockResolvedValueOnce({ stdout: 'origin\n', stderr: '' })
|
||||
.mockResolvedValueOnce({ stdout: 'refs/heads/feature\n', stderr: '' })
|
||||
.mockResolvedValueOnce({ stdout: '', stderr: '' })
|
||||
|
||||
await gitPush('/repo', false, undefined, { forceWithLease: true })
|
||||
|
||||
expect(gitExecFileAsyncMock).toHaveBeenLastCalledWith(
|
||||
['push', '--force-with-lease', '--set-upstream', 'origin', 'HEAD:feature'],
|
||||
{ cwd: '/repo' }
|
||||
)
|
||||
})
|
||||
|
||||
it('maps non-fast-forward push failures to an actionable message', async () => {
|
||||
gitExecFileAsyncMock
|
||||
.mockRejectedValueOnce(new Error('no branch'))
|
||||
|
|
|
|||
|
|
@ -42,7 +42,8 @@ function explicitPushTarget(target: GitPushTarget): { remote: string; refspec: s
|
|||
export async function gitPush(
|
||||
worktreePath: string,
|
||||
_publish = false,
|
||||
pushTarget?: GitPushTarget
|
||||
pushTarget?: GitPushTarget,
|
||||
options: { forceWithLease?: boolean } = {}
|
||||
): Promise<void> {
|
||||
try {
|
||||
if (pushTarget) {
|
||||
|
|
@ -61,9 +62,12 @@ export async function gitPush(
|
|||
const target = pushTarget
|
||||
? explicitPushTarget(pushTarget)
|
||||
: await getConfiguredPushTarget(worktreePath)
|
||||
const args = target
|
||||
? ['push', '--set-upstream', target.remote, target.refspec]
|
||||
: ['push', '--set-upstream', 'origin', 'HEAD']
|
||||
const args = [
|
||||
'push',
|
||||
...(options.forceWithLease ? ['--force-with-lease'] : []),
|
||||
'--set-upstream',
|
||||
...(target ? [target.remote, target.refspec] : ['origin', 'HEAD'])
|
||||
]
|
||||
await gitExecFileAsync(args, { cwd: worktreePath })
|
||||
} catch (error) {
|
||||
throw new Error(normalizeGitErrorMessage(error, 'push'))
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ describe('getUpstreamStatus', () => {
|
|||
gitExecFileAsyncMock
|
||||
.mockResolvedValueOnce({ stdout: 'origin/main\n' })
|
||||
.mockResolvedValueOnce({ stdout: '2\t3\n' })
|
||||
.mockResolvedValueOnce({ stdout: '+ abc123 remote work\n' })
|
||||
|
||||
const result = await getUpstreamStatus('/repo')
|
||||
|
||||
|
|
@ -32,7 +33,29 @@ describe('getUpstreamStatus', () => {
|
|||
hasUpstream: true,
|
||||
upstreamName: 'origin/main',
|
||||
ahead: 2,
|
||||
behind: 3
|
||||
behind: 3,
|
||||
behindCommitsArePatchEquivalent: false
|
||||
})
|
||||
})
|
||||
|
||||
it('marks diverged upstream commits as patch-equivalent after a rebase', async () => {
|
||||
gitExecFileAsyncMock
|
||||
.mockResolvedValueOnce({ stdout: 'origin/feature\n' })
|
||||
.mockResolvedValueOnce({ stdout: '14\t3\n' })
|
||||
.mockResolvedValueOnce({
|
||||
stdout:
|
||||
'= ac503deae Stabilize pull request creation flow\n' +
|
||||
'= 7dc0fc1a6 Clean up fork PR remotes after worktree deletion\n'
|
||||
})
|
||||
|
||||
const result = await getUpstreamStatus('/repo')
|
||||
|
||||
expect(result).toEqual({
|
||||
hasUpstream: true,
|
||||
upstreamName: 'origin/feature',
|
||||
ahead: 14,
|
||||
behind: 3,
|
||||
behindCommitsArePatchEquivalent: true
|
||||
})
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -1,7 +1,22 @@
|
|||
import type { GitUpstreamStatus } from '../../shared/types'
|
||||
import { upstreamOnlyCommitsArePatchEquivalent } from '../../shared/git-upstream-status'
|
||||
import { isNoUpstreamError, normalizeGitErrorMessage } from '../../shared/git-remote-error'
|
||||
import { gitExecFileAsync } from './runner'
|
||||
|
||||
async function getBehindCommitsArePatchEquivalent(worktreePath: string): Promise<boolean> {
|
||||
try {
|
||||
const { stdout } = await gitExecFileAsync(
|
||||
['log', '--oneline', '--cherry-mark', '--right-only', 'HEAD...@{u}', '--'],
|
||||
{ cwd: worktreePath }
|
||||
)
|
||||
return upstreamOnlyCommitsArePatchEquivalent(stdout)
|
||||
} catch {
|
||||
// Why: patch-equivalence is an optimization for the rebase case. If the
|
||||
// probe fails, keep the conservative pull-first behavior.
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
export async function getUpstreamStatus(worktreePath: string): Promise<GitUpstreamStatus> {
|
||||
try {
|
||||
const { stdout: upstreamStdout } = await gitExecFileAsync(
|
||||
|
|
@ -35,11 +50,15 @@ export async function getUpstreamStatus(worktreePath: string): Promise<GitUpstre
|
|||
throw new Error(`Unparseable git rev-list counts: ${JSON.stringify(countsStdout)}`)
|
||||
}
|
||||
|
||||
const behindCommitsArePatchEquivalent =
|
||||
ahead > 0 && behind > 0 ? await getBehindCommitsArePatchEquivalent(worktreePath) : undefined
|
||||
|
||||
return {
|
||||
hasUpstream: true,
|
||||
upstreamName,
|
||||
ahead,
|
||||
behind
|
||||
behind,
|
||||
...(behindCommitsArePatchEquivalent !== undefined ? { behindCommitsArePatchEquivalent } : {})
|
||||
}
|
||||
} catch (error) {
|
||||
// Why: we only swallow clearly-no-upstream signals — that's an expected
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ import { resolve, relative, dirname, basename, isAbsolute } from 'path'
|
|||
import { realpathSync } from 'fs'
|
||||
import { realpath } from 'fs/promises'
|
||||
import type { Store } from '../persistence'
|
||||
import { listRepoWorktrees } from '../repo-worktrees'
|
||||
import { isRepoRoot, listRepoWorktrees } from '../repo-worktrees'
|
||||
|
||||
export const PATH_ACCESS_DENIED_MESSAGE =
|
||||
'Access denied: path resolves outside allowed directories. If this blocks a legitimate workflow, please file a GitHub issue.'
|
||||
|
|
@ -271,12 +271,9 @@ async function isPathAllowedIncludingRegisteredWorktrees(
|
|||
/**
|
||||
* Resolve and verify that a worktree path belongs to a registered repo.
|
||||
*
|
||||
* Why this doesn't use resolveAuthorizedPath: linked worktrees can live
|
||||
* anywhere on disk (e.g. ~/.codex/worktrees/), far outside the repo root
|
||||
* and workspaceDir that resolveAuthorizedPath allows. The security boundary
|
||||
* for git operations is *worktree registration* — the path must match a
|
||||
* worktree reported by `git worktree list` for a known repo — not
|
||||
* directory containment within allowed roots.
|
||||
* Why this doesn't use resolveAuthorizedPath: linked worktrees can live outside
|
||||
* repo/workspace roots. Git operations trust exact worktree registration from
|
||||
* `git worktree list`, not directory containment.
|
||||
*/
|
||||
export async function resolveRegisteredWorktreePath(
|
||||
worktreePath: string,
|
||||
|
|
@ -289,8 +286,7 @@ export async function resolveRegisteredWorktreePath(
|
|||
}
|
||||
|
||||
const resolvedTarget = resolve(worktreePath)
|
||||
|
||||
if (registeredWorktreeRoots.has(resolvedTarget)) {
|
||||
if (registeredWorktreeRoots.has(resolvedTarget) || isRepoRoot(store.getRepos(), resolvedTarget)) {
|
||||
return resolvedTarget
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -510,6 +510,18 @@ describe('registerFilesystemHandlers', () => {
|
|||
expect(getStatusMock).toHaveBeenCalledWith(WORKTREE_FEATURE_PATH, { includeIgnored: false })
|
||||
})
|
||||
|
||||
it('allows git operations on the known repo root without rebuilding the worktree cache', async () => {
|
||||
getStatusMock.mockResolvedValue({ entries: [] })
|
||||
|
||||
registerFilesystemHandlers(store as never)
|
||||
|
||||
await handlers.get('git:status')!(null, { worktreePath: REPO_PATH })
|
||||
|
||||
expect(listWorktreesMock).not.toHaveBeenCalled()
|
||||
expect(realpathMock).not.toHaveBeenCalledWith(REPO_PATH)
|
||||
expect(getStatusMock).toHaveBeenCalledWith(REPO_PATH, { includeIgnored: false })
|
||||
})
|
||||
|
||||
it('forwards includeIgnored through local and SSH git status IPC', async () => {
|
||||
registerWorktreeRootsForRepo(store as never, 'repo-1', [REPO_PATH, WORKTREE_FEATURE_PATH])
|
||||
getStatusMock.mockResolvedValue({ entries: [], conflictOperation: 'unknown' })
|
||||
|
|
|
|||
|
|
@ -930,6 +930,7 @@ export function registerFilesystemHandlers(
|
|||
args: {
|
||||
worktreePath: string
|
||||
publish?: boolean
|
||||
forceWithLease?: boolean
|
||||
connectionId?: string
|
||||
pushTarget?: GitPushTarget
|
||||
}
|
||||
|
|
@ -946,13 +947,17 @@ export function registerFilesystemHandlers(
|
|||
if (!provider) {
|
||||
throw new Error(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE)
|
||||
}
|
||||
return provider.pushBranch(args.worktreePath, publish, args.pushTarget)
|
||||
return provider.pushBranch(args.worktreePath, publish, args.pushTarget, {
|
||||
forceWithLease: args.forceWithLease === true
|
||||
})
|
||||
}
|
||||
const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store)
|
||||
if (args.pushTarget) {
|
||||
await validateGitPushTarget(worktreePath, args.pushTarget)
|
||||
}
|
||||
await gitPush(worktreePath, publish, args.pushTarget)
|
||||
await gitPush(worktreePath, publish, args.pushTarget, {
|
||||
forceWithLease: args.forceWithLease === true
|
||||
})
|
||||
}
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -44,6 +44,7 @@ import {
|
|||
mergeWorktree,
|
||||
areWorktreePathsEqual
|
||||
} from './worktree-logic'
|
||||
import { getRepoIdFromWorktreeId } from '../../shared/worktree-id'
|
||||
import { invalidateAuthorizedRootsCache } from './filesystem-auth'
|
||||
import { createWorktreeSymlinks } from './worktree-symlinks'
|
||||
import { normalizeSparseDirectories } from './sparse-checkout-directories'
|
||||
|
|
@ -140,17 +141,34 @@ async function ensureUniqueRemoteName(repoPath: string, preferred: string): Prom
|
|||
|
||||
export async function prepareWorktreePushTarget(
|
||||
repoPath: string,
|
||||
target: GitPushTarget
|
||||
target: GitPushTarget,
|
||||
store?: WorktreePushTargetStore,
|
||||
repoId?: string
|
||||
): Promise<GitPushTarget> {
|
||||
await validateGitPushTarget(repoPath, target)
|
||||
const { remoteCreated: _ignoredRemoteCreated, ...sanitizedTarget } = target
|
||||
let remoteName = target.remoteName
|
||||
let remoteCreated = false
|
||||
if (target.remoteUrl) {
|
||||
const existingRemote = await findRemoteForUrl(repoPath, target.remoteUrl)
|
||||
if (existingRemote) {
|
||||
remoteName = existingRemote
|
||||
// Why: if a later PR worktree reuses an Orca-created fork remote, it
|
||||
// must inherit ownership so deleting the final user can remove it.
|
||||
remoteCreated = store
|
||||
? isPushTargetRemoteCreatedByKnownWorktree(
|
||||
store,
|
||||
{
|
||||
...target,
|
||||
remoteName: existingRemote
|
||||
},
|
||||
repoId
|
||||
)
|
||||
: false
|
||||
} else {
|
||||
remoteName = await ensureUniqueRemoteName(repoPath, target.remoteName)
|
||||
await gitExecFileAsync(['remote', 'add', remoteName, target.remoteUrl], { cwd: repoPath })
|
||||
remoteCreated = true
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -163,8 +181,152 @@ export async function prepareWorktreePushTarget(
|
|||
{ cwd: repoPath }
|
||||
)
|
||||
return {
|
||||
...target,
|
||||
remoteName
|
||||
...sanitizedTarget,
|
||||
remoteName,
|
||||
...(remoteCreated ? { remoteCreated: true } : {})
|
||||
}
|
||||
}
|
||||
|
||||
type GitRemoteExec = (args: string[], cwd: string) => Promise<{ stdout: string; stderr?: string }>
|
||||
type WorktreePushTargetStore = Pick<Store, 'getAllWorktreeMeta'>
|
||||
|
||||
function sameGitHubRemoteUrl(left: string, right: string): boolean {
|
||||
if (left === right) {
|
||||
return true
|
||||
}
|
||||
const parsedLeft = parseGitHubOwnerRepo(left)
|
||||
const parsedRight = parseGitHubOwnerRepo(right)
|
||||
return Boolean(
|
||||
parsedLeft &&
|
||||
parsedRight &&
|
||||
parsedLeft.owner.toLowerCase() === parsedRight.owner.toLowerCase() &&
|
||||
parsedLeft.repo.toLowerCase() === parsedRight.repo.toLowerCase()
|
||||
)
|
||||
}
|
||||
|
||||
function isPushTargetUsedByAnotherWorktree(
|
||||
store: WorktreePushTargetStore,
|
||||
removedWorktreeId: string,
|
||||
target: GitPushTarget
|
||||
): boolean {
|
||||
const removedRepoId = getRepoIdFromWorktreeId(removedWorktreeId)
|
||||
return Object.entries(store.getAllWorktreeMeta()).some(([worktreeId, meta]) => {
|
||||
// Why: git remotes are repo-local; matching metadata from another repo
|
||||
// must not pin this repo's fork remote forever.
|
||||
const belongsToSameRepo = getRepoIdFromWorktreeId(worktreeId) === removedRepoId
|
||||
if (worktreeId === removedWorktreeId || !belongsToSameRepo || !meta.pushTarget) {
|
||||
return false
|
||||
}
|
||||
const otherRemoteUrl = meta.pushTarget.remoteUrl
|
||||
const targetRemoteUrl = target.remoteUrl
|
||||
return (
|
||||
meta.pushTarget.remoteName === target.remoteName ||
|
||||
(typeof otherRemoteUrl === 'string' &&
|
||||
typeof targetRemoteUrl === 'string' &&
|
||||
sameGitHubRemoteUrl(otherRemoteUrl, targetRemoteUrl))
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
function isPushTargetRemoteCreatedByKnownWorktree(
|
||||
store: WorktreePushTargetStore,
|
||||
target: GitPushTarget,
|
||||
repoId?: string
|
||||
): boolean {
|
||||
return Object.entries(store.getAllWorktreeMeta()).some(([worktreeId, meta]) => {
|
||||
if (repoId && getRepoIdFromWorktreeId(worktreeId) !== repoId) {
|
||||
return false
|
||||
}
|
||||
if (!meta.pushTarget?.remoteCreated) {
|
||||
return false
|
||||
}
|
||||
const otherRemoteUrl = meta.pushTarget.remoteUrl
|
||||
const targetRemoteUrl = target.remoteUrl
|
||||
return (
|
||||
meta.pushTarget.remoteName === target.remoteName ||
|
||||
(typeof otherRemoteUrl === 'string' &&
|
||||
typeof targetRemoteUrl === 'string' &&
|
||||
sameGitHubRemoteUrl(otherRemoteUrl, targetRemoteUrl))
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
async function hasBranchConfigUsingRemote(
|
||||
execGit: GitRemoteExec,
|
||||
repoPath: string,
|
||||
target: GitPushTarget
|
||||
): Promise<boolean> {
|
||||
try {
|
||||
const { stdout } = await execGit(
|
||||
['config', '--get-regexp', '^branch\\..*\\.(remote|pushRemote)$'],
|
||||
repoPath
|
||||
)
|
||||
return stdout
|
||||
.split(/\r?\n/)
|
||||
.map((line) => line.trim())
|
||||
.filter(Boolean)
|
||||
.some((line) => {
|
||||
const value = line.split(/\s+/).slice(1).join(' ')
|
||||
return value === target.remoteName || value === target.remoteUrl
|
||||
})
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
async function cleanupUnusedWorktreePushTargetRemoteWithExec(
|
||||
repoPath: string,
|
||||
removedWorktreeId: string,
|
||||
target: GitPushTarget | undefined,
|
||||
store: WorktreePushTargetStore,
|
||||
execGit: GitRemoteExec
|
||||
): Promise<void> {
|
||||
if (
|
||||
!target?.remoteCreated ||
|
||||
!target.remoteUrl ||
|
||||
target.remoteName === 'origin' ||
|
||||
target.remoteName === 'upstream'
|
||||
) {
|
||||
return
|
||||
}
|
||||
if (isPushTargetUsedByAnotherWorktree(store, removedWorktreeId, target)) {
|
||||
return
|
||||
}
|
||||
if (await hasBranchConfigUsingRemote(execGit, repoPath, target)) {
|
||||
return
|
||||
}
|
||||
|
||||
let configuredRemoteUrl: string
|
||||
try {
|
||||
configuredRemoteUrl = (
|
||||
await execGit(['remote', 'get-url', target.remoteName], repoPath)
|
||||
).stdout.trim()
|
||||
} catch {
|
||||
return
|
||||
}
|
||||
if (!sameGitHubRemoteUrl(configuredRemoteUrl, target.remoteUrl)) {
|
||||
return
|
||||
}
|
||||
|
||||
await execGit(['remote', 'remove', target.remoteName], repoPath)
|
||||
}
|
||||
|
||||
export async function cleanupUnusedWorktreePushTargetRemote(
|
||||
repoPath: string,
|
||||
removedWorktreeId: string,
|
||||
target: GitPushTarget | undefined,
|
||||
store: WorktreePushTargetStore
|
||||
): Promise<void> {
|
||||
try {
|
||||
await cleanupUnusedWorktreePushTargetRemoteWithExec(
|
||||
repoPath,
|
||||
removedWorktreeId,
|
||||
target,
|
||||
store,
|
||||
(args, cwd) => gitExecFileAsync(args, { cwd })
|
||||
)
|
||||
} catch (error) {
|
||||
console.warn(`[worktrees] Failed to clean up fork PR remote for ${removedWorktreeId}`, error)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -244,18 +406,35 @@ async function ensureUniqueRemoteNameSsh(
|
|||
async function prepareWorktreePushTargetSsh(
|
||||
provider: SshGitProvider,
|
||||
repoPath: string,
|
||||
target: GitPushTarget
|
||||
target: GitPushTarget,
|
||||
store?: WorktreePushTargetStore,
|
||||
repoId?: string
|
||||
): Promise<GitPushTarget> {
|
||||
assertGitPushTargetShape(target)
|
||||
const { remoteCreated: _ignoredRemoteCreated, ...sanitizedTarget } = target
|
||||
await provider.exec(['check-ref-format', '--branch', target.branchName], repoPath)
|
||||
let remoteName = target.remoteName
|
||||
let remoteCreated = false
|
||||
if (target.remoteUrl) {
|
||||
const existingRemote = await findRemoteForUrlSsh(provider, repoPath, target.remoteUrl)
|
||||
if (existingRemote) {
|
||||
remoteName = existingRemote
|
||||
// Why: if a later PR worktree reuses an Orca-created fork remote, it
|
||||
// must inherit ownership so deleting the final user can remove it.
|
||||
remoteCreated = store
|
||||
? isPushTargetRemoteCreatedByKnownWorktree(
|
||||
store,
|
||||
{
|
||||
...target,
|
||||
remoteName: existingRemote
|
||||
},
|
||||
repoId
|
||||
)
|
||||
: false
|
||||
} else {
|
||||
remoteName = await ensureUniqueRemoteNameSsh(provider, repoPath, target.remoteName)
|
||||
await provider.exec(['remote', 'add', remoteName, target.remoteUrl], repoPath)
|
||||
remoteCreated = true
|
||||
}
|
||||
}
|
||||
await provider.exec(
|
||||
|
|
@ -266,7 +445,30 @@ async function prepareWorktreePushTargetSsh(
|
|||
],
|
||||
repoPath
|
||||
)
|
||||
return { ...target, remoteName }
|
||||
return { ...sanitizedTarget, remoteName, ...(remoteCreated ? { remoteCreated: true } : {}) }
|
||||
}
|
||||
|
||||
export async function cleanupUnusedWorktreePushTargetRemoteSsh(
|
||||
provider: SshGitProvider,
|
||||
repoPath: string,
|
||||
removedWorktreeId: string,
|
||||
target: GitPushTarget | undefined,
|
||||
store: WorktreePushTargetStore
|
||||
): Promise<void> {
|
||||
try {
|
||||
await cleanupUnusedWorktreePushTargetRemoteWithExec(
|
||||
repoPath,
|
||||
removedWorktreeId,
|
||||
target,
|
||||
store,
|
||||
(args, cwd) => provider.exec(args, cwd)
|
||||
)
|
||||
} catch (error) {
|
||||
console.warn(
|
||||
`[worktrees] Failed to clean up remote fork PR remote for ${removedWorktreeId}`,
|
||||
error
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
async function configureCreatedWorktreePushTargetSsh(
|
||||
|
|
@ -444,7 +646,13 @@ export async function createRemoteWorktree(
|
|||
if (args.pushTarget) {
|
||||
// Why: fork-PR SSH worktrees need the same contributor-remote setup as
|
||||
// local worktrees before creation, otherwise Push/Sync can target origin.
|
||||
preparedPushTarget = await prepareWorktreePushTargetSsh(provider, repo.path, args.pushTarget)
|
||||
preparedPushTarget = await prepareWorktreePushTargetSsh(
|
||||
provider,
|
||||
repo.path,
|
||||
args.pushTarget,
|
||||
store,
|
||||
repo.id
|
||||
)
|
||||
}
|
||||
|
||||
const mux = getActiveMultiplexer(repo.connectionId!)
|
||||
|
|
@ -779,7 +987,7 @@ export async function createLocalWorktree(
|
|||
// Why: validate and fetch the contributor remote before creating the
|
||||
// worktree. If this fails, retrying won't hit branch/path conflicts from a
|
||||
// half-created worktree.
|
||||
preparedPushTarget = await prepareWorktreePushTarget(repo.path, args.pushTarget)
|
||||
preparedPushTarget = await prepareWorktreePushTarget(repo.path, args.pushTarget, store, repo.id)
|
||||
}
|
||||
|
||||
await (sparseDirectories.length > 0
|
||||
|
|
|
|||
|
|
@ -621,11 +621,69 @@ describe('registerWorktreeHandlers', () => {
|
|||
expect(store.setWorktreeMeta).toHaveBeenCalledWith(
|
||||
'repo-1::/workspace/improve-dashboard',
|
||||
expect.objectContaining({
|
||||
pushTarget: {
|
||||
pushTarget: expect.objectContaining({
|
||||
remoteName: 'pr-prateek-orca',
|
||||
branchName: 'prateek/fix-sidebar-agents-toggle',
|
||||
remoteUrl: 'git@github.com:prateek/orca.git'
|
||||
}
|
||||
remoteUrl: 'git@github.com:prateek/orca.git',
|
||||
remoteCreated: true
|
||||
})
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('keeps the Orca-created marker when a new worktree reuses an Orca-created fork remote', async () => {
|
||||
listWorktreesMock.mockResolvedValue([
|
||||
{
|
||||
path: '/workspace/improve-dashboard',
|
||||
head: 'abc123',
|
||||
branch: 'refs/heads/improve-dashboard',
|
||||
isBare: false,
|
||||
isMainWorktree: false
|
||||
}
|
||||
])
|
||||
const existingPushTarget = {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/previous-fix',
|
||||
remoteUrl: 'https://github.com/contributor/orca.git',
|
||||
remoteCreated: true
|
||||
}
|
||||
store.getAllWorktreeMeta.mockReturnValue({
|
||||
'repo-1::/workspace/previous-fix': makeWorktreeMeta({ pushTarget: existingPushTarget })
|
||||
})
|
||||
store.setWorktreeMeta.mockImplementation((_worktreeId, meta) => meta)
|
||||
gitExecFileAsyncMock.mockImplementation(async (args: string[]) => {
|
||||
if (args[0] === 'remote' && args.length === 1) {
|
||||
return { stdout: 'pr-contributor-orca\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
return { stdout: 'https://github.com/contributor/orca.git\n', stderr: '' }
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
|
||||
await handlers['worktrees:create'](null, {
|
||||
repoId: 'repo-1',
|
||||
name: 'improve-dashboard',
|
||||
pushTarget: {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/new-fix',
|
||||
remoteUrl: 'https://github.com/contributor/orca.git'
|
||||
}
|
||||
})
|
||||
|
||||
expect(gitExecFileAsyncMock).not.toHaveBeenCalledWith(
|
||||
['remote', 'add', expect.any(String), expect.any(String)],
|
||||
expect.any(Object)
|
||||
)
|
||||
expect(store.setWorktreeMeta).toHaveBeenCalledWith(
|
||||
'repo-1::/workspace/improve-dashboard',
|
||||
expect.objectContaining({
|
||||
pushTarget: expect.objectContaining({
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/new-fix',
|
||||
remoteUrl: 'https://github.com/contributor/orca.git',
|
||||
remoteCreated: true
|
||||
})
|
||||
})
|
||||
)
|
||||
})
|
||||
|
|
@ -2035,6 +2093,106 @@ describe('registerWorktreeHandlers', () => {
|
|||
)
|
||||
})
|
||||
|
||||
it('removes an unused Orca-created fork remote after deleting its worktree', async () => {
|
||||
mockKnownFeatureWorktree()
|
||||
removeWorktreeMock.mockResolvedValue(undefined)
|
||||
const pushTarget = {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'feature/from-fork',
|
||||
remoteUrl: 'https://github.com/contributor/orca.git',
|
||||
remoteCreated: true
|
||||
}
|
||||
store.getWorktreeMeta.mockReturnValue(makeWorktreeMeta({ pushTarget }))
|
||||
store.getAllWorktreeMeta.mockReturnValue({
|
||||
'repo-1::/workspace/feature-wt': makeWorktreeMeta({ pushTarget })
|
||||
})
|
||||
gitExecFileAsyncMock.mockImplementation(async (args: string[]) => {
|
||||
if (args[0] === 'config') {
|
||||
throw new Error('no branch config')
|
||||
}
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
return { stdout: 'https://github.com/contributor/orca.git\n', stderr: '' }
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
|
||||
await handlers['worktrees:remove'](null, {
|
||||
worktreeId: 'repo-1::/workspace/feature-wt'
|
||||
})
|
||||
|
||||
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['remote', 'remove', 'pr-contributor-orca'], {
|
||||
cwd: '/workspace/repo'
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps an Orca-created fork remote while another worktree still uses it', async () => {
|
||||
mockKnownFeatureWorktree()
|
||||
removeWorktreeMock.mockResolvedValue(undefined)
|
||||
const pushTarget = {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'feature/from-fork',
|
||||
remoteUrl: 'https://github.com/contributor/orca.git',
|
||||
remoteCreated: true
|
||||
}
|
||||
store.getWorktreeMeta.mockReturnValue(makeWorktreeMeta({ pushTarget }))
|
||||
store.getAllWorktreeMeta.mockReturnValue({
|
||||
'repo-1::/workspace/feature-wt': makeWorktreeMeta({ pushTarget }),
|
||||
'repo-1::/workspace/other-wt': makeWorktreeMeta({
|
||||
pushTarget: {
|
||||
...pushTarget,
|
||||
branchName: 'other-branch'
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
await handlers['worktrees:remove'](null, {
|
||||
worktreeId: 'repo-1::/workspace/feature-wt'
|
||||
})
|
||||
|
||||
expect(gitExecFileAsyncMock).not.toHaveBeenCalledWith(
|
||||
['remote', 'remove', 'pr-contributor-orca'],
|
||||
expect.any(Object)
|
||||
)
|
||||
})
|
||||
|
||||
it('ignores matching push targets from other repos when deciding fork remote cleanup', async () => {
|
||||
mockKnownFeatureWorktree()
|
||||
removeWorktreeMock.mockResolvedValue(undefined)
|
||||
const pushTarget = {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'feature/from-fork',
|
||||
remoteUrl: 'https://github.com/contributor/orca.git',
|
||||
remoteCreated: true
|
||||
}
|
||||
store.getWorktreeMeta.mockReturnValue(makeWorktreeMeta({ pushTarget }))
|
||||
store.getAllWorktreeMeta.mockReturnValue({
|
||||
'repo-1::/workspace/feature-wt': makeWorktreeMeta({ pushTarget }),
|
||||
'repo-2::/workspace/other-wt': makeWorktreeMeta({
|
||||
pushTarget: {
|
||||
...pushTarget,
|
||||
branchName: 'other-branch'
|
||||
}
|
||||
})
|
||||
})
|
||||
gitExecFileAsyncMock.mockImplementation(async (args: string[]) => {
|
||||
if (args[0] === 'config') {
|
||||
throw new Error('no branch config')
|
||||
}
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
return { stdout: 'https://github.com/contributor/orca.git\n', stderr: '' }
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
|
||||
await handlers['worktrees:remove'](null, {
|
||||
worktreeId: 'repo-1::/workspace/feature-wt'
|
||||
})
|
||||
|
||||
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['remote', 'remove', 'pr-contributor-orca'], {
|
||||
cwd: '/workspace/repo'
|
||||
})
|
||||
})
|
||||
|
||||
it('rejects unregistered delete paths before teardown, hooks, or git removal', async () => {
|
||||
mockKnownFeatureWorktree('/workspace/real-feature')
|
||||
getEffectiveHooksMock.mockReturnValue({
|
||||
|
|
|
|||
|
|
@ -51,6 +51,8 @@ import { joinWorktreeRelativePath } from '../runtime/runtime-relative-paths'
|
|||
import {
|
||||
createLocalWorktree,
|
||||
createRemoteWorktree,
|
||||
cleanupUnusedWorktreePushTargetRemote,
|
||||
cleanupUnusedWorktreePushTargetRemoteSsh,
|
||||
notifyWorktreesChanged
|
||||
} from './worktree-remote'
|
||||
import {
|
||||
|
|
@ -603,9 +605,17 @@ export function registerWorktreeHandlers(
|
|||
worktreePath,
|
||||
registeredWorktrees
|
||||
).path
|
||||
const removedPushTarget = store.getWorktreeMeta(args.worktreeId)?.pushTarget
|
||||
|
||||
if (repo.connectionId) {
|
||||
await provider!.removeWorktree(canonicalWorktreePath, args.force)
|
||||
await cleanupUnusedWorktreePushTargetRemoteSsh(
|
||||
provider!,
|
||||
repo.path,
|
||||
args.worktreeId,
|
||||
removedPushTarget,
|
||||
store
|
||||
)
|
||||
runtime.clearOptimisticReconcileToken(args.worktreeId)
|
||||
store.removeWorktreeMeta(args.worktreeId)
|
||||
deleteWorktreeHistoryDir(args.worktreeId)
|
||||
|
|
@ -685,6 +695,12 @@ export function registerWorktreeHandlers(
|
|||
// list` continues to show the stale entry and the branch it had checked out
|
||||
// remains locked — other worktrees cannot check it out.
|
||||
await gitExecFileAsync(['worktree', 'prune'], { cwd: repo.path }).catch(() => {})
|
||||
await cleanupUnusedWorktreePushTargetRemote(
|
||||
repo.path,
|
||||
args.worktreeId,
|
||||
removedPushTarget,
|
||||
store
|
||||
)
|
||||
runtime.clearOptimisticReconcileToken(args.worktreeId)
|
||||
store.removeWorktreeMeta(args.worktreeId)
|
||||
deleteWorktreeHistoryDir(args.worktreeId)
|
||||
|
|
@ -696,6 +712,12 @@ export function registerWorktreeHandlers(
|
|||
formatWorktreeRemovalError(error, canonicalWorktreePath, args.force ?? false)
|
||||
)
|
||||
}
|
||||
await cleanupUnusedWorktreePushTargetRemote(
|
||||
repo.path,
|
||||
args.worktreeId,
|
||||
removedPushTarget,
|
||||
store
|
||||
)
|
||||
runtime.clearOptimisticReconcileToken(args.worktreeId)
|
||||
store.removeWorktreeMeta(args.worktreeId)
|
||||
deleteWorktreeHistoryDir(args.worktreeId)
|
||||
|
|
|
|||
|
|
@ -290,6 +290,17 @@ describe('SshGitProvider', () => {
|
|||
})
|
||||
})
|
||||
|
||||
it('pushBranch forwards force-with-lease mode', async () => {
|
||||
await provider.pushBranch('/home/user/repo', false, undefined, { forceWithLease: true })
|
||||
|
||||
expect(mux.request).toHaveBeenCalledWith('git.push', {
|
||||
worktreePath: '/home/user/repo',
|
||||
publish: false,
|
||||
pushTarget: undefined,
|
||||
forceWithLease: true
|
||||
})
|
||||
})
|
||||
|
||||
it('pullBranch sends git.pull request', async () => {
|
||||
await provider.pullBranch('/home/user/repo')
|
||||
expect(mux.request).toHaveBeenCalledWith('git.pull', {
|
||||
|
|
|
|||
|
|
@ -186,9 +186,15 @@ export class SshGitProvider implements IGitProvider {
|
|||
async pushBranch(
|
||||
worktreePath: string,
|
||||
publish = false,
|
||||
pushTarget?: GitPushTarget
|
||||
pushTarget?: GitPushTarget,
|
||||
options: { forceWithLease?: boolean } = {}
|
||||
): Promise<void> {
|
||||
await this.mux.request('git.push', { worktreePath, publish, pushTarget })
|
||||
await this.mux.request('git.push', {
|
||||
worktreePath,
|
||||
publish,
|
||||
pushTarget,
|
||||
...(options.forceWithLease === true ? { forceWithLease: true } : {})
|
||||
})
|
||||
}
|
||||
|
||||
async pullBranch(worktreePath: string): Promise<void> {
|
||||
|
|
|
|||
|
|
@ -167,7 +167,12 @@ export type IGitProvider = {
|
|||
getBranchCompare(worktreePath: string, baseRef: string): Promise<GitBranchCompareResult>
|
||||
getCommitCompare(worktreePath: string, commitId: string): Promise<GitCommitCompareResult>
|
||||
getUpstreamStatus(worktreePath: string): Promise<GitUpstreamStatus>
|
||||
pushBranch(worktreePath: string, publish?: boolean, pushTarget?: GitPushTarget): Promise<void>
|
||||
pushBranch(
|
||||
worktreePath: string,
|
||||
publish?: boolean,
|
||||
pushTarget?: GitPushTarget,
|
||||
options?: { forceWithLease?: boolean }
|
||||
): Promise<void>
|
||||
pullBranch(worktreePath: string): Promise<void>
|
||||
fetchRemote(worktreePath: string): Promise<void>
|
||||
getBranchDiff(
|
||||
|
|
|
|||
|
|
@ -1,8 +1,13 @@
|
|||
import type { GitWorktreeInfo, Repo } from '../shared/types'
|
||||
import { resolve } from 'path'
|
||||
import { listWorktrees } from './git/worktree'
|
||||
import { isFolderRepo } from '../shared/repo-kind'
|
||||
import { getSshGitProvider } from './providers/ssh-git-dispatch'
|
||||
|
||||
export function isRepoRoot(repos: Repo[], resolvedTarget: string): boolean {
|
||||
return repos.some((repo) => !repo.connectionId && resolve(repo.path) === resolvedTarget)
|
||||
}
|
||||
|
||||
export function createFolderWorktree(repo: Repo): GitWorktreeInfo {
|
||||
return {
|
||||
path: repo.path,
|
||||
|
|
|
|||
|
|
@ -239,7 +239,8 @@ export class RuntimeGitCommands {
|
|||
async pushRuntimeGit(
|
||||
worktreeSelector: string,
|
||||
publish?: boolean,
|
||||
pushTarget?: GitPushTarget
|
||||
pushTarget?: GitPushTarget,
|
||||
forceWithLease?: boolean
|
||||
): Promise<{ ok: true }> {
|
||||
const target = await this.host.resolveRuntimeGitTarget(worktreeSelector)
|
||||
const provider = target.connectionId ? getSshGitProvider(target.connectionId) : null
|
||||
|
|
@ -247,10 +248,14 @@ export class RuntimeGitCommands {
|
|||
if (!provider) {
|
||||
throw new Error(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE)
|
||||
}
|
||||
await provider.pushBranch(target.worktree.path, publish === true, pushTarget)
|
||||
await provider.pushBranch(target.worktree.path, publish === true, pushTarget, {
|
||||
forceWithLease: forceWithLease === true
|
||||
})
|
||||
return { ok: true }
|
||||
}
|
||||
await gitPush(target.worktree.path, publish === true, pushTarget)
|
||||
await gitPush(target.worktree.path, publish === true, pushTarget, {
|
||||
forceWithLease: forceWithLease === true
|
||||
})
|
||||
return { ok: true }
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -269,6 +269,8 @@ import { DEFAULT_REPO_BADGE_COLOR, getDefaultVoiceSettings } from '../../shared/
|
|||
import { listRepoWorktrees } from '../repo-worktrees'
|
||||
import { createWorktreeSymlinks } from '../ipc/worktree-symlinks'
|
||||
import {
|
||||
cleanupUnusedWorktreePushTargetRemote,
|
||||
cleanupUnusedWorktreePushTargetRemoteSsh,
|
||||
createRemoteWorktree,
|
||||
configureCreatedWorktreePushTarget,
|
||||
prepareWorktreePushTarget
|
||||
|
|
@ -5989,7 +5991,12 @@ export class OrcaRuntimeService {
|
|||
// Why: fork-PR worktrees created through a remote runtime need the same
|
||||
// upstream target setup as local desktop creates, or Push would publish
|
||||
// to the wrong remote after the client/server split.
|
||||
preparedPushTarget = await prepareWorktreePushTarget(repo.path, args.pushTarget)
|
||||
preparedPushTarget = await prepareWorktreePushTarget(
|
||||
repo.path,
|
||||
args.pushTarget,
|
||||
this.store,
|
||||
repo.id
|
||||
)
|
||||
}
|
||||
|
||||
await (sparseDirectories.length > 0
|
||||
|
|
@ -6818,6 +6825,13 @@ export class OrcaRuntimeService {
|
|||
if (repo.connectionId) {
|
||||
const provider = requireSshGitProvider(repo.connectionId)
|
||||
await provider.removeWorktree(worktree.path, force)
|
||||
await cleanupUnusedWorktreePushTargetRemoteSsh(
|
||||
provider,
|
||||
repo.path,
|
||||
worktree.id,
|
||||
worktree.pushTarget,
|
||||
this.store
|
||||
)
|
||||
this.clearOptimisticReconcileToken(worktree.id)
|
||||
this.store.removeWorktreeMeta(worktree.id)
|
||||
this.invalidateResolvedWorktreeCache()
|
||||
|
|
@ -6894,6 +6908,12 @@ export class OrcaRuntimeService {
|
|||
// list` continues to show the stale entry and the branch it had checked out
|
||||
// remains locked — other worktrees cannot check it out.
|
||||
await gitExecFileAsync(['worktree', 'prune'], { cwd: repo.path }).catch(() => {})
|
||||
await cleanupUnusedWorktreePushTargetRemote(
|
||||
repo.path,
|
||||
worktree.id,
|
||||
worktree.pushTarget,
|
||||
this.store
|
||||
)
|
||||
this.clearOptimisticReconcileToken(worktree.id)
|
||||
this.store.removeWorktreeMeta(worktree.id)
|
||||
this.invalidateResolvedWorktreeCache()
|
||||
|
|
@ -6906,6 +6926,12 @@ export class OrcaRuntimeService {
|
|||
throw new Error(formatWorktreeRemovalError(error, worktree.path, force))
|
||||
}
|
||||
|
||||
await cleanupUnusedWorktreePushTargetRemote(
|
||||
repo.path,
|
||||
worktree.id,
|
||||
worktree.pushTarget,
|
||||
this.store
|
||||
)
|
||||
this.clearOptimisticReconcileToken(worktree.id)
|
||||
this.store.removeWorktreeMeta(worktree.id)
|
||||
this.invalidateResolvedWorktreeCache()
|
||||
|
|
|
|||
|
|
@ -124,6 +124,7 @@ export const GitBulkPaths = WorktreeSelector.extend({
|
|||
|
||||
export const GitPush = WorktreeSelector.extend({
|
||||
publish: z.boolean().optional(),
|
||||
forceWithLease: z.boolean().optional(),
|
||||
pushTarget: z.unknown().optional()
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -237,10 +237,32 @@ describe('git RPC methods', () => {
|
|||
agentCmdOverrides: { cursor: 'cursor-agent' }
|
||||
})
|
||||
expect(runtime.cancelRuntimeGenerateCommitMessage).toHaveBeenCalledWith('id:wt-1')
|
||||
expect(runtime.pushRuntimeGit).toHaveBeenCalledWith('id:wt-1', true, { remote: 'origin' })
|
||||
expect(runtime.pushRuntimeGit).toHaveBeenCalledWith(
|
||||
'id:wt-1',
|
||||
true,
|
||||
{ remote: 'origin' },
|
||||
undefined
|
||||
)
|
||||
expect(response).toMatchObject({ ok: true, result: 'https://example.com/file#L3' })
|
||||
})
|
||||
|
||||
it('forwards force-with-lease push mode to the runtime', async () => {
|
||||
const runtime = {
|
||||
getRuntimeId: () => 'test-runtime',
|
||||
pushRuntimeGit: vi.fn().mockResolvedValue({ ok: true })
|
||||
} as unknown as OrcaRuntimeService
|
||||
const dispatcher = new RpcDispatcher({ runtime, methods: GIT_METHODS })
|
||||
|
||||
await dispatcher.dispatch(
|
||||
makeRequest('git.push', {
|
||||
worktree: 'id:wt-1',
|
||||
forceWithLease: true
|
||||
})
|
||||
)
|
||||
|
||||
expect(runtime.pushRuntimeGit).toHaveBeenCalledWith('id:wt-1', undefined, undefined, true)
|
||||
})
|
||||
|
||||
it('forwards commit-message settings to the runtime', async () => {
|
||||
const commitMessageAi = {
|
||||
enabled: true,
|
||||
|
|
|
|||
|
|
@ -91,7 +91,12 @@ export const GIT_METHODS: RpcMethod[] = [
|
|||
name: 'git.push',
|
||||
params: GitPush,
|
||||
handler: async (params, { runtime }) =>
|
||||
runtime.pushRuntimeGit(params.worktree, params.publish, params.pushTarget as never)
|
||||
runtime.pushRuntimeGit(
|
||||
params.worktree,
|
||||
params.publish,
|
||||
params.pushTarget as never,
|
||||
params.forceWithLease
|
||||
)
|
||||
}),
|
||||
defineMethod({
|
||||
name: 'git.branchDiff',
|
||||
|
|
|
|||
|
|
@ -1016,7 +1016,7 @@ describe('OrcaRuntimeRpcServer', () => {
|
|||
expect(selectCodexAccount).toHaveBeenCalledWith(null)
|
||||
expect(readTerminal).toHaveBeenCalledWith('term-1', { cursor: undefined })
|
||||
expect(getRuntimeGitStatus).toHaveBeenCalledWith('id:wt-1')
|
||||
expect(pushRuntimeGit).toHaveBeenCalledWith('id:wt-1', true, undefined)
|
||||
expect(pushRuntimeGit).toHaveBeenCalledWith('id:wt-1', true, undefined, undefined)
|
||||
expect(getRuntimeGitUpstreamStatus).toHaveBeenCalledWith('id:wt-1')
|
||||
expect(bulkStageRuntimeGitPaths).toHaveBeenCalledWith('id:wt-1', ['a.ts', 'b.ts'])
|
||||
expect(bulkUnstageRuntimeGitPaths).toHaveBeenCalledWith('id:wt-1', ['c.ts'])
|
||||
|
|
@ -1099,7 +1099,7 @@ describe('OrcaRuntimeRpcServer', () => {
|
|||
)
|
||||
|
||||
expect(replies).toContainEqual(expect.objectContaining({ id: 'req_push', ok: true }))
|
||||
expect(pushRuntimeGit).toHaveBeenCalledWith('id:wt-1', undefined, undefined)
|
||||
expect(pushRuntimeGit).toHaveBeenCalledWith('id:wt-1', undefined, undefined, undefined)
|
||||
})
|
||||
|
||||
it('leaves the last published metadata in place when a runtime stops', async () => {
|
||||
|
|
|
|||
|
|
@ -1,3 +1,7 @@
|
|||
/* eslint-disable max-lines */
|
||||
// Why: PR context generation depends on command order across remote-state
|
||||
// variants; keeping the table of git command mocks together makes regressions
|
||||
// easier to audit than splitting the suite by helper.
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { getPullRequestDraftContext } from './pull-request-context'
|
||||
|
||||
|
|
@ -13,20 +17,17 @@ function createContextInput(base = 'main') {
|
|||
}
|
||||
|
||||
describe('getPullRequestDraftContext', () => {
|
||||
it('fetches and rebases onto the resolved remote base before collecting PR context', async () => {
|
||||
it('fetches the resolved remote base before collecting PR context without mutating HEAD', async () => {
|
||||
const execGit = vi.fn<GitExec>(async (args) => {
|
||||
if (args[0] === 'fetch') {
|
||||
return { stdout: '', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'remote') {
|
||||
return { stdout: 'origin\nupstream\n', 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: '' }
|
||||
}
|
||||
|
|
@ -54,27 +55,149 @@ describe('getPullRequestDraftContext', () => {
|
|||
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(
|
||||
['fetch', '--no-tags', 'origin', '+refs/heads/main:refs/remotes/origin/main'],
|
||||
expect.any(Object)
|
||||
)
|
||||
expect(execGit).not.toHaveBeenCalledWith(expect.arrayContaining(['rebase']), expect.anything())
|
||||
expect(execGit).not.toHaveBeenCalledWith(
|
||||
expect.arrayContaining(['rev-parse']),
|
||||
expect.anything()
|
||||
)
|
||||
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'))
|
||||
expect(commandNames.indexOf('fetch')).toBeLessThan(commandNames.indexOf('merge-base'))
|
||||
})
|
||||
|
||||
it('reports when preparation changes HEAD', async () => {
|
||||
let revParseCount = 0
|
||||
it('fetches the preferred remote base even when the tracking ref is absent locally', async () => {
|
||||
const execGit = vi.fn<GitExec>(async (args) => {
|
||||
if (args[0] === 'fetch' || args[0] === 'rebase') {
|
||||
if (args[0] === 'fetch') {
|
||||
return { stdout: '', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'remote') {
|
||||
return { stdout: 'origin\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'for-each-ref') {
|
||||
return { stdout: '', 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') {
|
||||
return { stdout: 'M\tREADME.md\n', stderr: '' }
|
||||
}
|
||||
throw new Error(`Unexpected git args: ${args.join(' ')}`)
|
||||
})
|
||||
|
||||
await getPullRequestDraftContext(execGit, createContextInput())
|
||||
|
||||
expect(execGit).toHaveBeenCalledWith(
|
||||
['fetch', '--no-tags', 'origin', '+refs/heads/main:refs/remotes/origin/main'],
|
||||
expect.any(Object)
|
||||
)
|
||||
expect(execGit).not.toHaveBeenCalledWith(expect.arrayContaining(['rebase']), expect.anything())
|
||||
})
|
||||
|
||||
it('does not fetch unrelated fork remotes before generating PR context', async () => {
|
||||
const execGit = vi.fn<GitExec>(async (args) => {
|
||||
if (args[0] === 'fetch') {
|
||||
expect(args).not.toContain('--all')
|
||||
expect(args[2]).toBe('origin')
|
||||
return { stdout: '', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'remote') {
|
||||
return { stdout: 'origin\nstale-fork\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'for-each-ref') {
|
||||
return {
|
||||
stdout: 'origin/main\nstale-fork/feature/from-stale-fork\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: change\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'diff') {
|
||||
return { stdout: 'M\tREADME.md\n', stderr: '' }
|
||||
}
|
||||
throw new Error(`Unexpected git args: ${args.join(' ')}`)
|
||||
})
|
||||
|
||||
await expect(getPullRequestDraftContext(execGit, createContextInput())).resolves.toMatchObject({
|
||||
branch: 'feature/pr-details'
|
||||
})
|
||||
|
||||
expect(execGit).not.toHaveBeenCalledWith(['fetch', '--all', '--prune'], expect.any(Object))
|
||||
expect(execGit).not.toHaveBeenCalledWith(
|
||||
expect.arrayContaining(['stale-fork']),
|
||||
expect.any(Object)
|
||||
)
|
||||
})
|
||||
|
||||
it('does not guess between multiple non-preferred remote bases for a bare base name', async () => {
|
||||
const execGit = vi.fn<GitExec>(async (args) => {
|
||||
if (args[0] === 'fetch') {
|
||||
throw new Error(`Unexpected fetch: ${args.join(' ')}`)
|
||||
}
|
||||
if (args[0] === 'remote') {
|
||||
return { stdout: 'contributor-a\ncontributor-b\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'for-each-ref') {
|
||||
return { stdout: 'contributor-a/main\ncontributor-b/main\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'branch') {
|
||||
return { stdout: 'feature\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'merge-base') {
|
||||
expect(args[1]).toBe('main')
|
||||
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(' ')}`)
|
||||
})
|
||||
|
||||
await getPullRequestDraftContext(execGit, createContextInput())
|
||||
|
||||
expect(execGit).not.toHaveBeenCalledWith(
|
||||
expect.arrayContaining(['contributor-a']),
|
||||
expect.any(Object)
|
||||
)
|
||||
expect(execGit).not.toHaveBeenCalledWith(
|
||||
expect.arrayContaining(['contributor-b']),
|
||||
expect.any(Object)
|
||||
)
|
||||
expect(execGit).not.toHaveBeenCalledWith(expect.arrayContaining(['rebase']), expect.anything())
|
||||
})
|
||||
|
||||
it('reports no branch change because PR context preparation is read-only', async () => {
|
||||
const execGit = vi.fn<GitExec>(async (args) => {
|
||||
if (args[0] === 'fetch') {
|
||||
return { stdout: '', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'remote') {
|
||||
return { stdout: 'origin\n', 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: '' }
|
||||
}
|
||||
|
|
@ -92,23 +215,28 @@ describe('getPullRequestDraftContext', () => {
|
|||
|
||||
const context = await getPullRequestDraftContext(execGit, createContextInput())
|
||||
|
||||
expect(context?.branchChangedByPreparation).toBe(true)
|
||||
expect(context?.branchChangedByPreparation).toBe(false)
|
||||
expect(execGit).not.toHaveBeenCalledWith(expect.arrayContaining(['rebase']), expect.anything())
|
||||
expect(execGit).not.toHaveBeenCalledWith(
|
||||
expect.arrayContaining(['rev-parse']),
|
||||
expect.anything()
|
||||
)
|
||||
})
|
||||
|
||||
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') {
|
||||
if (args[0] === 'fetch') {
|
||||
return { stdout: '', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'remote') {
|
||||
return { stdout: 'origin\nupstream\n', 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: '' }
|
||||
}
|
||||
|
|
@ -123,36 +251,73 @@ describe('getPullRequestDraftContext', () => {
|
|||
|
||||
await getPullRequestDraftContext(execGit, createContextInput('upstream/main'))
|
||||
|
||||
expect(execGit).toHaveBeenCalledWith(['rebase', 'upstream/main'], expect.any(Object))
|
||||
expect(execGit).toHaveBeenCalledWith(
|
||||
['fetch', '--no-tags', 'upstream', '+refs/heads/main:refs/remotes/upstream/main'],
|
||||
expect.any(Object)
|
||||
)
|
||||
expect(execGit).not.toHaveBeenCalledWith(expect.arrayContaining(['rebase']), expect.anything())
|
||||
expect(execGit).toHaveBeenCalledWith(
|
||||
['merge-base', 'upstream/main', 'HEAD'],
|
||||
expect.any(Object)
|
||||
)
|
||||
})
|
||||
|
||||
it('stops generation when the rebase fails', async () => {
|
||||
it('does not run rebase before collecting PR context', async () => {
|
||||
const execGit = vi.fn<GitExec>(async (args) => {
|
||||
if (args[0] === 'fetch') {
|
||||
return { stdout: '', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'remote') {
|
||||
return { stdout: 'origin\n', 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] === 'branch') {
|
||||
return { stdout: 'feature\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'rebase') {
|
||||
throw new Error('Command failed: git rebase origin/main\nCONFLICT (content): README.md')
|
||||
throw new Error('Generate must not rebase the live worktree')
|
||||
}
|
||||
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(' ')}`)
|
||||
})
|
||||
|
||||
await expect(getPullRequestDraftContext(execGit, createContextInput())).resolves.toMatchObject({
|
||||
branch: 'feature'
|
||||
})
|
||||
expect(execGit).not.toHaveBeenCalledWith(expect.arrayContaining(['rebase']), expect.anything())
|
||||
})
|
||||
|
||||
it('stops generation when the relevant base fetch fails', async () => {
|
||||
const execGit = vi.fn<GitExec>(async (args) => {
|
||||
if (args[0] === 'remote') {
|
||||
return { stdout: 'origin\nstale-fork\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'for-each-ref') {
|
||||
return { stdout: 'origin/main\nstale-fork/main\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'fetch') {
|
||||
if (args[2] !== 'origin') {
|
||||
throw new Error(`Fetched unrelated remote: ${args.join(' ')}`)
|
||||
}
|
||||
throw new Error(
|
||||
'Command failed: git fetch --no-tags origin +refs/heads/main:refs/remotes/origin/main\nfatal: unable to access origin'
|
||||
)
|
||||
}
|
||||
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()
|
||||
'Fetch before generating PR details failed: fatal: unable to access origin'
|
||||
)
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -43,26 +43,108 @@ async function requiredExec(execGit: GitExec, args: string[], label: string): Pr
|
|||
}
|
||||
}
|
||||
|
||||
async function resolveComparisonBase(execGit: GitExec, base: string): Promise<string> {
|
||||
const refs = (
|
||||
await safeExec(execGit, ['for-each-ref', '--format=%(refname:short)', 'refs/remotes'])
|
||||
)
|
||||
type RemoteState = {
|
||||
remotes: string[]
|
||||
refs: string[]
|
||||
}
|
||||
|
||||
type RemoteBranch = {
|
||||
remote: string
|
||||
branch: string
|
||||
ref: string
|
||||
}
|
||||
|
||||
function splitGitLines(output: string): string[] {
|
||||
return output
|
||||
.split('\n')
|
||||
.map((line) => line.trim())
|
||||
.filter((line) => line && !line.endsWith('/HEAD'))
|
||||
.filter(Boolean)
|
||||
}
|
||||
|
||||
if (refs.includes(base)) {
|
||||
return base
|
||||
async function getRemoteState(execGit: GitExec): Promise<RemoteState> {
|
||||
const [remoteOutput, refOutput] = await Promise.all([
|
||||
safeExec(execGit, ['remote']),
|
||||
safeExec(execGit, ['for-each-ref', '--format=%(refname:short)', 'refs/remotes'])
|
||||
])
|
||||
return {
|
||||
remotes: splitGitLines(remoteOutput),
|
||||
refs: splitGitLines(refOutput).filter((line) => !line.endsWith('/HEAD'))
|
||||
}
|
||||
}
|
||||
|
||||
function parseRemoteBranch(ref: string, remotes: string[]): RemoteBranch | null {
|
||||
const remote = [...remotes]
|
||||
.sort((a, b) => b.length - a.length)
|
||||
.find((candidate) => ref.startsWith(`${candidate}/`))
|
||||
if (!remote) {
|
||||
return null
|
||||
}
|
||||
const branch = ref.slice(remote.length + 1)
|
||||
return branch ? { remote, branch, ref } : null
|
||||
}
|
||||
|
||||
function parseRemoteRef(ref: string, remotes: string[]): RemoteBranch | null {
|
||||
const parsed = parseRemoteBranch(ref, remotes)
|
||||
if (parsed) {
|
||||
return parsed
|
||||
}
|
||||
const slashIndex = ref.indexOf('/')
|
||||
if (slashIndex <= 0 || slashIndex === ref.length - 1) {
|
||||
return null
|
||||
}
|
||||
return {
|
||||
remote: ref.slice(0, slashIndex),
|
||||
branch: ref.slice(slashIndex + 1),
|
||||
ref
|
||||
}
|
||||
}
|
||||
|
||||
function resolveComparisonBase(
|
||||
base: string,
|
||||
state: RemoteState
|
||||
): {
|
||||
comparisonBase: string
|
||||
fetchTarget: RemoteBranch | null
|
||||
} {
|
||||
const qualifiedBase = parseRemoteBranch(base, state.remotes)
|
||||
if (qualifiedBase) {
|
||||
return { comparisonBase: qualifiedBase.ref, fetchTarget: qualifiedBase }
|
||||
}
|
||||
if (state.refs.includes(base)) {
|
||||
return { comparisonBase: base, fetchTarget: parseRemoteRef(base, state.remotes) }
|
||||
}
|
||||
|
||||
const preferredRemoteRefs = [`origin/${base}`, `upstream/${base}`]
|
||||
for (const ref of preferredRemoteRefs) {
|
||||
if (refs.includes(ref)) {
|
||||
return ref
|
||||
const parsed = parseRemoteRef(ref, state.remotes)
|
||||
if (parsed && (state.refs.includes(ref) || state.remotes.includes(parsed.remote))) {
|
||||
return { comparisonBase: ref, fetchTarget: parsed }
|
||||
}
|
||||
}
|
||||
|
||||
return refs.find((ref) => ref.endsWith(`/${base}`)) ?? base
|
||||
const matchingRefs = state.refs.filter((ref) => ref.endsWith(`/${base}`))
|
||||
if (matchingRefs.length === 1) {
|
||||
const ref = matchingRefs[0]
|
||||
return { comparisonBase: ref, fetchTarget: parseRemoteRef(ref, state.remotes) }
|
||||
}
|
||||
|
||||
return { comparisonBase: base, fetchTarget: null }
|
||||
}
|
||||
|
||||
async function fetchComparisonBase(execGit: GitExec, target: RemoteBranch | null): Promise<void> {
|
||||
if (!target) {
|
||||
return
|
||||
}
|
||||
await requiredExec(
|
||||
execGit,
|
||||
[
|
||||
'fetch',
|
||||
'--no-tags',
|
||||
target.remote,
|
||||
`+refs/heads/${target.branch}:refs/remotes/${target.remote}/${target.branch}`
|
||||
],
|
||||
'Fetch before generating PR details failed'
|
||||
)
|
||||
}
|
||||
|
||||
type PullRequestBranchPreparation = {
|
||||
|
|
@ -74,25 +156,15 @@ 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'])
|
||||
const { comparisonBase, fetchTarget } = resolveComparisonBase(base, await getRemoteState(execGit))
|
||||
// Why: PR generation only needs the selected base branch. A repo-wide
|
||||
// `fetch --all` makes stale contributor fork remotes block unrelated PRs.
|
||||
await fetchComparisonBase(execGit, fetchTarget)
|
||||
return {
|
||||
comparisonBase,
|
||||
branchChanged:
|
||||
Boolean(headBeforeRebase) && Boolean(headAfterRebase) && headBeforeRebase !== headAfterRebase
|
||||
// Why: Generate must be read-only. Rebasing the live worktree can rewrite
|
||||
// files under the running dev app and trigger a full Electron/Vite reload.
|
||||
branchChanged: false
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1509,6 +1509,7 @@ export type PreloadApi = {
|
|||
push: (args: {
|
||||
worktreePath: string
|
||||
publish?: boolean
|
||||
forceWithLease?: boolean
|
||||
connectionId?: string
|
||||
pushTarget?: GitPushTarget
|
||||
}) => Promise<void>
|
||||
|
|
|
|||
|
|
@ -2081,6 +2081,7 @@ const api = {
|
|||
push: (args: {
|
||||
worktreePath: string
|
||||
publish?: boolean
|
||||
forceWithLease?: boolean
|
||||
connectionId?: string
|
||||
pushTarget?: unknown
|
||||
}): Promise<void> => ipcRenderer.invoke('git:push', args),
|
||||
|
|
|
|||
|
|
@ -0,0 +1,83 @@
|
|||
import { describe, expect, it, vi } from 'vitest'
|
||||
import type { GitExec } from './git-handler-ops'
|
||||
import { removeWorktreeOp } from './git-handler-worktree-ops'
|
||||
|
||||
function worktreeList(...entries: { path: string; branch?: string }[]): string {
|
||||
return entries
|
||||
.map((entry, index) =>
|
||||
[
|
||||
`worktree ${entry.path}`,
|
||||
`HEAD ${index}`,
|
||||
...(entry.branch ? [`branch refs/heads/${entry.branch}`] : [])
|
||||
].join('\n')
|
||||
)
|
||||
.join('\n\n')
|
||||
}
|
||||
|
||||
describe('removeWorktreeOp', () => {
|
||||
it('deletes the now-unused branch after removing an SSH worktree', async () => {
|
||||
const calls: string[] = []
|
||||
let listCount = 0
|
||||
const git = vi.fn<GitExec>(async (args, cwd) => {
|
||||
calls.push(`${cwd}$ ${args.join(' ')}`)
|
||||
if (args[0] === 'rev-parse') {
|
||||
return { stdout: '/repo/.git\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'worktree' && args[1] === 'list') {
|
||||
listCount += 1
|
||||
return {
|
||||
stdout:
|
||||
listCount === 1
|
||||
? worktreeList(
|
||||
{ path: '/repo', branch: 'main' },
|
||||
{ path: '/repo-feature', branch: 'feature/test' }
|
||||
)
|
||||
: worktreeList({ path: '/repo', branch: 'main' }),
|
||||
stderr: ''
|
||||
}
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
|
||||
await removeWorktreeOp(git, { worktreePath: '/repo-feature' })
|
||||
|
||||
expect(calls).toEqual([
|
||||
'/repo-feature$ rev-parse --git-common-dir',
|
||||
'/repo$ worktree list --porcelain',
|
||||
'/repo$ worktree remove /repo-feature',
|
||||
'/repo$ worktree prune',
|
||||
'/repo$ worktree list --porcelain',
|
||||
'/repo$ branch -D feature/test'
|
||||
])
|
||||
})
|
||||
|
||||
it('keeps the branch when another SSH worktree still uses it', async () => {
|
||||
let listCount = 0
|
||||
const git = vi.fn<GitExec>(async (args, _cwd) => {
|
||||
if (args[0] === 'rev-parse') {
|
||||
return { stdout: '/repo/.git\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'worktree' && args[1] === 'list') {
|
||||
listCount += 1
|
||||
return {
|
||||
stdout:
|
||||
listCount === 1
|
||||
? worktreeList(
|
||||
{ path: '/repo', branch: 'main' },
|
||||
{ path: '/repo-feature', branch: 'feature/test' }
|
||||
)
|
||||
: worktreeList(
|
||||
{ path: '/repo', branch: 'main' },
|
||||
{ path: '/repo-other', branch: 'feature/test' }
|
||||
),
|
||||
stderr: ''
|
||||
}
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
|
||||
await removeWorktreeOp(git, { worktreePath: '/repo-feature' })
|
||||
|
||||
expect(git).not.toHaveBeenCalledWith(['branch', '-D', 'feature/test'], expect.any(String))
|
||||
})
|
||||
})
|
||||
|
|
@ -6,6 +6,7 @@
|
|||
*/
|
||||
import * as path from 'path'
|
||||
import type { GitExec } from './git-handler-ops'
|
||||
import { parseWorktreeList } from './git-handler-utils'
|
||||
|
||||
// ─── Worktree management ─────────────────────────────────────────────
|
||||
|
||||
|
|
@ -83,6 +84,12 @@ export async function removeWorktreeOp(
|
|||
// fall through with worktreePath as repo
|
||||
}
|
||||
|
||||
const worktreesBeforeRemoval = await listRelayWorktrees(git, repoPath)
|
||||
const removedWorktree = worktreesBeforeRemoval.find((worktree) =>
|
||||
areRelayWorktreePathsEqual(worktree.path, worktreePath)
|
||||
)
|
||||
const branchName = normalizeLocalBranchRef(removedWorktree?.branch ?? '')
|
||||
|
||||
const args = ['worktree', 'remove']
|
||||
if (force) {
|
||||
args.push('--force')
|
||||
|
|
@ -90,6 +97,59 @@ export async function removeWorktreeOp(
|
|||
args.push(worktreePath)
|
||||
await git(args, repoPath)
|
||||
await git(['worktree', 'prune'], repoPath)
|
||||
|
||||
if (!branchName) {
|
||||
return
|
||||
}
|
||||
|
||||
// Why: SSH worktree deletion should mirror local deletion. Dropping the
|
||||
// branch also removes its upstream config, which lets fork-remotes cleanup
|
||||
// after the last PR review worktree is gone.
|
||||
const worktreesAfterPrune = await listRelayWorktrees(git, repoPath)
|
||||
const branchStillInUse = worktreesAfterPrune.some(
|
||||
(worktree) => normalizeLocalBranchRef(worktree.branch ?? '') === branchName
|
||||
)
|
||||
if (branchStillInUse) {
|
||||
return
|
||||
}
|
||||
|
||||
try {
|
||||
await git(['branch', '-D', branchName], repoPath)
|
||||
} catch (error) {
|
||||
console.warn(
|
||||
`relay removeWorktree: failed to delete local branch "${branchName}" after removing worktree`,
|
||||
error
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
type RelayWorktreeInfo = {
|
||||
path: string
|
||||
branch?: string
|
||||
}
|
||||
|
||||
async function listRelayWorktrees(git: GitExec, repoPath: string): Promise<RelayWorktreeInfo[]> {
|
||||
try {
|
||||
const { stdout } = await git(['worktree', 'list', '--porcelain'], repoPath)
|
||||
return parseWorktreeList(stdout)
|
||||
.map((worktree) => ({
|
||||
path: typeof worktree.path === 'string' ? worktree.path : '',
|
||||
branch: typeof worktree.branch === 'string' ? worktree.branch : undefined
|
||||
}))
|
||||
.filter((worktree) => worktree.path.length > 0)
|
||||
} catch {
|
||||
return []
|
||||
}
|
||||
}
|
||||
|
||||
function normalizeLocalBranchRef(branch: string): string {
|
||||
return branch.replace(/^refs\/heads\//, '')
|
||||
}
|
||||
|
||||
function areRelayWorktreePathsEqual(leftPath: string, rightPath: string): boolean {
|
||||
const left = path.normalize(path.resolve(leftPath))
|
||||
const right = path.normalize(path.resolve(rightPath))
|
||||
return process.platform === 'win32' ? left.toLowerCase() === right.toLowerCase() : left === right
|
||||
}
|
||||
|
||||
// ─── Commit ──────────────────────────────────────────────────────────
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ import { commitChangesRelay, addWorktreeOp, removeWorktreeOp } from './git-handl
|
|||
import { checkIgnoredPathsOp, detectConflictOperation, getStatusOp } from './git-handler-status-ops'
|
||||
import { resolveRelayPushTarget } from './git-handler-push-target'
|
||||
import { normalizeGitErrorMessage, isNoUpstreamError } from '../shared/git-remote-error'
|
||||
import { upstreamOnlyCommitsArePatchEquivalent } from '../shared/git-upstream-status'
|
||||
import { loadGitHistoryFromExecutor } from '../shared/git-history'
|
||||
import { buildRelayCommandEnv } from './relay-command-env'
|
||||
|
||||
|
|
@ -312,11 +313,18 @@ export class GitHandler {
|
|||
if (!Number.isFinite(ahead) || !Number.isFinite(behind) || ahead < 0 || behind < 0) {
|
||||
throw new Error(`Unparseable git rev-list counts: ${JSON.stringify(countsStdout)}`)
|
||||
}
|
||||
const behindCommitsArePatchEquivalent =
|
||||
ahead > 0 && behind > 0
|
||||
? await this.getBehindCommitsArePatchEquivalent(worktreePath)
|
||||
: undefined
|
||||
return {
|
||||
hasUpstream: true,
|
||||
upstreamName,
|
||||
ahead,
|
||||
behind
|
||||
behind,
|
||||
...(behindCommitsArePatchEquivalent !== undefined
|
||||
? { behindCommitsArePatchEquivalent }
|
||||
: {})
|
||||
}
|
||||
} catch (error) {
|
||||
// Why: we only swallow the 'no upstream configured' error — that's an
|
||||
|
|
@ -331,6 +339,20 @@ export class GitHandler {
|
|||
}
|
||||
}
|
||||
|
||||
private async getBehindCommitsArePatchEquivalent(worktreePath: string): Promise<boolean> {
|
||||
try {
|
||||
const { stdout } = await this.git(
|
||||
['log', '--oneline', '--cherry-mark', '--right-only', 'HEAD...@{u}', '--'],
|
||||
worktreePath
|
||||
)
|
||||
return upstreamOnlyCommitsArePatchEquivalent(stdout)
|
||||
} catch {
|
||||
// Why: this only identifies stale post-rebase upstreams. If the probe
|
||||
// fails over SSH, keep the conservative pull-first sync path.
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
private async fetch(params: Record<string, unknown>) {
|
||||
const worktreePath = params.worktreePath as string
|
||||
try {
|
||||
|
|
@ -354,9 +376,12 @@ export class GitHandler {
|
|||
worktreePath,
|
||||
params.pushTarget
|
||||
)
|
||||
const args = target
|
||||
? ['push', '--set-upstream', target.remote, target.refspec]
|
||||
: ['push', '--set-upstream', 'origin', 'HEAD']
|
||||
const args = [
|
||||
'push',
|
||||
...(params.forceWithLease === true ? ['--force-with-lease'] : []),
|
||||
'--set-upstream',
|
||||
...(target ? [target.remote, target.refspec] : ['origin', 'HEAD'])
|
||||
]
|
||||
await this.git(args, worktreePath)
|
||||
} catch (error) {
|
||||
// Why: mirror the local gitPush normalization so SSH users see the same
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { renderToStaticMarkup } from 'react-dom/server'
|
||||
import { TooltipProvider } from '@/components/ui/tooltip'
|
||||
import { CommitArea } from './SourceControl'
|
||||
import { resolvePrimaryAction, type PrimaryActionInputs } from './source-control-primary-action'
|
||||
import { resolveDropdownItems, type DropdownActionKind } from './source-control-dropdown-items'
|
||||
|
|
@ -54,7 +55,13 @@ function buttons(markup: string): string[] {
|
|||
}
|
||||
|
||||
function renderButtons(props: ReturnType<typeof baseProps>): string[] {
|
||||
return buttons(renderToStaticMarkup(<CommitArea {...props} />))
|
||||
return buttons(
|
||||
renderToStaticMarkup(
|
||||
<TooltipProvider>
|
||||
<CommitArea {...props} />
|
||||
</TooltipProvider>
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
describe('CommitArea chevron spinner', () => {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { renderToStaticMarkup } from 'react-dom/server'
|
||||
import { TooltipProvider } from '@/components/ui/tooltip'
|
||||
import { CommitArea } from './SourceControl'
|
||||
import { resolvePrimaryAction, type PrimaryActionInputs } from './source-control-primary-action'
|
||||
import { resolveDropdownItems, type DropdownActionKind } from './source-control-dropdown-items'
|
||||
|
|
@ -50,7 +51,11 @@ function baseProps(overrides: Partial<PrimaryActionInputs> = {}) {
|
|||
}
|
||||
|
||||
function primaryButton(props: ReturnType<typeof baseProps>): string {
|
||||
const markup = renderToStaticMarkup(<CommitArea {...props} />)
|
||||
const markup = renderToStaticMarkup(
|
||||
<TooltipProvider>
|
||||
<CommitArea {...props} />
|
||||
</TooltipProvider>
|
||||
)
|
||||
const match = markup.match(/<button\b[\s\S]*?<\/button>/)
|
||||
if (!match) {
|
||||
throw new Error('primary button not found')
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ import { renderToStaticMarkup } from 'react-dom/server'
|
|||
import { CommitArea, ConflictSummaryCard } from './SourceControl'
|
||||
import { resolvePrimaryAction, type PrimaryActionInputs } from './source-control-primary-action'
|
||||
import { resolveDropdownItems, type DropdownActionKind } from './source-control-dropdown-items'
|
||||
import { TooltipProvider } from '@/components/ui/tooltip'
|
||||
|
||||
function buildInputs(overrides: Partial<PrimaryActionInputs> = {}): PrimaryActionInputs {
|
||||
return {
|
||||
|
|
@ -45,7 +46,11 @@ function baseProps(overrides: Partial<PrimaryActionInputs> = {}) {
|
|||
}
|
||||
|
||||
function renderCommitArea(props: ReturnType<typeof baseProps>): string {
|
||||
return renderToStaticMarkup(<CommitArea {...props} />)
|
||||
return renderToStaticMarkup(
|
||||
<TooltipProvider>
|
||||
<CommitArea {...props} />
|
||||
</TooltipProvider>
|
||||
)
|
||||
}
|
||||
|
||||
function firstButton(markup: string): string {
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import {
|
|||
normalizeSourceControlViewMode,
|
||||
pickDefaultSourceControlAgent,
|
||||
readCommitDraftForWorktree,
|
||||
refreshSourceControlAfterRemoteAction,
|
||||
requestSourceControlViewModePreferenceWrite,
|
||||
shouldRenderCommitArea,
|
||||
type SourceControlViewModePreferenceWriteState,
|
||||
|
|
@ -155,6 +156,41 @@ describe('SourceControl conflict resolution state', () => {
|
|||
})
|
||||
})
|
||||
|
||||
describe('SourceControl remote action refresh', () => {
|
||||
it('refreshes status, branch compare, and history after remote actions settle', async () => {
|
||||
const refreshGitStatus = vi.fn().mockResolvedValue(undefined)
|
||||
const refreshBranchCompare = vi.fn().mockResolvedValue(undefined)
|
||||
const refreshGitHistory = vi.fn().mockResolvedValue(undefined)
|
||||
|
||||
refreshSourceControlAfterRemoteAction({
|
||||
refreshGitStatus,
|
||||
refreshBranchCompare,
|
||||
refreshGitHistory
|
||||
})
|
||||
await Promise.resolve()
|
||||
|
||||
expect(refreshGitStatus).toHaveBeenCalledTimes(1)
|
||||
expect(refreshBranchCompare).toHaveBeenCalledTimes(1)
|
||||
expect(refreshGitHistory).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('routes post-remote refresh failures to the provided error handler', async () => {
|
||||
const error = new Error('refresh failed')
|
||||
const onError = vi.fn()
|
||||
|
||||
refreshSourceControlAfterRemoteAction({
|
||||
refreshGitStatus: vi.fn().mockResolvedValue(undefined),
|
||||
refreshBranchCompare: vi.fn().mockRejectedValue(error),
|
||||
refreshGitHistory: vi.fn().mockResolvedValue(undefined),
|
||||
onError
|
||||
})
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
|
||||
expect(onError).toHaveBeenCalledWith(error)
|
||||
})
|
||||
})
|
||||
|
||||
describe('SourceControl view mode preference', () => {
|
||||
it('normalizes missing and unknown persisted values to list', () => {
|
||||
expect(normalizeSourceControlViewMode(undefined)).toBe('list')
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load Diff
|
|
@ -26,7 +26,8 @@ describe('refreshGitStatusForWorktree', () => {
|
|||
hasUpstream: true,
|
||||
upstreamName: 'origin/feature',
|
||||
ahead: 2,
|
||||
behind: 1
|
||||
behind: 1,
|
||||
behindCommitsArePatchEquivalent: false
|
||||
}
|
||||
}
|
||||
const gitStatus = vi.fn().mockResolvedValue(status)
|
||||
|
|
@ -53,6 +54,31 @@ describe('refreshGitStatusForWorktree', () => {
|
|||
expect(deps.fetchUpstreamStatus).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('refreshes explicit upstream details for diverged porcelain-only status', async () => {
|
||||
const status: GitStatusResult = {
|
||||
entries: [],
|
||||
conflictOperation: 'unknown',
|
||||
upstreamStatus: {
|
||||
hasUpstream: true,
|
||||
upstreamName: 'origin/feature',
|
||||
ahead: 14,
|
||||
behind: 3
|
||||
}
|
||||
}
|
||||
const gitStatus = vi.fn().mockResolvedValue(status)
|
||||
vi.stubGlobal('window', { api: { git: { status: gitStatus } } })
|
||||
const deps = makeDeps()
|
||||
|
||||
await refreshGitStatusForWorktree({
|
||||
worktreeId: 'wt-1',
|
||||
worktreePath: '/repo',
|
||||
deps
|
||||
})
|
||||
|
||||
expect(deps.setUpstreamStatus).toHaveBeenCalledWith('wt-1', status.upstreamStatus)
|
||||
expect(deps.fetchUpstreamStatus).toHaveBeenCalledWith('wt-1', '/repo', undefined)
|
||||
})
|
||||
|
||||
it('falls back to explicit upstream refresh for legacy status payloads', async () => {
|
||||
const status: GitStatusResult = {
|
||||
entries: [],
|
||||
|
|
|
|||
|
|
@ -44,6 +44,16 @@ export async function refreshGitStatusForWorktree({
|
|||
})
|
||||
if (status.upstreamStatus) {
|
||||
deps.setUpstreamStatus(worktreeId, status.upstreamStatus)
|
||||
// Why: porcelain status has counts but cannot tell stale post-rebase
|
||||
// upstream commits from real remote work. A diverged branch needs the
|
||||
// richer explicit probe before the UI offers Pull/Sync.
|
||||
if (
|
||||
status.upstreamStatus.ahead > 0 &&
|
||||
status.upstreamStatus.behind > 0 &&
|
||||
status.upstreamStatus.behindCommitsArePatchEquivalent === undefined
|
||||
) {
|
||||
await deps.fetchUpstreamStatus(worktreeId, worktreePath, connectionId)
|
||||
}
|
||||
return
|
||||
}
|
||||
await deps.fetchUpstreamStatus(worktreeId, worktreePath, connectionId)
|
||||
|
|
|
|||
|
|
@ -1,10 +1,10 @@
|
|||
/* eslint-disable max-lines -- Why: the dropdown priority table is easier to audit when the row-state cases live together. */
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { resolveDropdownItems } from './source-control-dropdown-items'
|
||||
import type { PrimaryActionInputs } from './source-control-primary-action'
|
||||
import { resolveDropdownItems, type DropdownActionInputs } from './source-control-dropdown-items'
|
||||
|
||||
// Why: a shared defaults object keeps each case row terse while making the
|
||||
// "this is the one knob that differs from the baseline" intent obvious.
|
||||
function inputs(overrides: Partial<PrimaryActionInputs> = {}): PrimaryActionInputs {
|
||||
function inputs(overrides: Partial<DropdownActionInputs> = {}): DropdownActionInputs {
|
||||
return {
|
||||
stagedCount: 0,
|
||||
hasUnstagedChanges: false,
|
||||
|
|
@ -115,6 +115,76 @@ describe('resolveDropdownItems', () => {
|
|||
expect(byKind.sync.label).toBe('Sync (↓2 ↑3)')
|
||||
})
|
||||
|
||||
it('disables push-only actions on diverged branches so users sync first', () => {
|
||||
const items = resolveDropdownItems(
|
||||
inputs({
|
||||
stagedCount: 1,
|
||||
hasMessage: true,
|
||||
upstreamStatus: { hasUpstream: true, ahead: 2, behind: 3 }
|
||||
})
|
||||
)
|
||||
const byKind = Object.fromEntries(
|
||||
items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e])
|
||||
)
|
||||
|
||||
expect(byKind.push.disabled).toBe(true)
|
||||
expect(byKind.push.title).toBe('Sync first to pull remote changes before pushing')
|
||||
expect(byKind.commit_push.disabled).toBe(true)
|
||||
expect(byKind.commit_push.title).toBe('Use Commit & Sync to pull remote changes before pushing')
|
||||
expect(byKind.sync.disabled).toBe(false)
|
||||
expect(byKind.commit_sync.disabled).toBe(false)
|
||||
})
|
||||
|
||||
it('offers force-push-with-lease when remote-only commits are patch-equivalent', () => {
|
||||
const items = resolveDropdownItems(
|
||||
inputs({
|
||||
stagedCount: 1,
|
||||
hasMessage: true,
|
||||
branchCommitsAhead: 4,
|
||||
upstreamStatus: {
|
||||
hasUpstream: true,
|
||||
upstreamName: 'origin/feature',
|
||||
ahead: 14,
|
||||
behind: 3,
|
||||
behindCommitsArePatchEquivalent: true
|
||||
},
|
||||
hostedReviewCreation: {
|
||||
provider: 'github',
|
||||
review: null,
|
||||
canCreate: false,
|
||||
blockedReason: 'needs_sync',
|
||||
nextAction: 'sync'
|
||||
}
|
||||
})
|
||||
)
|
||||
const byKind = Object.fromEntries(
|
||||
items.filter((e) => e.kind !== 'separator').map((e) => [e.kind, e])
|
||||
)
|
||||
|
||||
expect(byKind.push.label).toBe('Force Push (4)')
|
||||
expect(byKind.push.disabled).toBe(false)
|
||||
expect(byKind.push.title).toBe(
|
||||
'Remote only has older copies of local commits. Force push 4 branch commits with lease to update origin/feature.'
|
||||
)
|
||||
expect(byKind.commit_push.label).toBe('Commit & Force Push')
|
||||
expect(byKind.commit_push.disabled).toBe(false)
|
||||
expect(byKind.commit_push.title).toBe('Commit staged changes and force push with lease')
|
||||
expect(byKind.pull.disabled).toBe(true)
|
||||
expect(byKind.pull.title).toBe(
|
||||
'Nothing new to pull — remote only has older copies of local commits'
|
||||
)
|
||||
expect(byKind.commit_sync.label).toBe('Commit & Sync')
|
||||
expect(byKind.commit_sync.disabled).toBe(true)
|
||||
expect(byKind.commit_sync.title).toBe(
|
||||
'Use Commit & Force Push — remote only has older copies of local commits'
|
||||
)
|
||||
expect(byKind.sync.disabled).toBe(true)
|
||||
expect(byKind.sync.title).toBe('Use Force Push — remote only has older copies of local commits')
|
||||
expect(byKind.create_pr.hint).toBe('Force Push first')
|
||||
expect(byKind.push_create_pr.label).toBe('Force Push before PR')
|
||||
expect(byKind.push_create_pr.disabled).toBe(false)
|
||||
})
|
||||
|
||||
it('omits counts from labels when ahead/behind are 0', () => {
|
||||
const items = resolveDropdownItems(
|
||||
inputs({ upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 } })
|
||||
|
|
@ -143,6 +213,29 @@ describe('resolveDropdownItems', () => {
|
|||
}
|
||||
})
|
||||
|
||||
it('locks every item while a pull request operation is running', () => {
|
||||
const items = resolveDropdownItems(
|
||||
inputs({
|
||||
isPullRequestOperationActive: true,
|
||||
upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 },
|
||||
hostedReviewCreation: {
|
||||
provider: 'github',
|
||||
review: null,
|
||||
canCreate: true,
|
||||
blockedReason: null,
|
||||
nextAction: null
|
||||
}
|
||||
})
|
||||
)
|
||||
|
||||
for (const entry of items) {
|
||||
if (entry.kind !== 'separator') {
|
||||
expect(entry.disabled).toBe(true)
|
||||
expect(entry.title).toBe('Pull request operation in progress…')
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
it('disables remote rows with a loading tooltip when upstreamStatus is undefined', () => {
|
||||
// Why: mirrors the primary-action guard — while fetchUpstreamStatus is in
|
||||
// flight we must not let the user click Publish on an already-tracked
|
||||
|
|
@ -259,7 +352,7 @@ describe('resolveDropdownItems', () => {
|
|||
expect(byKind.sync.label).toBe('Sync (↓3 ↑2)')
|
||||
})
|
||||
|
||||
it('enables Push & Create PR when review creation is only blocked by unpushed commits', () => {
|
||||
it('enables the push-before-PR recovery action when review creation is only blocked by unpushed commits', () => {
|
||||
const items = resolveDropdownItems(
|
||||
inputs({
|
||||
upstreamStatus: { hasUpstream: true, ahead: 2, behind: 0 },
|
||||
|
|
@ -277,6 +370,7 @@ describe('resolveDropdownItems', () => {
|
|||
)
|
||||
expect(byKind.create_pr.disabled).toBe(true)
|
||||
expect(byKind.create_pr.hint).toBe('Push first')
|
||||
expect(byKind.push_create_pr.label).toBe('Push before PR')
|
||||
expect(byKind.push_create_pr.disabled).toBe(false)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,6 +1,12 @@
|
|||
/* eslint-disable max-lines -- Why: this dropdown state machine keeps every action row in one table so priority and disabled-state regressions stay visible in tests. */
|
||||
// Why: split from source-control-primary-action because the primary and dropdown are independent derivations with different priority ladders; together they exceed the max-lines budget and tangle unrelated concerns.
|
||||
|
||||
import type { PrimaryActionInputs } from './source-control-primary-action'
|
||||
import { shouldForcePushWithLeaseForUpstream } from '../../../../shared/git-upstream-status'
|
||||
|
||||
export type DropdownActionInputs = PrimaryActionInputs & {
|
||||
isPullRequestOperationActive?: boolean
|
||||
}
|
||||
|
||||
export type DropdownActionKind =
|
||||
| 'commit'
|
||||
|
|
@ -49,12 +55,20 @@ function formatSyncLabel(base: string, ahead: number, behind: number): string {
|
|||
return `${base} (↓${behind} ↑${ahead})`
|
||||
}
|
||||
|
||||
function formatForcePushTitle(branchCommitsAhead: number | undefined, upstreamName?: string) {
|
||||
const countText =
|
||||
branchCommitsAhead && branchCommitsAhead > 0
|
||||
? `${branchCommitsAhead} branch commit${branchCommitsAhead === 1 ? '' : 's'}`
|
||||
: 'this branch'
|
||||
return `Remote only has older copies of local commits. Force push ${countText} with lease to update ${upstreamName ?? 'the remote branch'}.`
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the chevron dropdown items. Every item is always rendered so the
|
||||
* menu shape stays stable across states; inapplicable rows are disabled
|
||||
* with a tooltip reason rather than hidden.
|
||||
*/
|
||||
export function resolveDropdownItems(inputs: PrimaryActionInputs): DropdownEntry[] {
|
||||
export function resolveDropdownItems(inputs: DropdownActionInputs): DropdownEntry[] {
|
||||
const {
|
||||
stagedCount,
|
||||
hasPartiallyStagedChanges,
|
||||
|
|
@ -66,7 +80,8 @@ export function resolveDropdownItems(inputs: PrimaryActionInputs): DropdownEntry
|
|||
prState,
|
||||
isPRStateLoading,
|
||||
hostedReviewCreation,
|
||||
branchCommitsAhead
|
||||
branchCommitsAhead,
|
||||
isPullRequestOperationActive = false
|
||||
} = inputs
|
||||
|
||||
const hasStaged = stagedCount > 0
|
||||
|
|
@ -84,11 +99,15 @@ export function resolveDropdownItems(inputs: PrimaryActionInputs): DropdownEntry
|
|||
const publishBlockedByNoBranchCommits = !hasUpstream && branchCommitsAhead === 0
|
||||
const ahead = upstreamStatus?.ahead ?? 0
|
||||
const behind = upstreamStatus?.behind ?? 0
|
||||
const shouldForcePushWithLease = shouldForcePushWithLeaseForUpstream(upstreamStatus)
|
||||
const pushLabelCount =
|
||||
shouldForcePushWithLease && branchCommitsAhead !== undefined ? branchCommitsAhead : ahead
|
||||
const forcePushTitle = formatForcePushTitle(branchCommitsAhead, upstreamStatus?.upstreamName)
|
||||
|
||||
// Why: any in-flight commit or remote operation should lock the whole menu.
|
||||
// A running push shouldn't let a second pull/sync click queue up behind it
|
||||
// on a stale status snapshot.
|
||||
const globalBusy = isCommitting || isRemoteOperationActive
|
||||
const globalBusy = isCommitting || isRemoteOperationActive || isPullRequestOperationActive
|
||||
|
||||
const commitDisabledReason = (() => {
|
||||
if (hasUnresolvedConflicts) {
|
||||
|
|
@ -128,15 +147,21 @@ export function resolveDropdownItems(inputs: PrimaryActionInputs): DropdownEntry
|
|||
? 'PR is already merged'
|
||||
: !hasUpstream
|
||||
? 'Publish the branch first to push commits'
|
||||
: (commitDisabledReason ?? 'Commit staged changes and push')
|
||||
: (commitDisabledReason ??
|
||||
(shouldForcePushWithLease
|
||||
? 'Commit staged changes and force push with lease'
|
||||
: behind > 0
|
||||
? 'Use Commit & Sync to pull remote changes before pushing'
|
||||
: 'Commit staged changes and push'))
|
||||
const commitPushItem: DropdownItem = {
|
||||
kind: 'commit_push',
|
||||
label: 'Commit & Push',
|
||||
label: shouldForcePushWithLease ? 'Commit & Force Push' : 'Commit & Push',
|
||||
title: commitPushTitle,
|
||||
disabled:
|
||||
globalBusy ||
|
||||
upstreamLoading ||
|
||||
!hasUpstream ||
|
||||
(behind > 0 && !shouldForcePushWithLease) ||
|
||||
publishBlockedByPRLoading ||
|
||||
publishBlockedByMergedPR ||
|
||||
commitDisabledReason !== null
|
||||
|
|
@ -158,6 +183,12 @@ export function resolveDropdownItems(inputs: PrimaryActionInputs): DropdownEntry
|
|||
// nonexistent compound action.
|
||||
return 'Publish the branch first to sync commits'
|
||||
}
|
||||
if (shouldForcePushWithLease) {
|
||||
return (
|
||||
commitDisabledReason ??
|
||||
'Use Commit & Force Push — remote only has older copies of local commits'
|
||||
)
|
||||
}
|
||||
if (behind === 0) {
|
||||
return 'Nothing to pull — use Commit & Push instead'
|
||||
}
|
||||
|
|
@ -168,12 +199,17 @@ export function resolveDropdownItems(inputs: PrimaryActionInputs): DropdownEntry
|
|||
label: 'Commit & Sync',
|
||||
title: commitSyncTitle,
|
||||
disabled:
|
||||
globalBusy || upstreamLoading || !hasUpstream || behind === 0 || commitDisabledReason !== null
|
||||
globalBusy ||
|
||||
upstreamLoading ||
|
||||
!hasUpstream ||
|
||||
shouldForcePushWithLease ||
|
||||
behind === 0 ||
|
||||
commitDisabledReason !== null
|
||||
}
|
||||
|
||||
const pushItem: DropdownItem = {
|
||||
kind: 'push',
|
||||
label: formatCountLabel('Push', ahead),
|
||||
label: formatCountLabel(shouldForcePushWithLease ? 'Force Push' : 'Push', pushLabelCount),
|
||||
title: upstreamLoading
|
||||
? 'Checking branch status…'
|
||||
: publishBlockedByPRLoading
|
||||
|
|
@ -182,10 +218,19 @@ export function resolveDropdownItems(inputs: PrimaryActionInputs): DropdownEntry
|
|||
? 'PR is already merged'
|
||||
: !hasUpstream
|
||||
? 'Publish the branch first to push commits'
|
||||
: ahead === 0
|
||||
? 'Nothing to push'
|
||||
: describePushCount(ahead),
|
||||
disabled: globalBusy || upstreamLoading || !hasUpstream || ahead === 0
|
||||
: shouldForcePushWithLease
|
||||
? forcePushTitle
|
||||
: behind > 0 && ahead > 0
|
||||
? 'Sync first to pull remote changes before pushing'
|
||||
: ahead === 0
|
||||
? 'Nothing to push'
|
||||
: describePushCount(ahead),
|
||||
disabled:
|
||||
globalBusy ||
|
||||
upstreamLoading ||
|
||||
!hasUpstream ||
|
||||
ahead === 0 ||
|
||||
(behind > 0 && !shouldForcePushWithLease)
|
||||
}
|
||||
|
||||
const pullItem: DropdownItem = {
|
||||
|
|
@ -199,10 +244,13 @@ export function resolveDropdownItems(inputs: PrimaryActionInputs): DropdownEntry
|
|||
? 'PR is already merged'
|
||||
: !hasUpstream
|
||||
? 'Publish the branch first to pull commits'
|
||||
: behind === 0
|
||||
? 'Nothing to pull'
|
||||
: describePullCount(behind),
|
||||
disabled: globalBusy || upstreamLoading || !hasUpstream || behind === 0
|
||||
: shouldForcePushWithLease
|
||||
? 'Nothing new to pull — remote only has older copies of local commits'
|
||||
: behind === 0
|
||||
? 'Nothing to pull'
|
||||
: describePullCount(behind),
|
||||
disabled:
|
||||
globalBusy || upstreamLoading || !hasUpstream || behind === 0 || shouldForcePushWithLease
|
||||
}
|
||||
|
||||
const syncItem: DropdownItem = {
|
||||
|
|
@ -216,10 +264,17 @@ export function resolveDropdownItems(inputs: PrimaryActionInputs): DropdownEntry
|
|||
? 'PR is already merged'
|
||||
: !hasUpstream
|
||||
? 'Publish the branch first to sync commits'
|
||||
: ahead === 0 && behind === 0
|
||||
? 'Branch is up to date'
|
||||
: describeSyncCounts(ahead, behind),
|
||||
disabled: globalBusy || upstreamLoading || !hasUpstream || (ahead === 0 && behind === 0)
|
||||
: shouldForcePushWithLease
|
||||
? 'Use Force Push — remote only has older copies of local commits'
|
||||
: ahead === 0 && behind === 0
|
||||
? 'Branch is up to date'
|
||||
: describeSyncCounts(ahead, behind),
|
||||
disabled:
|
||||
globalBusy ||
|
||||
upstreamLoading ||
|
||||
!hasUpstream ||
|
||||
shouldForcePushWithLease ||
|
||||
(ahead === 0 && behind === 0)
|
||||
}
|
||||
|
||||
const fetchItem: DropdownItem = {
|
||||
|
|
@ -270,7 +325,7 @@ export function resolveDropdownItems(inputs: PrimaryActionInputs): DropdownEntry
|
|||
case 'needs_push':
|
||||
return 'Push first'
|
||||
case 'needs_sync':
|
||||
return 'Sync first'
|
||||
return shouldForcePushWithLease ? 'Force Push first' : 'Sync first'
|
||||
case 'auth_required':
|
||||
return 'Run gh auth login in this environment'
|
||||
case 'unsupported_provider':
|
||||
|
|
@ -298,16 +353,21 @@ export function resolveDropdownItems(inputs: PrimaryActionInputs): DropdownEntry
|
|||
!globalBusy &&
|
||||
!upstreamLoading &&
|
||||
hostedReviewCreation?.provider === 'github' &&
|
||||
hostedReviewCreation.blockedReason === 'needs_push'
|
||||
(hostedReviewCreation.blockedReason === 'needs_push' ||
|
||||
(hostedReviewCreation.blockedReason === 'needs_sync' && shouldForcePushWithLease))
|
||||
const pushCreatePRItem: DropdownItem = {
|
||||
kind: 'push_create_pr',
|
||||
label: 'Push & Create PR',
|
||||
title: canPushAndCreate ? 'Push local commits, then create a pull request' : createBlockedHint,
|
||||
label: shouldForcePushWithLease ? 'Force Push before PR' : 'Push before PR',
|
||||
title: canPushAndCreate
|
||||
? shouldForcePushWithLease
|
||||
? 'Force push with lease before creating a pull request'
|
||||
: 'Push local commits before creating a pull request'
|
||||
: createBlockedHint,
|
||||
hint: canPushAndCreate ? undefined : createBlockedHint,
|
||||
disabled: !canPushAndCreate
|
||||
}
|
||||
|
||||
return [
|
||||
const entries: DropdownEntry[] = [
|
||||
commitItem,
|
||||
commitPushItem,
|
||||
commitSyncItem,
|
||||
|
|
@ -320,4 +380,16 @@ export function resolveDropdownItems(inputs: PrimaryActionInputs): DropdownEntry
|
|||
fetchItem,
|
||||
publishItem
|
||||
]
|
||||
if (!isPullRequestOperationActive) {
|
||||
return entries
|
||||
}
|
||||
return entries.map((entry) =>
|
||||
entry.kind === 'separator'
|
||||
? entry
|
||||
: {
|
||||
...entry,
|
||||
title: 'Pull request operation in progress…',
|
||||
disabled: true
|
||||
}
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -218,6 +218,28 @@ describe('resolvePrimaryAction', () => {
|
|||
})
|
||||
})
|
||||
|
||||
it('returns Force Push when remote-only commits are patch-equivalent after a rebase', () => {
|
||||
const result = resolvePrimaryAction(
|
||||
inputs({
|
||||
branchCommitsAhead: 4,
|
||||
upstreamStatus: {
|
||||
hasUpstream: true,
|
||||
upstreamName: 'origin/feature',
|
||||
ahead: 14,
|
||||
behind: 3,
|
||||
behindCommitsArePatchEquivalent: true
|
||||
}
|
||||
})
|
||||
)
|
||||
expect(result).toEqual({
|
||||
kind: 'push',
|
||||
label: 'Force Push',
|
||||
title:
|
||||
'Remote only has older copies of local commits. Force push 4 branch commits with lease to update origin/feature.',
|
||||
disabled: false
|
||||
})
|
||||
})
|
||||
|
||||
it('returns Pull when clean + behind-only', () => {
|
||||
const result = resolvePrimaryAction(
|
||||
inputs({ upstreamStatus: { hasUpstream: true, ahead: 0, behind: 4 } })
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@
|
|||
|
||||
import type { HostedReviewCreationEligibility } from '../../../../shared/hosted-review'
|
||||
import type { GitUpstreamStatus, PRState } from '../../../../shared/types'
|
||||
import { shouldForcePushWithLeaseForUpstream } from '../../../../shared/git-upstream-status'
|
||||
|
||||
// Why: this module owns the pure state-machine logic for the Source Control
|
||||
// primary action (split button). Keeping the logic outside the React component
|
||||
|
|
@ -83,6 +84,12 @@ function describeSyncCounts(ahead: number, behind: number): string {
|
|||
return `Pull ${behind}, push ${ahead}`
|
||||
}
|
||||
|
||||
function describeForcePushWithLease(count: number | undefined, upstreamName?: string): string {
|
||||
const countText =
|
||||
count && count > 0 ? `${count} branch commit${count === 1 ? '' : 's'}` : 'this branch'
|
||||
return `Remote only has older copies of local commits. Force push ${countText} with lease to update ${upstreamName ?? 'the remote branch'}.`
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the primary split-button action.
|
||||
*
|
||||
|
|
@ -286,6 +293,14 @@ export function resolvePrimaryAction(inputs: PrimaryActionInputs): PrimaryAction
|
|||
}
|
||||
|
||||
if (upstreamStatus.ahead > 0 && upstreamStatus.behind > 0) {
|
||||
if (shouldForcePushWithLeaseForUpstream(upstreamStatus)) {
|
||||
return {
|
||||
kind: 'push',
|
||||
label: 'Force Push',
|
||||
title: describeForcePushWithLease(branchCommitsAhead, upstreamStatus.upstreamName),
|
||||
disabled: false
|
||||
}
|
||||
}
|
||||
return {
|
||||
kind: 'sync',
|
||||
label: 'Sync',
|
||||
|
|
|
|||
|
|
@ -121,7 +121,7 @@ describe('useGitStatusPolling', () => {
|
|||
hasUpstream: true,
|
||||
upstreamName: 'origin/main',
|
||||
ahead: 2,
|
||||
behind: 1
|
||||
behind: 0
|
||||
}
|
||||
})
|
||||
|
||||
|
|
@ -129,7 +129,7 @@ describe('useGitStatusPolling', () => {
|
|||
hasUpstream: true,
|
||||
upstreamName: 'origin/main',
|
||||
ahead: 2,
|
||||
behind: 1
|
||||
behind: 0
|
||||
})
|
||||
expect(state.fetchUpstreamStatus).not.toHaveBeenCalled()
|
||||
})
|
||||
|
|
|
|||
|
|
@ -273,22 +273,28 @@ export async function pullRuntimeGit(context: RuntimeGitContext): Promise<void>
|
|||
|
||||
export async function pushRuntimeGit(
|
||||
context: RuntimeGitContext,
|
||||
args: { publish?: boolean; pushTarget?: GitPushTarget } = {}
|
||||
args: { publish?: boolean; pushTarget?: GitPushTarget; forceWithLease?: boolean } = {}
|
||||
): Promise<void> {
|
||||
const target = getActiveRuntimeTarget(context.settings)
|
||||
if (target.kind === 'local' || !context.worktreeId) {
|
||||
await window.api.git.push({
|
||||
worktreePath: context.worktreePath,
|
||||
publish: args.publish,
|
||||
pushTarget: args.pushTarget,
|
||||
connectionId: context.connectionId
|
||||
connectionId: context.connectionId,
|
||||
...(args.publish !== undefined ? { publish: args.publish } : {}),
|
||||
...(args.pushTarget !== undefined ? { pushTarget: args.pushTarget } : {}),
|
||||
...(args.forceWithLease !== undefined ? { forceWithLease: args.forceWithLease } : {})
|
||||
})
|
||||
return
|
||||
}
|
||||
await callRuntimeRpc(
|
||||
target,
|
||||
'git.push',
|
||||
{ worktree: context.worktreeId, publish: args.publish, pushTarget: args.pushTarget },
|
||||
{
|
||||
worktree: context.worktreeId,
|
||||
...(args.publish !== undefined ? { publish: args.publish } : {}),
|
||||
...(args.pushTarget !== undefined ? { pushTarget: args.pushTarget } : {}),
|
||||
...(args.forceWithLease !== undefined ? { forceWithLease: args.forceWithLease } : {})
|
||||
},
|
||||
{ timeoutMs: 30_000 }
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1340,6 +1340,36 @@ describe('createEditorSlice remote branch actions', () => {
|
|||
expect(listener).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('updates subscribers when explicit upstream status adds patch equivalence', () => {
|
||||
const store = createEditorStore()
|
||||
store.getState().setUpstreamStatus('wt-1', {
|
||||
hasUpstream: true,
|
||||
upstreamName: 'origin/feature',
|
||||
ahead: 14,
|
||||
behind: 3
|
||||
})
|
||||
const listener = vi.fn()
|
||||
const unsubscribe = store.subscribe(listener)
|
||||
|
||||
store.getState().setUpstreamStatus('wt-1', {
|
||||
hasUpstream: true,
|
||||
upstreamName: 'origin/feature',
|
||||
ahead: 14,
|
||||
behind: 3,
|
||||
behindCommitsArePatchEquivalent: true
|
||||
})
|
||||
unsubscribe()
|
||||
|
||||
expect(listener).toHaveBeenCalled()
|
||||
expect(store.getState().remoteStatusesByWorktree['wt-1']).toEqual({
|
||||
hasUpstream: true,
|
||||
upstreamName: 'origin/feature',
|
||||
ahead: 14,
|
||||
behind: 3,
|
||||
behindCommitsArePatchEquivalent: true
|
||||
})
|
||||
})
|
||||
|
||||
it('runs pull and refreshes status + upstream on success', async () => {
|
||||
const store = createEditorStore()
|
||||
store.getState().setGitStatus('wt-1', {
|
||||
|
|
@ -1657,12 +1687,19 @@ describe('createEditorSlice remote branch actions', () => {
|
|||
// Why: guards against a no-op push round-trip after a pure fast-forward
|
||||
// pull. See syncBranch's ahead>0 guard in editor.ts.
|
||||
const store = createEditorStore()
|
||||
gitUpstreamStatusMock.mockResolvedValueOnce({
|
||||
hasUpstream: true,
|
||||
upstreamName: 'origin/main',
|
||||
ahead: 0,
|
||||
behind: 0
|
||||
})
|
||||
gitUpstreamStatusMock
|
||||
.mockResolvedValueOnce({
|
||||
hasUpstream: true,
|
||||
upstreamName: 'origin/main',
|
||||
ahead: 0,
|
||||
behind: 1
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
hasUpstream: true,
|
||||
upstreamName: 'origin/main',
|
||||
ahead: 0,
|
||||
behind: 0
|
||||
})
|
||||
|
||||
await store.getState().syncBranch('wt-1', '/repo')
|
||||
|
||||
|
|
@ -1672,6 +1709,29 @@ describe('createEditorSlice remote branch actions', () => {
|
|||
expect(toastErrorMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('force-pushes with lease instead of pulling when sync sees a stale rebased upstream', async () => {
|
||||
const store = createEditorStore()
|
||||
gitUpstreamStatusMock.mockResolvedValueOnce({
|
||||
hasUpstream: true,
|
||||
upstreamName: 'origin/feature',
|
||||
ahead: 14,
|
||||
behind: 3,
|
||||
behindCommitsArePatchEquivalent: true
|
||||
})
|
||||
|
||||
await store.getState().syncBranch('wt-1', '/repo')
|
||||
|
||||
expect(gitFetchMock).toHaveBeenCalled()
|
||||
expect(gitPullMock).not.toHaveBeenCalled()
|
||||
expect(gitPushMock).toHaveBeenCalledWith({
|
||||
worktreePath: '/repo',
|
||||
connectionId: undefined,
|
||||
forceWithLease: true
|
||||
})
|
||||
expect(gitUpstreamStatusMock).toHaveBeenCalledTimes(2)
|
||||
expect(toastErrorMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('surfaces a sync-labeled toast when syncBranch inner push fails with auth error', async () => {
|
||||
// Why: the user invoked Sync — the toast must read "Sync failed..." even
|
||||
// though the underlying step is push. Detail extraction still surfaces
|
||||
|
|
|
|||
|
|
@ -30,6 +30,7 @@ import type {
|
|||
import { stripCredentialsFromMessage } from '../../../../shared/git-remote-error'
|
||||
import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants'
|
||||
import type { RemoteOpKind } from '@/components/right-sidebar/source-control-primary-action'
|
||||
import { shouldForcePushWithLeaseForUpstream } from '../../../../shared/git-upstream-status'
|
||||
import {
|
||||
fetchRuntimeGit,
|
||||
getRuntimeGitUpstreamStatus,
|
||||
|
|
@ -418,7 +419,8 @@ export type EditorSlice = {
|
|||
worktreePath: string,
|
||||
publish?: boolean,
|
||||
connectionId?: string,
|
||||
pushTarget?: GitPushTarget
|
||||
pushTarget?: GitPushTarget,
|
||||
options?: { forceWithLease?: boolean }
|
||||
) => Promise<void>
|
||||
pullBranch: (worktreeId: string, worktreePath: string, connectionId?: string) => Promise<void>
|
||||
syncBranch: (
|
||||
|
|
@ -2674,7 +2676,14 @@ export const createEditorSlice: StateCreator<AppState, [], [], EditorSlice> = (s
|
|||
console.error('fetchUpstreamStatus failed', error)
|
||||
}
|
||||
},
|
||||
pushBranch: async (worktreeId, worktreePath, publish = false, connectionId, pushTarget) => {
|
||||
pushBranch: async (
|
||||
worktreeId,
|
||||
worktreePath,
|
||||
publish = false,
|
||||
connectionId,
|
||||
pushTarget,
|
||||
options = {}
|
||||
) => {
|
||||
// Why: don't *await* a post-op git status / upstream refresh here.
|
||||
// Chaining awaited refreshes inside the mutation extends the gap before
|
||||
// compound flows (runCompoundCommitAction → runRemoteAction) reach the
|
||||
|
|
@ -2688,7 +2697,7 @@ export const createEditorSlice: StateCreator<AppState, [], [], EditorSlice> = (s
|
|||
try {
|
||||
await pushRuntimeGit(
|
||||
{ settings: get().settings, worktreeId, worktreePath, connectionId },
|
||||
{ publish, pushTarget }
|
||||
{ publish, pushTarget, forceWithLease: options.forceWithLease }
|
||||
)
|
||||
} catch (error) {
|
||||
toast.error(resolveRemoteOperationErrorMessage(error, { publish, isPush: true }))
|
||||
|
|
@ -2732,24 +2741,36 @@ export const createEditorSlice: StateCreator<AppState, [], [], EditorSlice> = (s
|
|||
try {
|
||||
const context = { settings: get().settings, worktreeId, worktreePath, connectionId }
|
||||
await fetchRuntimeGit(context)
|
||||
await pullRuntimeGit(context)
|
||||
// Why: push only if the pull left local commits that aren't on the
|
||||
// remote. After a merge pull the ahead count can be >0 (local commits +
|
||||
// the new merge commit) or 0 (pure fast-forward), and we avoid a
|
||||
// no-op push round-trip in the fast-forward case.
|
||||
const upstreamStatus = await getRuntimeGitUpstreamStatus(context)
|
||||
if (upstreamStatus.ahead > 0) {
|
||||
const upstreamStatusBeforePull = await getRuntimeGitUpstreamStatus(context)
|
||||
if (shouldForcePushWithLeaseForUpstream(upstreamStatusBeforePull)) {
|
||||
try {
|
||||
await pushRuntimeGit(context, { pushTarget })
|
||||
await pushRuntimeGit(context, { pushTarget, forceWithLease: true })
|
||||
pushed = true
|
||||
} catch (error) {
|
||||
// Why: format under the user-facing operation (sync) rather than
|
||||
// the inner step (push) — the user clicked Sync and shouldn't see
|
||||
// a "Push failed" toast for a step they didn't directly invoke.
|
||||
toast.error(resolveRemoteOperationErrorMessage(error, { isSync: true }))
|
||||
pushStageToastShown = true
|
||||
throw error
|
||||
}
|
||||
} else {
|
||||
await pullRuntimeGit(context)
|
||||
// Why: push only if the pull left local commits that aren't on the
|
||||
// remote. After a merge pull the ahead count can be >0 (local commits +
|
||||
// the new merge commit) or 0 (pure fast-forward), and we avoid a
|
||||
// no-op push round-trip in the fast-forward case.
|
||||
const upstreamStatus = await getRuntimeGitUpstreamStatus(context)
|
||||
if (upstreamStatus.ahead > 0) {
|
||||
try {
|
||||
await pushRuntimeGit(context, { pushTarget })
|
||||
pushed = true
|
||||
} catch (error) {
|
||||
// Why: format under the user-facing operation (sync) rather than
|
||||
// the inner step (push) — the user clicked Sync and shouldn't see
|
||||
// a "Push failed" toast for a step they didn't directly invoke.
|
||||
toast.error(resolveRemoteOperationErrorMessage(error, { isSync: true }))
|
||||
pushStageToastShown = true
|
||||
throw error
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
if (!pushStageToastShown) {
|
||||
|
|
@ -3324,7 +3345,8 @@ function areUpstreamStatusesEqual(
|
|||
prev.hasUpstream === next.hasUpstream &&
|
||||
prev.upstreamName === next.upstreamName &&
|
||||
prev.ahead === next.ahead &&
|
||||
prev.behind === next.behind
|
||||
prev.behind === next.behind &&
|
||||
prev.behindCommitsArePatchEquivalent === next.behindCommitsArePatchEquivalent
|
||||
)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -56,6 +56,10 @@ export type GitUpstreamStatus = {
|
|||
upstreamName?: string
|
||||
ahead: number
|
||||
behind: number
|
||||
// Why: when a branch was rebased, the upstream-only commits can be older
|
||||
// patch-equivalent copies. Pulling them reintroduces stale history; a
|
||||
// lease-protected force push is the correct reconciliation.
|
||||
behindCommitsArePatchEquivalent?: boolean
|
||||
}
|
||||
|
||||
export type GitBranchChangeStatus = 'modified' | 'added' | 'deleted' | 'renamed' | 'copied'
|
||||
|
|
|
|||
|
|
@ -0,0 +1,20 @@
|
|||
import type { GitUpstreamStatus } from './git-status-types'
|
||||
|
||||
export function upstreamOnlyCommitsArePatchEquivalent(cherryMarkOutput: string): boolean {
|
||||
const lines = cherryMarkOutput
|
||||
.split(/\r?\n/)
|
||||
.map((line) => line.trim())
|
||||
.filter(Boolean)
|
||||
return lines.length > 0 && lines.every((line) => line.startsWith('='))
|
||||
}
|
||||
|
||||
export function shouldForcePushWithLeaseForUpstream(
|
||||
status: GitUpstreamStatus | undefined
|
||||
): boolean {
|
||||
return (
|
||||
status?.hasUpstream === true &&
|
||||
status.ahead > 0 &&
|
||||
status.behind > 0 &&
|
||||
status.behindCommitsArePatchEquivalent === true
|
||||
)
|
||||
}
|
||||
|
|
@ -194,6 +194,8 @@ export type GitPushTarget = {
|
|||
remoteName: string
|
||||
branchName: string
|
||||
remoteUrl?: string
|
||||
/** True when Orca added this remote while preparing a fork-PR worktree. */
|
||||
remoteCreated?: boolean
|
||||
}
|
||||
|
||||
// ─── Worktree metadata (persisted user-authored fields only) ─────────
|
||||
|
|
|
|||
|
|
@ -45,7 +45,6 @@ async function openSourceControl(page: Page, expectedWorktreeId: string): Promis
|
|||
)
|
||||
.toBe(true)
|
||||
await expect(page.getByRole('button', { name: /Source Control/ })).toBeVisible()
|
||||
await expect(page.getByRole('textbox', { name: 'Commit message' })).toBeVisible()
|
||||
}
|
||||
|
||||
async function forceCreatePREligibleStatus(
|
||||
|
|
@ -181,9 +180,7 @@ test.describe('Source Control create pull request', () => {
|
|||
await waitForActiveWorktree(orcaPage)
|
||||
})
|
||||
|
||||
test('opens the PR dialog from Source Control and creates the pull request', async ({
|
||||
orcaPage
|
||||
}) => {
|
||||
test('creates the pull request from the Source Control primary action', async ({ orcaPage }) => {
|
||||
const { branch, worktreeId } = await seedCreatePREligibleBranch(orcaPage)
|
||||
await openSourceControl(orcaPage, worktreeId)
|
||||
await forceCreatePREligibleStatus(orcaPage, worktreeId, branch)
|
||||
|
|
@ -191,19 +188,28 @@ test.describe('Source Control create pull request', () => {
|
|||
const createButton = orcaPage.getByRole('button', { name: 'Create PR' })
|
||||
await expect(createButton).toBeVisible({ timeout: 10_000 })
|
||||
await expect(createButton).toBeEnabled()
|
||||
await expect(orcaPage.getByRole('textbox', { name: 'Pull request title' })).toHaveValue(
|
||||
'Create PR from E2E'
|
||||
)
|
||||
await expect(orcaPage.getByRole('textbox', { name: 'Pull request base branch' })).toHaveValue(
|
||||
'main'
|
||||
)
|
||||
await expect(orcaPage.getByRole('textbox', { name: 'Pull request description' })).toHaveValue(
|
||||
'- Initial commit for E2E'
|
||||
)
|
||||
await createButton.click()
|
||||
|
||||
const dialog = orcaPage.getByRole('dialog', { name: 'Create Pull Request' })
|
||||
await expect(dialog).toBeVisible()
|
||||
await expect(dialog).toContainText(branch)
|
||||
await expect(dialog.getByLabel('Base branch')).toHaveValue('main')
|
||||
await expect(dialog.getByLabel('Title')).toHaveValue('Create PR from E2E')
|
||||
await expect(dialog.getByLabel('Description')).toHaveValue('- Initial commit for E2E')
|
||||
|
||||
await dialog.getByRole('button', { name: 'Create PR' }).click()
|
||||
|
||||
await expect(dialog).toBeHidden({ timeout: 10_000 })
|
||||
await expect(orcaPage.getByText('Create PR from E2E')).toBeVisible({ timeout: 10_000 })
|
||||
await expect
|
||||
.poll(
|
||||
() =>
|
||||
orcaPage.evaluate(
|
||||
() =>
|
||||
(window as unknown as { __createPRPayloads: CreatePRPayload[] }).__createPRPayloads
|
||||
.length
|
||||
),
|
||||
{ timeout: 10_000 }
|
||||
)
|
||||
.toBe(1)
|
||||
|
||||
const payloads = await orcaPage.evaluate(
|
||||
() => (window as unknown as { __createPRPayloads: CreatePRPayload[] }).__createPRPayloads
|
||||
|
|
|
|||
Loading…
Reference in New Issue