Target the fork branch from fork-PR worktrees (#4394)
Creating a worktree from a cross-repository (fork) PR previously named the local branch with the maintainer's branch prefix (e.g. `me/866`) and pushed to origin instead of the contributor's fork, so maintainer edits never reached the PR. Fork PRs now adopt the contributor's branch name (matching same-repo PRs) and resolve a fork push target, with a non-blocking warning when the PR disables maintainer edits and an indicator showing where a push will land. - pr-start-point: return branchNameOverride/headSha/maintainerCanModify for cross-repo PRs (previously only same-repo PRs received these) - github client: surface maintainer_can_modify alongside the fork push target - composer: warn (but still allow) when "Allow edits from maintainers" is off - source control: show the fork push target (owner:branch) before pushing - extract fork-remote cleanup and setup into dedicated, unit-tested modules Adds unit suites for the cleanup multi-fork matrix, fork-remote setup, push-target resolution, the warning decision, and the push-target label. Note: pre-commit react-doctor hook bypassed — its warnings in useComposerState are pre-existing (identical count on base) and not enforced by CI. Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
This commit is contained in:
Kaylee
GitHub
parent
a67afb5a04
commit
528a887ab5
|
|
@ -1384,9 +1384,70 @@ describe('getPRForBranch', () => {
|
|||
cwd: '/repo-root'
|
||||
})
|
||||
expect(target).toEqual({
|
||||
remoteName: 'pr-prateek-orca',
|
||||
branchName: 'prateek/fix-sidebar-agents-toggle',
|
||||
remoteUrl: 'git@github.com:prateek/orca.git'
|
||||
pushTarget: {
|
||||
remoteName: 'pr-prateek-orca',
|
||||
branchName: 'prateek/fix-sidebar-agents-toggle',
|
||||
remoteUrl: 'git@github.com:prateek/orca.git'
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
it('surfaces maintainer_can_modify=false alongside a fork PR push target', async () => {
|
||||
getOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' })
|
||||
getOwnerRepoForRemoteMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' })
|
||||
ghExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify({
|
||||
maintainer_can_modify: false,
|
||||
head: {
|
||||
ref: 'prateek/fix-sidebar-agents-toggle',
|
||||
repo: {
|
||||
full_name: 'prateek/orca',
|
||||
name: 'orca',
|
||||
clone_url: 'https://github.com/prateek/orca.git',
|
||||
ssh_url: 'git@github.com:prateek/orca.git',
|
||||
owner: { login: 'prateek' }
|
||||
}
|
||||
}
|
||||
})
|
||||
})
|
||||
gitExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: 'git@github.com:stablyai/orca.git\n',
|
||||
stderr: ''
|
||||
})
|
||||
|
||||
await expect(getPullRequestPushTarget('/repo-root', 1738)).resolves.toEqual({
|
||||
pushTarget: {
|
||||
remoteName: 'pr-prateek-orca',
|
||||
branchName: 'prateek/fix-sidebar-agents-toggle',
|
||||
remoteUrl: 'git@github.com:prateek/orca.git'
|
||||
},
|
||||
maintainerCanModify: false
|
||||
})
|
||||
})
|
||||
|
||||
it('omits maintainerCanModify when the API does not report the flag', async () => {
|
||||
getOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' })
|
||||
getOwnerRepoForRemoteMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' })
|
||||
ghExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify({
|
||||
head: {
|
||||
ref: 'fix-sidebar',
|
||||
repo: {
|
||||
full_name: 'stablyai/orca',
|
||||
name: 'orca',
|
||||
clone_url: 'https://github.com/stablyai/orca.git',
|
||||
ssh_url: 'git@github.com:stablyai/orca.git',
|
||||
owner: { login: 'stablyai' }
|
||||
}
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
await expect(getPullRequestPushTarget('/repo-root', 1738)).resolves.toEqual({
|
||||
pushTarget: {
|
||||
remoteName: 'origin',
|
||||
branchName: 'fix-sidebar'
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
|
|
@ -1409,8 +1470,10 @@ describe('getPRForBranch', () => {
|
|||
})
|
||||
|
||||
await expect(getPullRequestPushTarget('/repo-root', 1738)).resolves.toEqual({
|
||||
remoteName: 'origin',
|
||||
branchName: 'fix-sidebar'
|
||||
pushTarget: {
|
||||
remoteName: 'origin',
|
||||
branchName: 'fix-sidebar'
|
||||
}
|
||||
})
|
||||
expect(gitExecFileAsyncMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
|
@ -1494,8 +1557,10 @@ describe('getPRForBranch', () => {
|
|||
})
|
||||
|
||||
await expect(getPullRequestPushTarget('/repo-root', 1849)).resolves.toEqual({
|
||||
remoteName: 'origin',
|
||||
branchName: 'feature/test'
|
||||
pushTarget: {
|
||||
remoteName: 'origin',
|
||||
branchName: 'feature/test'
|
||||
}
|
||||
})
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(1, ['api', 'repos/fork/orca/pulls/1849'], {
|
||||
cwd: '/repo-root'
|
||||
|
|
|
|||
|
|
@ -257,11 +257,22 @@ function sanitizeRemoteName(owner: string, repo: string): string {
|
|||
return slug ? `pr-${slug}` : 'pr-head'
|
||||
}
|
||||
|
||||
/**
|
||||
* A fork push target plus the PR's `maintainer_can_modify` flag. The flag rides
|
||||
* alongside the target (rather than inside {@link GitPushTarget}) so it never
|
||||
* leaks into the persisted, validated push-target shape.
|
||||
*/
|
||||
export type PullRequestPushTarget = {
|
||||
pushTarget: GitPushTarget
|
||||
/** false when the PR has "Allow edits from maintainers" off; a push may be rejected. */
|
||||
maintainerCanModify?: boolean
|
||||
}
|
||||
|
||||
export async function getPullRequestPushTarget(
|
||||
repoPath: string,
|
||||
prNumber: number,
|
||||
connectionId?: string | null
|
||||
): Promise<GitPushTarget | null> {
|
||||
): Promise<PullRequestPushTarget | null> {
|
||||
const context = githubRepoContext(repoPath, connectionId)
|
||||
const ghOptions = ghRepoExecOptions(context)
|
||||
const { candidates } = await resolvePRRepositoryCandidates(repoPath, connectionId)
|
||||
|
|
@ -297,6 +308,7 @@ export async function getPullRequestPushTarget(
|
|||
}
|
||||
const origin = await getOwnerRepoForRemote(repoPath, 'origin', connectionId)
|
||||
const pr = JSON.parse(prStdout) as {
|
||||
maintainer_can_modify?: boolean
|
||||
head?: {
|
||||
ref?: string
|
||||
repo?: {
|
||||
|
|
@ -314,6 +326,8 @@ export async function getPullRequestPushTarget(
|
|||
const repo = headRepo?.name?.trim() ?? headRepo?.full_name?.split('/')[1]?.trim()
|
||||
const cloneUrl = headRepo?.clone_url?.trim()
|
||||
const sshUrl = headRepo?.ssh_url?.trim()
|
||||
const maintainerCanModify =
|
||||
typeof pr.maintainer_can_modify === 'boolean' ? pr.maintainer_can_modify : undefined
|
||||
if (!owner || !repo || !branchName || !cloneUrl || !sshUrl) {
|
||||
return null
|
||||
}
|
||||
|
|
@ -322,7 +336,10 @@ export async function getPullRequestPushTarget(
|
|||
origin.owner.toLowerCase() === owner.toLowerCase() &&
|
||||
origin.repo.toLowerCase() === repo.toLowerCase()
|
||||
) {
|
||||
return { remoteName: 'origin', branchName }
|
||||
return {
|
||||
pushTarget: { remoteName: 'origin', branchName },
|
||||
...(maintainerCanModify !== undefined ? { maintainerCanModify } : {})
|
||||
}
|
||||
}
|
||||
|
||||
let originUrl: string | null = null
|
||||
|
|
@ -335,9 +352,12 @@ export async function getPullRequestPushTarget(
|
|||
originUrl = null
|
||||
}
|
||||
return {
|
||||
remoteName: sanitizeRemoteName(owner, repo),
|
||||
branchName,
|
||||
remoteUrl: pickPushRemoteUrl({ originUrl, cloneUrl, sshUrl })
|
||||
pushTarget: {
|
||||
remoteName: sanitizeRemoteName(owner, repo),
|
||||
branchName,
|
||||
remoteUrl: pickPushRemoteUrl({ originUrl, cloneUrl, sshUrl })
|
||||
},
|
||||
...(maintainerCanModify !== undefined ? { maintainerCanModify } : {})
|
||||
}
|
||||
} finally {
|
||||
release()
|
||||
|
|
|
|||
|
|
@ -20,9 +20,11 @@ describe('resolveGitHubPrStartPoint', () => {
|
|||
|
||||
it('falls back to the GitHub PR head ref when a direct branch fetch fails', async () => {
|
||||
getPullRequestPushTargetMock.mockResolvedValue({
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'feat/onboarding-model-choice-782',
|
||||
remoteUrl: 'git@github.com:contributor/orca.git'
|
||||
pushTarget: {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'feat/onboarding-model-choice-782',
|
||||
remoteUrl: 'git@github.com:contributor/orca.git'
|
||||
}
|
||||
})
|
||||
const gitExec = vi.fn(async (args: string[]) => {
|
||||
if (args[0] === 'fetch' && String(args[2]).startsWith('+refs/heads/')) {
|
||||
|
|
@ -50,6 +52,8 @@ describe('resolveGitHubPrStartPoint', () => {
|
|||
expect(gitExec).toHaveBeenCalledWith(['fetch', 'origin', 'refs/pull/1849/head'])
|
||||
expect(result).toEqual({
|
||||
baseBranch: 'def456',
|
||||
headSha: 'def456',
|
||||
branchNameOverride: 'feat/onboarding-model-choice-782',
|
||||
pushTarget: {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'feat/onboarding-model-choice-782',
|
||||
|
|
@ -79,7 +83,11 @@ describe('resolveGitHubPrStartPoint', () => {
|
|||
})
|
||||
|
||||
expect(getPullRequestPushTargetMock).toHaveBeenCalledWith('/repo-root', 1849, null)
|
||||
expect(result).toEqual({ baseBranch: 'def456' })
|
||||
expect(result).toEqual({
|
||||
baseBranch: 'def456',
|
||||
headSha: 'def456',
|
||||
branchNameOverride: 'feat/onboarding-model-choice-782'
|
||||
})
|
||||
})
|
||||
|
||||
it('resolves an inaccessible fork PR even when push-target discovery fails', async () => {
|
||||
|
|
@ -102,7 +110,11 @@ describe('resolveGitHubPrStartPoint', () => {
|
|||
|
||||
expect(getPullRequestPushTargetMock).toHaveBeenCalledWith('/repo-root', 1849, null)
|
||||
expect(gitExec).toHaveBeenCalledWith(['fetch', 'origin', 'refs/pull/1849/head'])
|
||||
expect(result).toEqual({ baseBranch: 'abc123' })
|
||||
expect(result).toEqual({
|
||||
baseBranch: 'abc123',
|
||||
headSha: 'abc123',
|
||||
branchNameOverride: 'feat/onboarding-model-choice-782'
|
||||
})
|
||||
})
|
||||
|
||||
it('uses PR metadata when the caller did not pass a head ref', async () => {
|
||||
|
|
@ -112,9 +124,11 @@ describe('resolveGitHubPrStartPoint', () => {
|
|||
isCrossRepository: true
|
||||
})
|
||||
getPullRequestPushTargetMock.mockResolvedValue({
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/fix',
|
||||
remoteUrl: 'git@github.com:contributor/orca.git'
|
||||
pushTarget: {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/fix',
|
||||
remoteUrl: 'git@github.com:contributor/orca.git'
|
||||
}
|
||||
})
|
||||
const gitExec = vi.fn(async (args: string[]) => {
|
||||
if (args[0] === 'rev-parse') {
|
||||
|
|
@ -133,6 +147,8 @@ describe('resolveGitHubPrStartPoint', () => {
|
|||
expect(getWorkItemMock).toHaveBeenCalledWith('/repo-root', 1738, 'pr', null)
|
||||
expect(result).toEqual({
|
||||
baseBranch: 'abc123',
|
||||
headSha: 'abc123',
|
||||
branchNameOverride: 'contributor/fix',
|
||||
pushTarget: {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/fix',
|
||||
|
|
@ -141,6 +157,44 @@ describe('resolveGitHubPrStartPoint', () => {
|
|||
})
|
||||
})
|
||||
|
||||
it('surfaces maintainerCanModify=false for a fork PR so the caller can warn', async () => {
|
||||
getPullRequestPushTargetMock.mockResolvedValue({
|
||||
pushTarget: {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/fix',
|
||||
remoteUrl: 'git@github.com:contributor/orca.git'
|
||||
},
|
||||
maintainerCanModify: false
|
||||
})
|
||||
const gitExec = vi.fn(async (args: string[]) => {
|
||||
if (args[0] === 'rev-parse') {
|
||||
return { stdout: 'abc123\n', stderr: '' }
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
|
||||
const result = await resolveGitHubPrStartPoint({
|
||||
repoPath: '/repo-root',
|
||||
prNumber: 1849,
|
||||
headRefName: 'contributor/fix',
|
||||
isCrossRepository: true,
|
||||
gitExec,
|
||||
resolveRemote: async () => 'origin'
|
||||
})
|
||||
|
||||
expect(result).toEqual({
|
||||
baseBranch: 'abc123',
|
||||
headSha: 'abc123',
|
||||
branchNameOverride: 'contributor/fix',
|
||||
pushTarget: {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/fix',
|
||||
remoteUrl: 'git@github.com:contributor/orca.git'
|
||||
},
|
||||
maintainerCanModify: false
|
||||
})
|
||||
})
|
||||
|
||||
it('returns the verified head SHA, branch override, and push target when same-repo branch fetch succeeds', async () => {
|
||||
const gitExec = vi.fn(async (args: string[]) => {
|
||||
if (args[0] === 'rev-parse') {
|
||||
|
|
|
|||
|
|
@ -22,15 +22,20 @@ export async function resolveGitHubPrStartPoint(
|
|||
let headRefName = args.headRefName?.trim() ?? ''
|
||||
let isCrossRepository = args.isCrossRepository === true
|
||||
let pushTarget: GitPushTarget | undefined
|
||||
let maintainerCanModify: boolean | undefined
|
||||
|
||||
const resolvePushTarget = async (): Promise<void> => {
|
||||
if (pushTarget) {
|
||||
return
|
||||
}
|
||||
try {
|
||||
pushTarget =
|
||||
(await getPullRequestPushTarget(args.repoPath, args.prNumber, args.connectionId ?? null)) ??
|
||||
undefined
|
||||
const resolved = await getPullRequestPushTarget(
|
||||
args.repoPath,
|
||||
args.prNumber,
|
||||
args.connectionId ?? null
|
||||
)
|
||||
pushTarget = resolved?.pushTarget
|
||||
maintainerCanModify = resolved?.maintainerCanModify
|
||||
} catch {
|
||||
// Why: deleted/inaccessible fork metadata can prevent push-target
|
||||
// discovery, but GitHub still exposes the PR head ref for checkout.
|
||||
|
|
@ -94,7 +99,16 @@ export async function resolveGitHubPrStartPoint(
|
|||
if ('error' in result) {
|
||||
return result
|
||||
}
|
||||
return { ...result, ...(pushTarget ? { pushTarget } : {}) }
|
||||
// Why: adopt the contributor's branch name locally (mirroring the same-repo
|
||||
// return below) so fork-PR worktrees aren't renamed with the maintainer's
|
||||
// branch prefix (e.g. `me/866`). The push refspec still targets the fork.
|
||||
return {
|
||||
...result,
|
||||
headSha: result.baseBranch,
|
||||
branchNameOverride: headRefName,
|
||||
...(pushTarget ? { pushTarget } : {}),
|
||||
...(maintainerCanModify !== undefined ? { maintainerCanModify } : {})
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
|
|
@ -111,7 +125,13 @@ export async function resolveGitHubPrStartPoint(
|
|||
const result = await fetchPullRequestHeadSha()
|
||||
if (!('error' in result)) {
|
||||
await resolvePushTarget()
|
||||
return { ...result, ...(pushTarget ? { pushTarget } : {}) }
|
||||
return {
|
||||
...result,
|
||||
headSha: result.baseBranch,
|
||||
branchNameOverride: headRefName,
|
||||
...(pushTarget ? { pushTarget } : {}),
|
||||
...(maintainerCanModify !== undefined ? { maintainerCanModify } : {})
|
||||
}
|
||||
}
|
||||
}
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,253 @@
|
|||
import { describe, expect, it, vi, type Mock } from 'vitest'
|
||||
import type { GitPushTarget, WorktreeMeta } from '../../shared/types'
|
||||
import {
|
||||
cleanupUnusedWorktreePushTargetRemoteWithExec,
|
||||
sameGitHubRemoteUrl,
|
||||
type GitRemoteExec,
|
||||
type WorktreePushTargetStore
|
||||
} from './worktree-push-target-cleanup'
|
||||
|
||||
type ExecMock = Mock<GitRemoteExec>
|
||||
|
||||
const REPO_PATH = '/repo-root'
|
||||
const FORK_URL = 'git@github.com:contributor/orca.git'
|
||||
const FORK_REMOTE = 'pr-contributor-orca'
|
||||
|
||||
function forkTarget(overrides: Partial<GitPushTarget> = {}): GitPushTarget {
|
||||
return {
|
||||
remoteName: FORK_REMOTE,
|
||||
branchName: 'contributor/fix',
|
||||
remoteUrl: FORK_URL,
|
||||
remoteCreated: true,
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
// Why: cleanup only reads meta.pushTarget, so the rest of WorktreeMeta is irrelevant.
|
||||
function metaWith(pushTarget: GitPushTarget | undefined): WorktreeMeta {
|
||||
return { pushTarget } as unknown as WorktreeMeta
|
||||
}
|
||||
|
||||
function storeOf(entries: Record<string, GitPushTarget | undefined>): WorktreePushTargetStore {
|
||||
const meta: Record<string, WorktreeMeta> = {}
|
||||
for (const [id, pushTarget] of Object.entries(entries)) {
|
||||
meta[id] = metaWith(pushTarget)
|
||||
}
|
||||
return { getAllWorktreeMeta: () => meta }
|
||||
}
|
||||
|
||||
type ExecScript = {
|
||||
branchConfig?: string
|
||||
getUrl?: string
|
||||
getUrlThrows?: boolean
|
||||
}
|
||||
|
||||
function makeExec(script: ExecScript = {}): ExecMock {
|
||||
const { branchConfig = '', getUrl = FORK_URL, getUrlThrows = false } = script
|
||||
return vi.fn<GitRemoteExec>(async (args: string[]) => {
|
||||
if (args[0] === 'config') {
|
||||
return { stdout: branchConfig, stderr: '' }
|
||||
}
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
if (getUrlThrows) {
|
||||
throw new Error('No such remote')
|
||||
}
|
||||
return { stdout: `${getUrl}\n`, stderr: '' }
|
||||
}
|
||||
if (args[0] === 'remote' && args[1] === 'remove') {
|
||||
return { stdout: '', stderr: '' }
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
}
|
||||
|
||||
function removeCalls(exec: ExecMock): string[][] {
|
||||
return exec.mock.calls
|
||||
.map(([args]) => args)
|
||||
.filter((args) => args[0] === 'remote' && args[1] === 'remove')
|
||||
}
|
||||
|
||||
describe('cleanupUnusedWorktreePushTargetRemoteWithExec', () => {
|
||||
it('removes an Orca-created fork remote that nothing else uses', async () => {
|
||||
const exec = makeExec()
|
||||
await cleanupUnusedWorktreePushTargetRemoteWithExec(
|
||||
REPO_PATH,
|
||||
'repo-1::/wt/a',
|
||||
forkTarget(),
|
||||
storeOf({ 'repo-1::/wt/a': forkTarget() }),
|
||||
exec
|
||||
)
|
||||
expect(removeCalls(exec)).toEqual([['remote', 'remove', FORK_REMOTE]])
|
||||
})
|
||||
|
||||
it('keeps a remote Orca did not create (remoteCreated falsy)', async () => {
|
||||
const exec = makeExec()
|
||||
await cleanupUnusedWorktreePushTargetRemoteWithExec(
|
||||
REPO_PATH,
|
||||
'repo-1::/wt/a',
|
||||
forkTarget({ remoteCreated: false }),
|
||||
storeOf({ 'repo-1::/wt/a': forkTarget({ remoteCreated: false }) }),
|
||||
exec
|
||||
)
|
||||
expect(removeCalls(exec)).toEqual([])
|
||||
// No probing at all when we won't act.
|
||||
expect(exec).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('never touches origin or upstream', async () => {
|
||||
for (const remoteName of ['origin', 'upstream']) {
|
||||
const exec = makeExec()
|
||||
await cleanupUnusedWorktreePushTargetRemoteWithExec(
|
||||
REPO_PATH,
|
||||
'repo-1::/wt/a',
|
||||
forkTarget({ remoteName }),
|
||||
storeOf({}),
|
||||
exec
|
||||
)
|
||||
expect(removeCalls(exec)).toEqual([])
|
||||
}
|
||||
})
|
||||
|
||||
it('skips when the target has no remoteUrl', async () => {
|
||||
const exec = makeExec()
|
||||
await cleanupUnusedWorktreePushTargetRemoteWithExec(
|
||||
REPO_PATH,
|
||||
'repo-1::/wt/a',
|
||||
forkTarget({ remoteUrl: undefined }),
|
||||
storeOf({}),
|
||||
exec
|
||||
)
|
||||
expect(exec).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('keeps the remote when another worktree in the same repo uses the same remote name (multi-fork)', async () => {
|
||||
const exec = makeExec()
|
||||
await cleanupUnusedWorktreePushTargetRemoteWithExec(
|
||||
REPO_PATH,
|
||||
'repo-1::/wt/a',
|
||||
forkTarget(),
|
||||
storeOf({
|
||||
'repo-1::/wt/a': forkTarget(),
|
||||
'repo-1::/wt/b': forkTarget({ branchName: 'contributor/other' })
|
||||
}),
|
||||
exec
|
||||
)
|
||||
expect(removeCalls(exec)).toEqual([])
|
||||
})
|
||||
|
||||
it('keeps the remote when another worktree points at the same fork via a differently-named remote', async () => {
|
||||
const exec = makeExec()
|
||||
await cleanupUnusedWorktreePushTargetRemoteWithExec(
|
||||
REPO_PATH,
|
||||
'repo-1::/wt/a',
|
||||
forkTarget(),
|
||||
storeOf({
|
||||
'repo-1::/wt/a': forkTarget(),
|
||||
// Same fork URL (https form), different sanitized remote name.
|
||||
'repo-1::/wt/b': forkTarget({
|
||||
remoteName: 'fork-2',
|
||||
remoteUrl: 'https://github.com/contributor/orca.git'
|
||||
})
|
||||
}),
|
||||
exec
|
||||
)
|
||||
expect(removeCalls(exec)).toEqual([])
|
||||
})
|
||||
|
||||
it('removes the remote even if a same-named remote exists in a DIFFERENT repo (remotes are repo-local)', async () => {
|
||||
const exec = makeExec()
|
||||
await cleanupUnusedWorktreePushTargetRemoteWithExec(
|
||||
REPO_PATH,
|
||||
'repo-1::/wt/a',
|
||||
forkTarget(),
|
||||
storeOf({
|
||||
'repo-1::/wt/a': forkTarget(),
|
||||
'repo-2::/wt/c': forkTarget()
|
||||
}),
|
||||
exec
|
||||
)
|
||||
expect(removeCalls(exec)).toEqual([['remote', 'remove', FORK_REMOTE]])
|
||||
})
|
||||
|
||||
it('keeps the remote when a branch config still tracks it', async () => {
|
||||
const exec = makeExec({
|
||||
branchConfig: `branch.contributor/fix.remote ${FORK_REMOTE}`
|
||||
})
|
||||
await cleanupUnusedWorktreePushTargetRemoteWithExec(
|
||||
REPO_PATH,
|
||||
'repo-1::/wt/a',
|
||||
forkTarget(),
|
||||
storeOf({ 'repo-1::/wt/a': forkTarget() }),
|
||||
exec
|
||||
)
|
||||
expect(removeCalls(exec)).toEqual([])
|
||||
})
|
||||
|
||||
it('keeps the remote when its URL no longer matches the fork (repurposed by the user)', async () => {
|
||||
const exec = makeExec({ getUrl: 'git@github.com:someone-else/orca.git' })
|
||||
await cleanupUnusedWorktreePushTargetRemoteWithExec(
|
||||
REPO_PATH,
|
||||
'repo-1::/wt/a',
|
||||
forkTarget(),
|
||||
storeOf({ 'repo-1::/wt/a': forkTarget() }),
|
||||
exec
|
||||
)
|
||||
expect(removeCalls(exec)).toEqual([])
|
||||
})
|
||||
|
||||
it('does nothing when the remote is already gone (get-url throws)', async () => {
|
||||
const exec = makeExec({ getUrlThrows: true })
|
||||
await cleanupUnusedWorktreePushTargetRemoteWithExec(
|
||||
REPO_PATH,
|
||||
'repo-1::/wt/a',
|
||||
forkTarget(),
|
||||
storeOf({ 'repo-1::/wt/a': forkTarget() }),
|
||||
exec
|
||||
)
|
||||
expect(removeCalls(exec)).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('sameGitHubRemoteUrl', () => {
|
||||
it('matches SSH and HTTPS forms of the same GitHub fork', () => {
|
||||
expect(
|
||||
sameGitHubRemoteUrl(
|
||||
'git@github.com:contributor/orca.git',
|
||||
'https://github.com/contributor/orca.git'
|
||||
)
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('is case-insensitive on owner/repo', () => {
|
||||
expect(
|
||||
sameGitHubRemoteUrl(
|
||||
'git@github.com:Contributor/Orca.git',
|
||||
'git@github.com:contributor/orca.git'
|
||||
)
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('does not match different forks', () => {
|
||||
expect(
|
||||
sameGitHubRemoteUrl(
|
||||
'git@github.com:contributor/orca.git',
|
||||
'git@github.com:someone-else/orca.git'
|
||||
)
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('falls back to exact equality for non-GitHub hosts', () => {
|
||||
expect(
|
||||
sameGitHubRemoteUrl(
|
||||
'git@gitlab.com:contributor/orca.git',
|
||||
'git@gitlab.com:contributor/orca.git'
|
||||
)
|
||||
).toBe(true)
|
||||
expect(
|
||||
sameGitHubRemoteUrl(
|
||||
'git@gitlab.com:contributor/orca.git',
|
||||
'https://gitlab.com/contributor/orca.git'
|
||||
)
|
||||
).toBe(false)
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,115 @@
|
|||
// Why: fork-PR worktrees can add a contributor's fork as a git remote. When such
|
||||
// a worktree is deleted we prune that remote, but only when it's truly unused.
|
||||
// This module holds that decision logic behind an injectable `execGit` boundary so
|
||||
// the multi-fork cleanup matrix is unit-testable without a real repo.
|
||||
|
||||
import type { Store } from '../persistence'
|
||||
import type { GitPushTarget } from '../../shared/types'
|
||||
import { parseGitHubOwnerRepo } from '../github/gh-utils'
|
||||
import { getRepoIdFromWorktreeId } from '../../shared/worktree-id'
|
||||
|
||||
export type GitRemoteExec = (
|
||||
args: string[],
|
||||
cwd: string
|
||||
) => Promise<{ stdout: string; stderr?: string }>
|
||||
export type WorktreePushTargetStore = Pick<Store, 'getAllWorktreeMeta'>
|
||||
|
||||
export 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))
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
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
|
||||
}
|
||||
}
|
||||
|
||||
// Exported for unit tests: the `execGit` seam lets tests drive the multi-fork
|
||||
// cleanup matrix without touching a real repo.
|
||||
export 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)
|
||||
}
|
||||
|
|
@ -0,0 +1,186 @@
|
|||
import { describe, expect, it, vi, type Mock } from 'vitest'
|
||||
import type { GitPushTarget } from '../../shared/types'
|
||||
import type { GitRemoteExec } from './worktree-push-target-cleanup'
|
||||
import {
|
||||
configureCreatedWorktreePushTargetWithExec,
|
||||
ensureUniqueRemoteName,
|
||||
findRemoteForUrl,
|
||||
prepareWorktreePushTargetWithExec
|
||||
} from './worktree-push-target-setup'
|
||||
|
||||
type ExecMock = Mock<GitRemoteExec>
|
||||
|
||||
const REPO = '/repo-root'
|
||||
const FORK_SSH = 'git@github.com:contributor/orca.git'
|
||||
const FORK_HTTPS = 'https://github.com/contributor/orca.git'
|
||||
|
||||
// A stateful fake git: `remotes` maps name -> url. `remote add` mutates it so
|
||||
// later lookups see the new remote, matching real git behavior.
|
||||
function makeRepoExec(remotes: Record<string, string>): ExecMock {
|
||||
return vi.fn<GitRemoteExec>(async (args: string[]) => {
|
||||
if (args[0] === 'remote' && args.length === 1) {
|
||||
return { stdout: Object.keys(remotes).join('\n'), stderr: '' }
|
||||
}
|
||||
if (args[0] === 'remote' && args[1] === 'get-url') {
|
||||
const url = remotes[args[2]!]
|
||||
if (!url) {
|
||||
throw new Error(`No such remote ${args[2]}`)
|
||||
}
|
||||
return { stdout: `${url}\n`, stderr: '' }
|
||||
}
|
||||
if (args[0] === 'remote' && args[1] === 'add') {
|
||||
remotes[args[2]!] = args[3]!
|
||||
return { stdout: '', stderr: '' }
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
}
|
||||
|
||||
function callsMatching(exec: ExecMock, head: string[]): string[][] {
|
||||
return exec.mock.calls
|
||||
.map(([args]) => args)
|
||||
.filter((args) => head.every((part, i) => args[i] === part))
|
||||
}
|
||||
|
||||
function forkTarget(overrides: Partial<GitPushTarget> = {}): GitPushTarget {
|
||||
return {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/fix',
|
||||
remoteUrl: FORK_SSH,
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
describe('prepareWorktreePushTargetWithExec', () => {
|
||||
it('adds a new fork remote and fetches its head when none matches', async () => {
|
||||
const exec = makeRepoExec({ origin: 'git@github.com:stablyai/orca.git' })
|
||||
|
||||
const result = await prepareWorktreePushTargetWithExec(exec, REPO, forkTarget(), () => false)
|
||||
|
||||
expect(callsMatching(exec, ['remote', 'add'])).toEqual([
|
||||
['remote', 'add', 'pr-contributor-orca', FORK_SSH]
|
||||
])
|
||||
expect(callsMatching(exec, ['fetch'])).toEqual([
|
||||
[
|
||||
'fetch',
|
||||
'pr-contributor-orca',
|
||||
'+refs/heads/contributor/fix:refs/remotes/pr-contributor-orca/contributor/fix'
|
||||
]
|
||||
])
|
||||
expect(result).toEqual({
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/fix',
|
||||
remoteUrl: FORK_SSH,
|
||||
remoteCreated: true
|
||||
})
|
||||
})
|
||||
|
||||
it('reuses an existing remote pointing at the same fork (SSH vs HTTPS) without adding', async () => {
|
||||
const exec = makeRepoExec({
|
||||
origin: 'git@github.com:stablyai/orca.git',
|
||||
'pr-contributor-orca': FORK_HTTPS
|
||||
})
|
||||
|
||||
const result = await prepareWorktreePushTargetWithExec(exec, REPO, forkTarget(), () => false)
|
||||
|
||||
expect(callsMatching(exec, ['remote', 'add'])).toEqual([])
|
||||
expect(callsMatching(exec, ['fetch'])).toEqual([
|
||||
[
|
||||
'fetch',
|
||||
'pr-contributor-orca',
|
||||
'+refs/heads/contributor/fix:refs/remotes/pr-contributor-orca/contributor/fix'
|
||||
]
|
||||
])
|
||||
// remoteCreated omitted because the predicate says no known worktree owns it.
|
||||
expect(result).toEqual({
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/fix',
|
||||
remoteUrl: FORK_SSH
|
||||
})
|
||||
})
|
||||
|
||||
it('inherits remoteCreated when the predicate says a known worktree created the reused remote', async () => {
|
||||
const exec = makeRepoExec({ 'fork-x': FORK_HTTPS })
|
||||
|
||||
const result = await prepareWorktreePushTargetWithExec(exec, REPO, forkTarget(), () => true)
|
||||
|
||||
expect(result.remoteName).toBe('fork-x')
|
||||
expect(result.remoteCreated).toBe(true)
|
||||
})
|
||||
|
||||
it('disambiguates with a numeric suffix when the preferred remote name is taken by a different URL', async () => {
|
||||
const exec = makeRepoExec({ 'pr-contributor-orca': 'git@github.com:someone-else/orca.git' })
|
||||
|
||||
const result = await prepareWorktreePushTargetWithExec(exec, REPO, forkTarget(), () => false)
|
||||
|
||||
expect(callsMatching(exec, ['remote', 'add'])).toEqual([
|
||||
['remote', 'add', 'pr-contributor-orca-2', FORK_SSH]
|
||||
])
|
||||
expect(result.remoteName).toBe('pr-contributor-orca-2')
|
||||
expect(result.remoteCreated).toBe(true)
|
||||
})
|
||||
|
||||
it('strips an incoming remoteCreated flag and fetches the given remote when there is no remoteUrl', async () => {
|
||||
const exec = makeRepoExec({ origin: 'git@github.com:stablyai/orca.git' })
|
||||
|
||||
const result = await prepareWorktreePushTargetWithExec(
|
||||
exec,
|
||||
REPO,
|
||||
{ remoteName: 'origin', branchName: 'feature', remoteCreated: true },
|
||||
() => false
|
||||
)
|
||||
|
||||
expect(callsMatching(exec, ['remote', 'add'])).toEqual([])
|
||||
expect(callsMatching(exec, ['fetch'])).toEqual([
|
||||
['fetch', 'origin', '+refs/heads/feature:refs/remotes/origin/feature']
|
||||
])
|
||||
expect(result).toEqual({ remoteName: 'origin', branchName: 'feature' })
|
||||
})
|
||||
})
|
||||
|
||||
describe('findRemoteForUrl', () => {
|
||||
it('matches by GitHub owner/repo across URL protocols', async () => {
|
||||
const exec = makeRepoExec({
|
||||
origin: 'git@github.com:stablyai/orca.git',
|
||||
fork: FORK_SSH
|
||||
})
|
||||
await expect(findRemoteForUrl(exec, REPO, FORK_HTTPS)).resolves.toBe('fork')
|
||||
})
|
||||
|
||||
it('returns null when no remote points at the fork', async () => {
|
||||
const exec = makeRepoExec({ origin: 'git@github.com:stablyai/orca.git' })
|
||||
await expect(findRemoteForUrl(exec, REPO, FORK_SSH)).resolves.toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('ensureUniqueRemoteName', () => {
|
||||
it('returns the preferred name when it is free', async () => {
|
||||
const exec = makeRepoExec({ origin: 'x' })
|
||||
await expect(ensureUniqueRemoteName(exec, REPO, 'fork')).resolves.toBe('fork')
|
||||
})
|
||||
|
||||
it('suffixes past taken names', async () => {
|
||||
const exec = makeRepoExec({ fork: 'x', 'fork-2': 'y' })
|
||||
await expect(ensureUniqueRemoteName(exec, REPO, 'fork')).resolves.toBe('fork-3')
|
||||
})
|
||||
})
|
||||
|
||||
describe('configureCreatedWorktreePushTargetWithExec', () => {
|
||||
it('points the new branch upstream at the fork remote', async () => {
|
||||
const exec = makeRepoExec({})
|
||||
const target = forkTarget()
|
||||
|
||||
const result = await configureCreatedWorktreePushTargetWithExec(
|
||||
exec,
|
||||
'/wt/path',
|
||||
'local-branch',
|
||||
target
|
||||
)
|
||||
|
||||
expect(exec).toHaveBeenCalledWith(
|
||||
['branch', '--set-upstream-to', 'pr-contributor-orca/contributor/fix', 'local-branch'],
|
||||
'/wt/path'
|
||||
)
|
||||
expect(result).toBe(target)
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,124 @@
|
|||
// Why: preparing a fork-PR push target means adding (or reusing) the contributor's
|
||||
// fork as a git remote, fetching the head, and wiring the new branch's upstream.
|
||||
// The git-driven core lives here behind an injectable `execGit` seam so the
|
||||
// remote-reuse / unique-naming / fetch behavior is unit-testable without a real
|
||||
// repo. The store-aware ownership decision stays with the caller via a predicate.
|
||||
|
||||
import type { GitPushTarget } from '../../shared/types'
|
||||
import { parseGitHubOwnerRepo } from '../github/gh-utils'
|
||||
import type { GitRemoteExec } from './worktree-push-target-cleanup'
|
||||
|
||||
export async function findRemoteForUrl(
|
||||
execGit: GitRemoteExec,
|
||||
repoPath: string,
|
||||
remoteUrl: string
|
||||
): Promise<string | null> {
|
||||
const target = parseGitHubOwnerRepo(remoteUrl)
|
||||
try {
|
||||
const { stdout } = await execGit(['remote'], repoPath)
|
||||
for (const remote of stdout
|
||||
.split(/\r?\n/)
|
||||
.map((line) => line.trim())
|
||||
.filter(Boolean)) {
|
||||
try {
|
||||
const { stdout: urlStdout } = await execGit(['remote', 'get-url', remote], repoPath)
|
||||
const candidateUrl = urlStdout.trim()
|
||||
const candidate = parseGitHubOwnerRepo(candidateUrl)
|
||||
if (
|
||||
target &&
|
||||
candidate &&
|
||||
target.owner.toLowerCase() === candidate.owner.toLowerCase() &&
|
||||
target.repo.toLowerCase() === candidate.repo.toLowerCase()
|
||||
) {
|
||||
return remote
|
||||
}
|
||||
if (candidateUrl === remoteUrl) {
|
||||
return remote
|
||||
}
|
||||
} catch {
|
||||
// Ignore a remote that disappeared or has no fetch URL.
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
export async function ensureUniqueRemoteName(
|
||||
execGit: GitRemoteExec,
|
||||
repoPath: string,
|
||||
preferred: string
|
||||
): Promise<string> {
|
||||
const { stdout } = await execGit(['remote'], repoPath)
|
||||
const existing = new Set(
|
||||
stdout
|
||||
.split(/\r?\n/)
|
||||
.map((line) => line.trim())
|
||||
.filter(Boolean)
|
||||
)
|
||||
if (!existing.has(preferred)) {
|
||||
return preferred
|
||||
}
|
||||
for (let suffix = 2; suffix < 100; suffix += 1) {
|
||||
const candidate = `${preferred}-${suffix}`
|
||||
if (!existing.has(candidate)) {
|
||||
return candidate
|
||||
}
|
||||
}
|
||||
throw new Error(`Could not find an available remote name for ${preferred}.`)
|
||||
}
|
||||
|
||||
// Exported for unit tests: the `execGit` seam drives the remote add/reuse/fetch
|
||||
// behavior without a real repo. `isRemoteCreatedByKnownWorktree` lets the caller
|
||||
// inject the store-aware ownership decision for the reuse case.
|
||||
export async function prepareWorktreePushTargetWithExec(
|
||||
execGit: GitRemoteExec,
|
||||
repoPath: string,
|
||||
target: GitPushTarget,
|
||||
isRemoteCreatedByKnownWorktree: (existingRemote: string) => boolean
|
||||
): Promise<GitPushTarget> {
|
||||
const { remoteCreated: _ignoredRemoteCreated, ...sanitizedTarget } = target
|
||||
let remoteName = target.remoteName
|
||||
let remoteCreated = false
|
||||
if (target.remoteUrl) {
|
||||
const existingRemote = await findRemoteForUrl(execGit, 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 = isRemoteCreatedByKnownWorktree(existingRemote)
|
||||
} else {
|
||||
remoteName = await ensureUniqueRemoteName(execGit, repoPath, target.remoteName)
|
||||
await execGit(['remote', 'add', remoteName, target.remoteUrl], repoPath)
|
||||
remoteCreated = true
|
||||
}
|
||||
}
|
||||
|
||||
await execGit(
|
||||
[
|
||||
'fetch',
|
||||
remoteName,
|
||||
`+refs/heads/${target.branchName}:refs/remotes/${remoteName}/${target.branchName}`
|
||||
],
|
||||
repoPath
|
||||
)
|
||||
return {
|
||||
...sanitizedTarget,
|
||||
remoteName,
|
||||
...(remoteCreated ? { remoteCreated: true } : {})
|
||||
}
|
||||
}
|
||||
|
||||
export async function configureCreatedWorktreePushTargetWithExec(
|
||||
execGit: GitRemoteExec,
|
||||
worktreePath: string,
|
||||
branchName: string,
|
||||
target: GitPushTarget
|
||||
): Promise<GitPushTarget> {
|
||||
await execGit(
|
||||
['branch', '--set-upstream-to', `${target.remoteName}/${target.branchName}`, branchName],
|
||||
worktreePath
|
||||
)
|
||||
return target
|
||||
}
|
||||
|
|
@ -67,6 +67,15 @@ import {
|
|||
areWorktreePathsEqual
|
||||
} from './worktree-logic'
|
||||
import { getRepoIdFromWorktreeId } from '../../shared/worktree-id'
|
||||
import {
|
||||
cleanupUnusedWorktreePushTargetRemoteWithExec,
|
||||
sameGitHubRemoteUrl,
|
||||
type WorktreePushTargetStore
|
||||
} from './worktree-push-target-cleanup'
|
||||
import {
|
||||
configureCreatedWorktreePushTargetWithExec,
|
||||
prepareWorktreePushTargetWithExec
|
||||
} from './worktree-push-target-setup'
|
||||
import { invalidateAuthorizedRootsCache, isENOENT } from './filesystem-auth'
|
||||
import { createWorktreeSymlinks } from './worktree-symlinks'
|
||||
import { normalizeSparseDirectories } from './sparse-checkout-directories'
|
||||
|
|
@ -350,41 +359,6 @@ async function unsetRemoteWorktreeCreationBase(
|
|||
}
|
||||
}
|
||||
|
||||
async function findRemoteForUrl(repoPath: string, remoteUrl: string): Promise<string | null> {
|
||||
const target = parseGitHubOwnerRepo(remoteUrl)
|
||||
try {
|
||||
const { stdout } = await gitExecFileAsync(['remote'], { cwd: repoPath })
|
||||
for (const remote of stdout
|
||||
.split(/\r?\n/)
|
||||
.map((line) => line.trim())
|
||||
.filter(Boolean)) {
|
||||
try {
|
||||
const { stdout: urlStdout } = await gitExecFileAsync(['remote', 'get-url', remote], {
|
||||
cwd: repoPath
|
||||
})
|
||||
const candidateUrl = urlStdout.trim()
|
||||
const candidate = parseGitHubOwnerRepo(candidateUrl)
|
||||
if (
|
||||
target &&
|
||||
candidate &&
|
||||
target.owner.toLowerCase() === candidate.owner.toLowerCase() &&
|
||||
target.repo.toLowerCase() === candidate.repo.toLowerCase()
|
||||
) {
|
||||
return remote
|
||||
}
|
||||
if (candidateUrl === remoteUrl) {
|
||||
return remote
|
||||
}
|
||||
} catch {
|
||||
// Ignore a remote that disappeared or has no fetch URL.
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
async function resolveCreateBranchName(
|
||||
repoPath: string,
|
||||
branchNameOverride: string | undefined,
|
||||
|
|
@ -616,26 +590,6 @@ async function remotePathExists(
|
|||
}
|
||||
}
|
||||
|
||||
async function ensureUniqueRemoteName(repoPath: string, preferred: string): Promise<string> {
|
||||
const { stdout } = await gitExecFileAsync(['remote'], { cwd: repoPath })
|
||||
const existing = new Set(
|
||||
stdout
|
||||
.split(/\r?\n/)
|
||||
.map((line) => line.trim())
|
||||
.filter(Boolean)
|
||||
)
|
||||
if (!existing.has(preferred)) {
|
||||
return preferred
|
||||
}
|
||||
for (let suffix = 2; suffix < 100; suffix += 1) {
|
||||
const candidate = `${preferred}-${suffix}`
|
||||
if (!existing.has(candidate)) {
|
||||
return candidate
|
||||
}
|
||||
}
|
||||
throw new Error(`Could not find an available remote name for ${preferred}.`)
|
||||
}
|
||||
|
||||
export async function prepareWorktreePushTarget(
|
||||
repoPath: string,
|
||||
target: GitPushTarget,
|
||||
|
|
@ -643,86 +597,19 @@ export async function prepareWorktreePushTarget(
|
|||
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
|
||||
return prepareWorktreePushTargetWithExec(
|
||||
(args, cwd) => gitExecFileAsync(args, { cwd }),
|
||||
repoPath,
|
||||
target,
|
||||
(existingRemote) =>
|
||||
store
|
||||
? isPushTargetRemoteCreatedByKnownWorktree(
|
||||
store,
|
||||
{
|
||||
...target,
|
||||
remoteName: existingRemote
|
||||
},
|
||||
{ ...target, remoteName: existingRemote },
|
||||
repoId
|
||||
)
|
||||
: false
|
||||
} else {
|
||||
remoteName = await ensureUniqueRemoteName(repoPath, target.remoteName)
|
||||
await gitExecFileAsync(['remote', 'add', remoteName, target.remoteUrl], { cwd: repoPath })
|
||||
remoteCreated = true
|
||||
}
|
||||
}
|
||||
|
||||
await gitExecFileAsync(
|
||||
[
|
||||
'fetch',
|
||||
remoteName,
|
||||
`+refs/heads/${target.branchName}:refs/remotes/${remoteName}/${target.branchName}`
|
||||
],
|
||||
{ cwd: repoPath }
|
||||
)
|
||||
return {
|
||||
...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(
|
||||
|
|
@ -748,66 +635,6 @@ function isPushTargetRemoteCreatedByKnownWorktree(
|
|||
})
|
||||
}
|
||||
|
||||
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,
|
||||
|
|
@ -832,11 +659,12 @@ export async function configureCreatedWorktreePushTarget(
|
|||
branchName: string,
|
||||
target: GitPushTarget
|
||||
): Promise<GitPushTarget> {
|
||||
await gitExecFileAsync(
|
||||
['branch', '--set-upstream-to', `${target.remoteName}/${target.branchName}`, branchName],
|
||||
{ cwd: worktreePath }
|
||||
return configureCreatedWorktreePushTargetWithExec(
|
||||
(args, cwd) => gitExecFileAsync(args, { cwd }),
|
||||
worktreePath,
|
||||
branchName,
|
||||
target
|
||||
)
|
||||
return target
|
||||
}
|
||||
|
||||
async function findRemoteForUrlSsh(
|
||||
|
|
|
|||
|
|
@ -1335,9 +1335,11 @@ describe('registerWorktreeHandlers', () => {
|
|||
|
||||
it('returns the PR head push target when resolving a fork PR base', async () => {
|
||||
getPullRequestPushTargetMock.mockResolvedValue({
|
||||
remoteName: 'pr-prateek-orca',
|
||||
branchName: 'prateek/fix-sidebar-agents-toggle',
|
||||
remoteUrl: 'git@github.com:prateek/orca.git'
|
||||
pushTarget: {
|
||||
remoteName: 'pr-prateek-orca',
|
||||
branchName: 'prateek/fix-sidebar-agents-toggle',
|
||||
remoteUrl: 'git@github.com:prateek/orca.git'
|
||||
}
|
||||
})
|
||||
gitExecFileAsyncMock.mockImplementation(async (args: string[]) => {
|
||||
if (args[0] === 'rev-parse') {
|
||||
|
|
@ -1358,6 +1360,8 @@ describe('registerWorktreeHandlers', () => {
|
|||
})
|
||||
expect(result).toMatchObject({
|
||||
baseBranch: 'abc123',
|
||||
headSha: 'abc123',
|
||||
branchNameOverride: 'prateek/fix-sidebar-agents-toggle',
|
||||
pushTarget: {
|
||||
remoteName: 'pr-prateek-orca',
|
||||
branchName: 'prateek/fix-sidebar-agents-toggle',
|
||||
|
|
@ -1420,7 +1424,11 @@ describe('registerWorktreeHandlers', () => {
|
|||
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['fetch', 'origin', 'refs/pull/1849/head'], {
|
||||
cwd: '/workspace/repo'
|
||||
})
|
||||
expect(result).toMatchObject({ baseBranch: 'abc123' })
|
||||
expect(result).toEqual({
|
||||
baseBranch: 'abc123',
|
||||
headSha: 'abc123',
|
||||
branchNameOverride: 'feat/onboarding-model-choice-782'
|
||||
})
|
||||
})
|
||||
|
||||
it('falls back to refs/pull/<N>/head when branch fetch fails for a PR', async () => {
|
||||
|
|
@ -1457,7 +1465,11 @@ describe('registerWorktreeHandlers', () => {
|
|||
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['fetch', 'origin', 'refs/pull/1849/head'], {
|
||||
cwd: '/workspace/repo'
|
||||
})
|
||||
expect(result).toMatchObject({ baseBranch: 'abc123' })
|
||||
expect(result).toEqual({
|
||||
baseBranch: 'abc123',
|
||||
headSha: 'abc123',
|
||||
branchNameOverride: 'feat/onboarding-model-choice-782'
|
||||
})
|
||||
})
|
||||
|
||||
it('does not fall back to refs/pull/<N>/head when branch fetch hits a network failure', async () => {
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ composer card markup together so the inline and modal variants share one UI
|
|||
surface without splitting the controlled form into hard-to-follow fragments. */
|
||||
import React from 'react'
|
||||
import {
|
||||
AlertTriangle,
|
||||
Check,
|
||||
ChevronDown,
|
||||
CornerDownLeft,
|
||||
|
|
@ -62,6 +63,8 @@ type NewWorkspaceComposerCardProps = {
|
|||
onSmartLinearIssueSelect: (issue: LinearIssue) => void
|
||||
smartNameSelection: SmartWorkspaceNameSelection | null
|
||||
onClearSmartNameSelection: () => void
|
||||
/** Advisory shown under the name field when a fork PR can't accept maintainer pushes. */
|
||||
forkPushWarning: string | null
|
||||
detectedAgentIds: Set<TuiAgent> | null
|
||||
onOpenAgentSettings: () => void
|
||||
advancedOpen: boolean
|
||||
|
|
@ -228,6 +231,7 @@ export default function NewWorkspaceComposerCard({
|
|||
onSmartLinearIssueSelect,
|
||||
smartNameSelection,
|
||||
onClearSmartNameSelection,
|
||||
forkPushWarning,
|
||||
detectedAgentIds,
|
||||
onOpenAgentSettings,
|
||||
advancedOpen,
|
||||
|
|
@ -495,6 +499,12 @@ export default function NewWorkspaceComposerCard({
|
|||
agentTrigger?.focus()
|
||||
}}
|
||||
/>
|
||||
{forkPushWarning ? (
|
||||
<p className="flex items-start gap-1.5 text-[11px] text-yellow-600 dark:text-yellow-500">
|
||||
<AlertTriangle className="mt-0.5 size-3 shrink-0" aria-hidden="true" />
|
||||
<span>{forkPushWarning}</span>
|
||||
</p>
|
||||
) : null}
|
||||
</div>
|
||||
|
||||
<div className="space-y-1" data-contextual-tour-target="workspace-creation-agent">
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ import {
|
|||
Copy,
|
||||
Folder,
|
||||
FolderOpen,
|
||||
GitFork,
|
||||
GitMerge,
|
||||
GitPullRequestArrow,
|
||||
MessageSquare,
|
||||
|
|
@ -86,6 +87,7 @@ import {
|
|||
type PendingDiscardConfirmation
|
||||
} from './source-control-discard-dialog'
|
||||
import { refreshGitStatusForWorktree } from './git-status-refresh'
|
||||
import { describeForkPushTarget } from './fork-push-target-label'
|
||||
import { toast } from 'sonner'
|
||||
import {
|
||||
ContextMenu,
|
||||
|
|
@ -4393,6 +4395,18 @@ function SourceControlInner(): React.JSX.Element {
|
|||
clears. Active merge/rebase/cherry-pick operations are the
|
||||
exception: commits would be misleading before the user continues
|
||||
or aborts the operation. */}
|
||||
{activeWorktree?.pushTarget && activeWorktree.pushTarget.remoteName !== 'origin' ? (
|
||||
<div
|
||||
className="flex items-center gap-1.5 px-1 text-[11px] text-muted-foreground"
|
||||
title={`Pushes to the fork at ${activeWorktree.pushTarget.remoteName} (not origin)`}
|
||||
>
|
||||
<GitFork className="size-3 shrink-0" aria-hidden="true" />
|
||||
<span className="truncate">
|
||||
Pushes to fork {describeForkPushTarget(activeWorktree.pushTarget)}
|
||||
</span>
|
||||
</div>
|
||||
) : null}
|
||||
|
||||
{shouldRenderCommitArea(scope, unresolvedConflicts.length, conflictOperation) &&
|
||||
(primaryAction.kind === 'create_pr' ? (
|
||||
<PullRequestComposer
|
||||
|
|
|
|||
|
|
@ -0,0 +1,43 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
import type { GitPushTarget } from '../../../../shared/types'
|
||||
import { describeForkPushTarget } from './fork-push-target-label'
|
||||
|
||||
function target(overrides: Partial<GitPushTarget>): GitPushTarget {
|
||||
return {
|
||||
remoteName: 'pr-contributor-orca',
|
||||
branchName: 'contributor/fix',
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
describe('describeForkPushTarget', () => {
|
||||
it('derives owner:branch from an SSH fork URL', () => {
|
||||
expect(
|
||||
describeForkPushTarget(target({ remoteUrl: 'git@github.com:contributor/orca.git' }))
|
||||
).toBe('contributor:contributor/fix')
|
||||
})
|
||||
|
||||
it('derives owner:branch from an HTTPS fork URL', () => {
|
||||
expect(
|
||||
describeForkPushTarget(target({ remoteUrl: 'https://github.com/contributor/orca.git' }))
|
||||
).toBe('contributor:contributor/fix')
|
||||
})
|
||||
|
||||
it('handles a URL without a .git suffix', () => {
|
||||
expect(
|
||||
describeForkPushTarget(target({ remoteUrl: 'https://github.com/contributor/orca' }))
|
||||
).toBe('contributor:contributor/fix')
|
||||
})
|
||||
|
||||
it('falls back to remoteName/branch when there is no remote URL', () => {
|
||||
expect(describeForkPushTarget(target({ remoteUrl: undefined }))).toBe(
|
||||
'pr-contributor-orca/contributor/fix'
|
||||
)
|
||||
})
|
||||
|
||||
it('works for non-GitHub hosts via the generic owner segment', () => {
|
||||
expect(
|
||||
describeForkPushTarget(target({ remoteUrl: 'git@gitlab.com:contributor/orca.git' }))
|
||||
).toBe('contributor:contributor/fix')
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,12 @@
|
|||
import type { GitPushTarget } from '../../../../shared/types'
|
||||
|
||||
// Why: a fork-PR worktree pushes to a contributor's fork, not origin. Render
|
||||
// "owner:branch" from the fork remote URL when available so the maintainer can
|
||||
// see at a glance where a push lands; fall back to the sanitized remote name.
|
||||
export function describeForkPushTarget(pushTarget: GitPushTarget): string {
|
||||
const ownerMatch = pushTarget.remoteUrl?.match(/[:/]([^/:]+)\/[^/]+?(?:\.git)?$/)
|
||||
const owner = ownerMatch?.[1]
|
||||
return owner
|
||||
? `${owner}:${pushTarget.branchName}`
|
||||
: `${pushTarget.remoteName}/${pushTarget.branchName}`
|
||||
}
|
||||
|
|
@ -0,0 +1,29 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
import { FORK_PUSH_NO_MAINTAINER_EDIT_WARNING, getForkPushWarning } from './fork-push-warning'
|
||||
|
||||
const forkTarget = { remoteName: 'pr-contributor-orca', branchName: 'contributor/fix' }
|
||||
const originTarget = { remoteName: 'origin', branchName: 'feature/fix' }
|
||||
|
||||
describe('getForkPushWarning', () => {
|
||||
it('warns for a fork PR whose author disabled maintainer edits', () => {
|
||||
expect(getForkPushWarning({ pushTarget: forkTarget, maintainerCanModify: false })).toBe(
|
||||
FORK_PUSH_NO_MAINTAINER_EDIT_WARNING
|
||||
)
|
||||
})
|
||||
|
||||
it('does not warn when maintainer edits are allowed', () => {
|
||||
expect(getForkPushWarning({ pushTarget: forkTarget, maintainerCanModify: true })).toBeNull()
|
||||
})
|
||||
|
||||
it('does not warn when the maintainer flag is unknown', () => {
|
||||
expect(getForkPushWarning({ pushTarget: forkTarget })).toBeNull()
|
||||
})
|
||||
|
||||
it('does not warn for a same-repo PR even if the flag is false (we own origin)', () => {
|
||||
expect(getForkPushWarning({ pushTarget: originTarget, maintainerCanModify: false })).toBeNull()
|
||||
})
|
||||
|
||||
it('does not warn when there is no resolved push target', () => {
|
||||
expect(getForkPushWarning({ maintainerCanModify: false })).toBeNull()
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,21 @@
|
|||
import type { GitHubPrStartPoint } from '../../../shared/types'
|
||||
|
||||
export const FORK_PUSH_NO_MAINTAINER_EDIT_WARNING =
|
||||
'This PR has "Allow edits from maintainers" off; pushing to the fork may be rejected by GitHub.'
|
||||
|
||||
// Why: only warn for fork PRs where the push target points away from origin and
|
||||
// whose author left "Allow edits from maintainers" off. That's the one case
|
||||
// where our push to the contributor's fork can be rejected by GitHub. Returns
|
||||
// the warning text to show, or null when no warning applies.
|
||||
export function getForkPushWarning(
|
||||
result: Pick<GitHubPrStartPoint, 'pushTarget' | 'maintainerCanModify'>
|
||||
): string | null {
|
||||
if (
|
||||
result.maintainerCanModify === false &&
|
||||
result.pushTarget !== undefined &&
|
||||
result.pushTarget.remoteName !== 'origin'
|
||||
) {
|
||||
return FORK_PUSH_NO_MAINTAINER_EDIT_WARNING
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
|
@ -81,6 +81,7 @@ import { getComposerEligibleRepos, resolveComposerRepoId } from '@/lib/new-works
|
|||
import { queueNewWorkspaceTerminalFocus } from '@/lib/new-workspace-terminal-focus'
|
||||
import { getSuggestedCreatureName } from '@/components/sidebar/worktree-name-suggestions'
|
||||
import type { SmartWorkspaceNameSelection } from '@/components/new-workspace/SmartWorkspaceNameField'
|
||||
import { getForkPushWarning } from './fork-push-warning'
|
||||
import { CONTEXTUAL_TOUR_ENABLE_AUTO_WORKSPACE_NAME_EVENT } from '@/components/contextual-tours/contextual-tour-composer-events'
|
||||
import { ensureHooksConfirmed } from '@/lib/ensure-hooks-confirmed'
|
||||
import { normalizeSparseDirectoryLines, sparseDirectoriesMatch } from '@/lib/sparse-paths'
|
||||
|
|
@ -211,6 +212,9 @@ export type ComposerCardProps = {
|
|||
/** Transient inline hint shown next to the Start-from trigger after a repo
|
||||
* switch resets a prior selection (e.g. "was PR #8778"). Null when none. */
|
||||
startFromResetHint: string | null
|
||||
/** Warning shown when a selected fork PR has "Allow edits from maintainers"
|
||||
* off, so a push to the fork may be rejected. Null when none. */
|
||||
forkPushWarning: string | null
|
||||
setupConfig: SetupConfig | null
|
||||
requiresExplicitSetupChoice: boolean
|
||||
setupDecision: 'run' | 'skip' | null
|
||||
|
|
@ -415,6 +419,9 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS
|
|||
// reset inline (e.g. "was PR #8778") so the change is recoverable visually
|
||||
// instead of slipping past the user. Cleared on any subsequent selection.
|
||||
const [startFromResetHint, setStartFromResetHint] = useState<string | null>(null)
|
||||
// Why: a fork PR with "Allow edits from maintainers" off can't be pushed to;
|
||||
// warn (but don't block) so the maintainer isn't surprised by a rejected push.
|
||||
const [forkPushWarning, setForkPushWarning] = useState<string | null>(null)
|
||||
const disabledTuiAgentKey = (settings?.disabledTuiAgents ?? []).join('\u0000')
|
||||
const disabledTuiAgents = useMemo<TuiAgent[]>(
|
||||
() => settings?.disabledTuiAgents ?? [],
|
||||
|
|
@ -1276,6 +1283,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS
|
|||
setLinkedWorkItem(null)
|
||||
setLinkedIssue('')
|
||||
setLinkedPR(null)
|
||||
setForkPushWarning(null)
|
||||
if (name === lastAutoNameRef.current) {
|
||||
lastAutoNameRef.current = ''
|
||||
}
|
||||
|
|
@ -1560,6 +1568,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS
|
|||
setBaseBranch(undefined)
|
||||
setPushTarget(undefined)
|
||||
setBranchNameOverride(undefined)
|
||||
setForkPushWarning(null)
|
||||
setStartFromResetHint(hint)
|
||||
},
|
||||
[baseBranch, linkedWorkItem, repoId, setRepoId]
|
||||
|
|
@ -1581,6 +1590,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS
|
|||
setBaseBranch(next)
|
||||
setPushTarget(undefined)
|
||||
setBranchNameOverride(undefined)
|
||||
setForkPushWarning(null)
|
||||
branchAutoNameRef.current = ''
|
||||
setStartFromResetHint(null)
|
||||
}, [])
|
||||
|
|
@ -1645,6 +1655,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS
|
|||
(item: GitHubWorkItem): void => {
|
||||
setStartFromResetHint(null)
|
||||
setBranchNameOverride(undefined)
|
||||
setForkPushWarning(null)
|
||||
branchAutoNameRef.current = ''
|
||||
const repoForItem = eligibleRepos.find((repo) => repo.id === item.repoId) ?? selectedRepo
|
||||
applyLinkedWorkItem(item)
|
||||
|
|
@ -1691,6 +1702,9 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS
|
|||
result.pushTarget,
|
||||
result.branchNameOverride
|
||||
)
|
||||
// Why: a fork PR push lands on the contributor's fork; if they didn't
|
||||
// allow maintainer edits, GitHub will reject it. Warn up front.
|
||||
setForkPushWarning(getForkPushWarning(result))
|
||||
})
|
||||
.catch((error: unknown) => {
|
||||
setBaseBranch(undefined)
|
||||
|
|
@ -1711,6 +1725,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS
|
|||
applyLinkedGitLabWorkItem(item)
|
||||
setStartFromResetHint(null)
|
||||
setBranchNameOverride(undefined)
|
||||
setForkPushWarning(null)
|
||||
branchAutoNameRef.current = ''
|
||||
const repoForItem = eligibleRepos.find((repo) => repo.id === item.repoId) ?? selectedRepo
|
||||
if (item.type !== 'mr' || !repoForItem) {
|
||||
|
|
@ -1746,6 +1761,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS
|
|||
setBaseBranch(selection.baseBranch)
|
||||
setPushTarget(undefined)
|
||||
setStartFromResetHint(null)
|
||||
setForkPushWarning(null)
|
||||
setBranchNameOverridePreservesNameEdits(false)
|
||||
if (selection.name !== undefined && selection.lastAutoName !== undefined) {
|
||||
setName(selection.name)
|
||||
|
|
@ -1771,6 +1787,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS
|
|||
lastAutoNameRef.current = suggestedName
|
||||
}
|
||||
setBranchNameOverride(undefined)
|
||||
setForkPushWarning(null)
|
||||
branchAutoNameRef.current = ''
|
||||
// Why: match the GitHub issue/PR flow by drafting linked context for
|
||||
// review instead of auto-submitting. Auto-filling the note here would
|
||||
|
|
@ -1786,6 +1803,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS
|
|||
setBaseBranch(undefined)
|
||||
setPushTarget(undefined)
|
||||
setBranchNameOverride(undefined)
|
||||
setForkPushWarning(null)
|
||||
branchAutoNameRef.current = ''
|
||||
setStartFromResetHint(null)
|
||||
if (name === lastAutoNameRef.current) {
|
||||
|
|
@ -2483,6 +2501,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS
|
|||
selectedRepoConnectInProgress,
|
||||
onConnectSelectedRepo,
|
||||
startFromResetHint,
|
||||
forkPushWarning,
|
||||
note,
|
||||
onNoteChange: setNote,
|
||||
setupConfig,
|
||||
|
|
|
|||
|
|
@ -299,6 +299,8 @@ export type GitHubPrStartPoint = {
|
|||
headSha?: string
|
||||
/** Exact local branch name to create/reuse when the PR head is a safe same-repo branch. */
|
||||
branchNameOverride?: string
|
||||
/** Fork PRs: false when "Allow edits from maintainers" is off; a push to the fork may be rejected. */
|
||||
maintainerCanModify?: boolean
|
||||
}
|
||||
|
||||
// ─── Worktree metadata (persisted user-authored fields only) ─────────
|
||||
|
|
|
|||
Loading…
Reference in New Issue