Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions src/main/git/fork-remote-refspec.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { describe, expect, it, vi, type Mock } from 'vitest'
import {
buildNarrowForkFetchRefspec,
ensureRemoteAlsoTracksBranch,
ensureRemoteTracksBranchNarrowly,
getRemoteFetchRefspecs,
pruneUntrackedForkRemoteRefs,
Expand Down Expand Up @@ -134,6 +135,42 @@ describe('ensureRemoteTracksBranchNarrowly', () => {
})
})

describe('ensureRemoteAlsoTracksBranch', () => {
it('appends the narrow refspec without dropping the user-configured wide default (#25703)', async () => {
const { exec } = makeConfigExec({ fork: [wildcardForkFetchRefspec('fork')] })

await ensureRemoteAlsoTracksBranch(exec, REPO, 'fork', 'main')

await expect(getRemoteFetchRefspecs(exec, REPO, 'fork')).resolves.toEqual([
wildcardForkFetchRefspec('fork'),
'+refs/heads/main*:refs/remotes/fork/main*'
])
})

it('is a no-op when the branch is already tracked', async () => {
const { exec } = makeConfigExec({ fork: ['+refs/heads/main*:refs/remotes/fork/main*'] })

await ensureRemoteAlsoTracksBranch(exec, REPO, 'fork', 'main')

const addCalls = exec.mock.calls.filter(([args]) => args[1] === '--add')
expect(addCalls).toEqual([])
})

it("keeps other branches' entries and never writes tagOpt", async () => {
const { exec, tagOptByRemote } = makeConfigExec({
fork: ['+refs/heads/other*:refs/remotes/fork/other*']
})

await ensureRemoteAlsoTracksBranch(exec, REPO, 'fork', 'main')

await expect(getRemoteFetchRefspecs(exec, REPO, 'fork')).resolves.toEqual([
'+refs/heads/other*:refs/remotes/fork/other*',
'+refs/heads/main*:refs/remotes/fork/main*'
])
expect(tagOptByRemote.fork).toBeUndefined()
})
})

describe('removeStaleForkFetchRefspec', () => {
it('drops only the refspec whose source matches the stale branch', async () => {
const { exec } = makeConfigExec({
Expand Down
19 changes: 19 additions & 0 deletions src/main/git/fork-remote-refspec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,25 @@ export async function ensureRemoteTracksBranchNarrowly(
await execGit(['config', `remote.${remoteName}.tagOpt`, '--no-tags'], repoPath)
}

/**
* Reuse-path twin of `ensureRemoteTracksBranchNarrowly` for remotes Orca did not create
* (#25703): appends the branch's narrow refspec when absent and never drops or rewrites
* anything already configured -- the user's wide default and other branches' entries stay
* intact, and `tagOpt` is left alone.
*/
export async function ensureRemoteAlsoTracksBranch(
execGit: GitExecFn,
repoPath: string,
remoteName: string,
branchName: string
): Promise<void> {
const desired = buildNarrowForkFetchRefspec(remoteName, branchName)
const existing = await getRemoteFetchRefspecs(execGit, repoPath, remoteName)
if (!existing.includes(desired)) {
await execGit(['config', '--add', `remote.${remoteName}.fetch`, desired], repoPath)
}
}

/**
* Removes every `remote.<name>.fetch` entry, leaving the remote pushable but importing
* nothing on a plain fetch. For a fork remote with no branch pinning it at all (no
Expand Down
5 changes: 3 additions & 2 deletions src/main/ipc/worktree-push-target-cleanup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,9 @@ function isBranchConfigSeparator(code: number): boolean {
// Why: on-demand materialization (push/pull/fetch/fast-forward, #17828) never
// updates the store's `pushTarget.remoteCreated` flag, so ownership must also be
// readable from the repo-local `remote.<name>.orca-created` config Orca writes
// when it creates the remote (see `worktree-push-target-setup.ts`).
async function remoteHasOrcaProvenance(
// when it creates the remote (see `worktree-push-target-setup.ts`). Also gates
// which reused remotes the refspec narrowing may rewrite (#25703).
export async function remoteHasOrcaProvenance(
execGit: GitRemoteExec,
repoPath: string,
remoteName: string
Expand Down
130 changes: 130 additions & 0 deletions src/main/ipc/worktree-push-target-setup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,63 @@ function callsMatching(exec: ExecMock, head: string[]): string[][] {
.filter((args) => head.every((part, i) => args[i] === part))
}

// `makeRepoExec` plus in-memory `remote.<name>.fetch` / `.orca-created` / `.tagOpt` config,
// mutated the way real `git config` would be (#25703's reuse-with-refspec matrix).
function makeConfigAwareRepoExec(
remotes: Record<string, string>,
fetchRefspecsByRemote: Record<string, string[]> = {},
orcaCreatedRemotes: string[] = []
): {
exec: ExecMock
fetchRefspecs: Record<string, string[]>
tagOptByRemote: Record<string, string>
} {
const fetchRefspecs: Record<string, string[]> = { ...fetchRefspecsByRemote }
const tagOptByRemote: Record<string, string> = {}
const orcaCreated = new Set(orcaCreatedRemotes)
const exec = vi.fn<GitRemoteExec>(async (args: string[]) => {
if (args[0] === 'config') {
const configKey =
args[1] === '--get-all' ||
args[1] === '--unset-all' ||
args[1] === '--add' ||
args[1] === '--get'
? args[2]
: args[1]
const fetchMatch = /^remote\.(.+)\.fetch$/.exec(configKey ?? '')
const markerMatch = /^remote\.(.+)\.orca-created$/.exec(configKey ?? '')
if (args[1] === '--get-all' && fetchMatch) {
const values = fetchRefspecs[fetchMatch[1]!] ?? []
if (values.length === 0) {
throw new Error('key not found')
}
return { stdout: `${values.join('\n')}\n`, stderr: '' }
}
if (args[1] === '--unset-all' && fetchMatch) {
fetchRefspecs[fetchMatch[1]!] = []
return { stdout: '', stderr: '' }
}
if (args[1] === '--add' && fetchMatch) {
fetchRefspecs[fetchMatch[1]!] = [...(fetchRefspecs[fetchMatch[1]!] ?? []), args[3]!]
return { stdout: '', stderr: '' }
}
if (args[1] === '--get' && markerMatch) {
if (!orcaCreated.has(markerMatch[1]!)) {
throw new Error('key not found')
}
return { stdout: 'true\n', stderr: '' }
}
if (args[1]?.startsWith('remote.') && args[1]?.endsWith('.tagOpt')) {
tagOptByRemote[args[1]!.slice('remote.'.length, -'.tagOpt'.length)] = args[2]!
return { stdout: '', stderr: '' }
}
return { stdout: '', stderr: '' }
}
return makeRepoExec(remotes)(args, REPO)
})
return { exec, fetchRefspecs, tagOptByRemote }
}

function forkTarget(overrides: Partial<GitPushTarget> = {}): GitPushTarget {
return {
remoteName: 'pr-contributor-orca',
Expand Down Expand Up @@ -156,6 +213,79 @@ describe('prepareWorktreePushTargetWithExec', () => {
expect(result.remoteCreated).toBe(true)
})

// #25703: a remote the user added by hand (wide default refspec, no provenance marker)
// must keep its own config -- only Orca's per-branch line is appended.
it('preserves a reused user-created remote with no marker and only appends its branch (#25703)', async () => {
const WIDE = '+refs/heads/*:refs/remotes/fork/*'
const { exec, fetchRefspecs, tagOptByRemote } = makeConfigAwareRepoExec(
{
origin: 'git@github.com:stablyai/orca.git',
fork: FORK_HTTPS
},
{ fork: [WIDE] }
)

await prepareWorktreePushTargetWithExec(
exec,
REPO,
{ remoteName: 'pr-contributor-orca', branchName: 'contributor/fix', remoteUrl: FORK_SSH },
() => false
)

expect(fetchRefspecs.fork).toEqual([
WIDE,
'+refs/heads/contributor/fix*:refs/remotes/fork/contributor/fix*'
])
// The remote's fetch behavior is the user's; no tagOpt rewrite either.
expect(tagOptByRemote.fork).toBeUndefined()
})

// The issue's workaround (re-add the wide refspec next to Orca's line) used to be
// undone by the next branch's reuse, because a present wide refspec dropped everything.
it('keeps the workaround wide refspec when a second branch reuses the same user remote (#25703)', async () => {
const WIDE = '+refs/heads/*:refs/remotes/fork/*'
const { exec, fetchRefspecs } = makeConfigAwareRepoExec(
{
origin: 'git@github.com:stablyai/orca.git',
fork: FORK_HTTPS
},
{ fork: [WIDE, '+refs/heads/a*:refs/remotes/fork/a*'] }
)

await prepareWorktreePushTargetWithExec(
exec,
REPO,
{ remoteName: 'pr-contributor-orca', branchName: 'b', remoteUrl: FORK_SSH },
() => false
)

expect(fetchRefspecs.fork).toEqual([
WIDE,
'+refs/heads/a*:refs/remotes/fork/a*',
'+refs/heads/b*:refs/remotes/fork/b*'
])
})

// Green guard: a remote carrying the `orca-created` marker is Orca's own, so the
// narrow rewrite (wide dropped, tagOpt pinned) still applies on reuse.
it('still narrows a reused remote Orca created (orca-created marker set)', async () => {
const { exec, fetchRefspecs, tagOptByRemote } = makeConfigAwareRepoExec(
{
origin: 'git@github.com:stablyai/orca.git',
'pr-contributor-orca': FORK_HTTPS
},
{ 'pr-contributor-orca': ['+refs/heads/*:refs/remotes/pr-contributor-orca/*'] },
['pr-contributor-orca']
)

await prepareWorktreePushTargetWithExec(exec, REPO, forkTarget(), () => false)

expect(fetchRefspecs['pr-contributor-orca']).toEqual([
'+refs/heads/contributor/fix*:refs/remotes/pr-contributor-orca/contributor/fix*'
])
expect(tagOptByRemote['pr-contributor-orca']).toBe('--no-tags')
})

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' })

Expand Down
20 changes: 15 additions & 5 deletions src/main/ipc/worktree-push-target-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,16 @@

import type { GitPushTarget } from '../../shared/worktree/types'
import { findGitRemoteNameByFetchUrl } from '../../shared/git-remote-url-index'
import { sameGitHubRemoteUrl, type GitRemoteExec } from './worktree-push-target-cleanup'
import {
buildNarrowForkFetchRefspec,
ensureRemoteAlsoTracksBranch,
ensureRemoteTracksBranchNarrowly
} from '../git/fork-remote-refspec'
import {
remoteHasOrcaProvenance,
sameGitHubRemoteUrl,
type GitRemoteExec
} from './worktree-push-target-cleanup'

// One `git remote -v` replaces `git remote` plus a serial `git remote get-url` per
// remote -- 59 subprocesses at 58 remotes, on every push-target resolution (#17914).
Expand Down Expand Up @@ -112,10 +117,15 @@ export async function prepareWorktreePushTargetWithExec(
// 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)
// Why: a remote created before this fix (or reused for a second branch on the
// same fork) may still carry the wide default refspec; widen-but-bound it to
// cover this branch too rather than trusting whatever is already configured.
await ensureRemoteTracksBranchNarrowly(execGit, repoPath, remoteName, target.branchName)
// Why (#25703): only a remote Orca minted (marker set) may have its refspec
// rewritten -- narrowing a user-created remote breaks tracking for every other
// branch they push (`@{u}`, ahead/behind, PR lookup). For those, widen add-only:
// append this branch's narrow refspec, never drop or rewrite their entries.
const orcaCreated = await remoteHasOrcaProvenance(execGit, repoPath, remoteName)
const widenRefspec = orcaCreated
? ensureRemoteTracksBranchNarrowly
: ensureRemoteAlsoTracksBranch
await widenRefspec(execGit, repoPath, remoteName, target.branchName)
} else {
remoteName = await ensureUniqueRemoteName(execGit, repoPath, target.remoteName)
// Why: `-t <branch> --no-tags` means this remote is never, even transiently,
Expand Down
50 changes: 50 additions & 0 deletions src/main/ipc/worktree-remote-push-target-materialization.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,15 @@ describe('materializeWorktreePushTargetRemote', () => {
if (args[0] === 'config' && args[1] === '--get-all') {
throw new Error('no such section')
}
// Orca's own minted remote: provenance marker reads true, so the narrow
// rewrite (not the add-only reuse path) applies.
if (
args[0] === 'config' &&
args[1] === '--get' &&
String(args[2]).endsWith('.orca-created')
) {
return { stdout: 'true\n', stderr: '' }
}
if (args[0] === 'symbolic-ref') {
return { stdout: 'contributor/fix\n', stderr: '' }
}
Expand Down Expand Up @@ -86,6 +95,47 @@ describe('materializeWorktreePushTargetRemote', () => {
])
})

// #25703: the deferred materialize path reaches user-created remotes too (the store may
// have persisted a reused remote's name). Without the provenance marker it must widen
// add-only, never rewrite the user's refspec config.
it('widens add-only on the short-circuit path when the remote has no orca-created marker', async () => {
gitExecFileAsyncMock.mockImplementation(async (args: string[]) => {
if (args[0] === 'remote' && args[1] === 'get-url') {
return { stdout: `${FORK_URL}\n`, stderr: '' }
}
if (args[0] === 'config' && args[1] === '--get-all') {
return { stdout: `+refs/heads/*:refs/remotes/${FORK_REMOTE}/*\n`, stderr: '' }
}
if (
args[0] === 'config' &&
args[1] === '--get' &&
String(args[2]).endsWith('.orca-created')
) {
throw new Error('no marker')
}
if (args[0] === 'rev-parse') {
throw new Error('unknown revision')
}
if (args[0] === 'symbolic-ref') {
return { stdout: 'contributor/fix\n', stderr: '' }
}
return { stdout: '', stderr: '' }
})
const target = forkTarget()

await materializeWorktreePushTargetRemote(REPO_PATH, target)

const calls = gitExecFileAsyncMock.mock.calls.map((call) => call[0] as string[])
expect(calls).not.toContainEqual(['config', '--unset-all', `remote.${FORK_REMOTE}.fetch`])
expect(calls).not.toContainEqual(['config', `remote.${FORK_REMOTE}.tagOpt`, '--no-tags'])
expect(calls).toContainEqual([
'config',
'--add',
`remote.${FORK_REMOTE}.fetch`,
`+refs/heads/${target.branchName}*:refs/remotes/${FORK_REMOTE}/${target.branchName}*`
])
})

it('materializes the remote (add + provenance + fetch) when the probe misses', async () => {
gitExecFileAsyncMock.mockImplementation(async (args: string[]) => {
if (args[0] === 'remote' && args[1] === 'get-url') {
Expand Down
8 changes: 7 additions & 1 deletion src/main/ipc/worktree-remote.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ import { parseWorkspaceKey, worktreeWorkspaceKey } from '../../shared/workspace-
import { sharesWorktreeLineageBoundary } from '../../shared/resolved-worktree-lineage'
import {
cleanupUnusedWorktreePushTargetRemoteWithExec,
remoteHasOrcaProvenance,
sameGitHubRemoteUrl,
type GitRemoteExec,
type WorktreePushTargetStore
Expand All @@ -122,6 +123,7 @@ import {
} from './worktree-push-target-setup'
import {
buildNarrowForkFetchRefspec,
ensureRemoteAlsoTracksBranch,
ensureRemoteTracksBranchNarrowly,
forkRemoteTrackingRefExists
} from '../git/fork-remote-refspec'
Expand Down Expand Up @@ -1107,7 +1109,11 @@ async function adoptExistingForkRemoteForBranch(
repoId: string | undefined,
worktreeId: string | undefined
): Promise<GitPushTarget> {
await ensureRemoteTracksBranchNarrowly(execGit, repoPath, target.remoteName, target.branchName)
// Why (#25703): the target's name can be a user-created remote the store persisted after
// a reuse. Without the provenance marker, widen add-only -- never rewrite their refspec.
const orcaCreated = await remoteHasOrcaProvenance(execGit, repoPath, target.remoteName)
const widenRefspec = orcaCreated ? ensureRemoteTracksBranchNarrowly : ensureRemoteAlsoTracksBranch
await widenRefspec(execGit, repoPath, target.remoteName, target.branchName)
// Why: widening only rewrites config -- it never imports anything. For a sibling worktree's
// first materialize of a *new* branch on an already-existing remote, the branch's tracking
// ref doesn't exist yet, and `--set-upstream-to` below hard-fails with "the requested
Expand Down