Files
orca/src/main/git/fork-remote-refspec.test.ts
T
Neil f176e49478 fix(git): narrow fork-remote fetch refspecs to tracked branches (#17887)
* fix(git): narrow fork-remote fetch refspecs to tracked branches

git remote add with no -t writes the wide +refs/heads/*:refs/remotes/<name>/*
refspec, so any later plain `git fetch` (user, agent, or Orca's own Fetch
action) re-imports a fork's entire branch set and its tags -- one real
machine had ~50 leaked/wide fork remotes producing 59,716 remote-tracking
refs. Mint and reuse now pin -t <branch> --no-tags; a rate-limited sweep
narrows and cleans up remotes minted before this fix; gitFetch self-heals
when a narrowed remote's tracked branch is later deleted upstream.

Refs #17828

* fix(git): soften narrow fork-remote refspec against deleted upstream branches

A bare `git fetch` in a worktree checked out on a fork-PR branch resolves to
the pr-* remote via branch.<name>.remote -- not origin -- making it the
dominant fetch shape in Orca's terminal-centric, agent-driven usage. The
previous literal-refspec design hard-failed that fetch ("couldn't find
remote ref") the moment the tracked branch was deleted/renamed upstream,
which is not the narrow edge case it was first described as.

Switch to a trailing-`*`-suffixed refspec source/destination
(refs/heads/<branch>*:refs/remotes/<name>/<branch>*). Verified against real
git: this restores wildcard zero-match tolerance (silent no-op instead of a
hard failure) and lets plain `git fetch --prune` reclaim the stale ref once
the branch disappears, at the cost of also matching sibling branches that
share the literal name as a prefix -- a materially smaller widening than the
original unbounded-import bug.

Also close a race with #17842's orphaned-pr-remote reconciliation sweep:
both sweeps read the same worktree-metadata store to pick candidate remotes,
so reconciliation can `remote remove` a remote this migration is
concurrently narrowing. `ensureRemoteTracksBranchNarrowly`'s plain `config
--add` would silently resurrect a url-less config section in that case;
re-check `remote.<name>.url` (via the new `remoteHasUrl`, plumbing rather
than porcelain `remote get-url`, which falls back to echoing the remote name
as a bogus URL) after the narrowing writes and remove the section if it's
gone.

* fix(git): update stale fork-remote mint assertions for -t/--no-tags and wildcard-suffix refspec

Four test files still asserted the pre-#17828 remote-add shape or the
literal (non-wildcard-suffixed) fetch refspec from before the
deleted-upstream-branch softening commit, so CI went red on that HEAD:

- worktree-push-target-refspec-real-git.test.ts: the migration fixture
  asserted a hardcoded tracked-ref count before narrowing. Under git
  >= 2.44, `followRemoteHEAD` auto-creates a `refs/remotes/<name>/HEAD`
  symref on the first fetch matching the full wildcard refspec, adding
  one untracked ref. Made the count/assertions robust to that ref's
  presence instead of hand-tuning the constant per git version.
- worktrees-wsl-runtime-routing.test.ts: assertions predated both the
  `-t <branch> --no-tags` mint change and the wildcard-suffix refspec
  change; updated to the full, correct call sequence and confirmed the
  WSL routing options (cwd, wslDistro) are threaded to every call.
- worktrees-create-metadata-persistence.test.ts and
  orca-runtime-tests/worktree-removal-and-reconciliation.spec.ts: same
  class of staleness, found via CI job log cross-referencing rather
  than being explicitly flagged.

Verified out of scope: the SSH fork-remote mint path
(prepareWorktreePushTargetSsh) is untouched by this PR -- it never
persists a `remote.<name>.fetch` refspec at all, using
provider.fetchRemoteTrackingRef for a targeted per-branch fetch
instead -- so worktrees-ssh-fork-push-target-remote.test.ts needed no
change.

* fix(git): migrate pr-* remotes with zero worktree-metadata trace too

The migration sweep's candidate discovery was purely metadata-driven
(store.getAllWorktreeMeta()), so a pr-* remote whose every referencing
worktree was removed outside preserve-on-delete (metadata purged, not
just the worktree) was permanently invisible to it and stayed on the
wide default forever.

Field data from a manual migration run against a real user's repo (31
pr-* remotes, 34,637 tracking refs, only 18 actually needed) found
exactly this: 15 of 31 remotes had no branch pinning them at all.

Widen discovery to every pr-* remote git reports on disk, in addition
to metadata-derived candidates. For a remote with no branch provenance
from either metadata or surviving branch.*.remote/.pushRemote config,
there's nothing to narrow *to* -- clear its fetch refspec entirely
instead (stays pushable, imports nothing on a plain fetch), gated on
it still carrying the untouched stock wide default so a user's own
custom pr-*-named remote isn't touched. Removing the remote outright
stays #17842's job.

Adds clearForkRemoteFetchRefspec (fork-remote-refspec.ts), 3 new
mocked-exec tests, and a real-git integration test proving a
subsequent plain `git fetch` on the cleared remote imports nothing.
2026-09-01 03:45:09 -07:00

220 lines
8.1 KiB
TypeScript

import { describe, expect, it, vi, type Mock } from 'vitest'
import {
buildNarrowForkFetchRefspec,
ensureRemoteTracksBranchNarrowly,
getRemoteFetchRefspecs,
pruneUntrackedForkRemoteRefs,
removeStaleForkFetchRefspec,
wildcardForkFetchRefspec,
type GitExecFn
} from './fork-remote-refspec'
type ExecMock = Mock<GitExecFn>
const REPO = '/repo-root'
// In-memory `remote.<name>.fetch`/`.tagOpt` config, mutated the way real `git config` would be.
function makeConfigExec(fetchByRemote: Record<string, string[]> = {}): {
exec: ExecMock
tagOptByRemote: Record<string, string>
} {
const tagOptByRemote: Record<string, string> = {}
const exec = vi.fn<GitExecFn>(async (args: string[]) => {
const key = args[2]
if (args[0] === 'config' && args[1] === '--get-all' && key?.endsWith('.fetch')) {
const remoteName = key.slice('remote.'.length, -'.fetch'.length)
const values = fetchByRemote[remoteName] ?? []
if (values.length === 0) {
throw new Error('key not found')
}
return { stdout: `${values.join('\n')}\n`, stderr: '' }
}
if (args[0] === 'config' && args[1] === '--unset-all' && key?.endsWith('.fetch')) {
const remoteName = key.slice('remote.'.length, -'.fetch'.length)
fetchByRemote[remoteName] = []
return { stdout: '', stderr: '' }
}
if (args[0] === 'config' && args[1] === '--add' && key?.endsWith('.fetch')) {
const remoteName = key.slice('remote.'.length, -'.fetch'.length)
fetchByRemote[remoteName] = [...(fetchByRemote[remoteName] ?? []), args[3]!]
return { stdout: '', stderr: '' }
}
if (args[0] === 'config' && args[1]?.endsWith('.tagOpt')) {
const remoteName = args[1].slice('remote.'.length, -'.tagOpt'.length)
tagOptByRemote[remoteName] = args[2]!
return { stdout: '', stderr: '' }
}
return { stdout: '', stderr: '' }
})
return { exec, tagOptByRemote }
}
describe('buildNarrowForkFetchRefspec / wildcardForkFetchRefspec', () => {
it('builds the narrow (trailing-* suffixed) and wide refspec shapes', () => {
expect(buildNarrowForkFetchRefspec('fork', 'feature/fix')).toBe(
'+refs/heads/feature/fix*:refs/remotes/fork/feature/fix*'
)
expect(wildcardForkFetchRefspec('fork')).toBe('+refs/heads/*:refs/remotes/fork/*')
})
})
describe('getRemoteFetchRefspecs', () => {
it('returns [] when the remote has no configured refspec', async () => {
const { exec } = makeConfigExec()
await expect(getRemoteFetchRefspecs(exec, REPO, 'fork')).resolves.toEqual([])
})
it('returns every configured refspec', async () => {
const { exec } = makeConfigExec({
fork: ['+refs/heads/a:refs/remotes/fork/a', '+refs/heads/b:refs/remotes/fork/b']
})
await expect(getRemoteFetchRefspecs(exec, REPO, 'fork')).resolves.toEqual([
'+refs/heads/a:refs/remotes/fork/a',
'+refs/heads/b:refs/remotes/fork/b'
])
})
})
describe('ensureRemoteTracksBranchNarrowly', () => {
it('replaces the wide default refspec with a single trailing-* narrow one', async () => {
const { exec, tagOptByRemote } = makeConfigExec({ fork: [wildcardForkFetchRefspec('fork')] })
await ensureRemoteTracksBranchNarrowly(exec, REPO, 'fork', 'main')
await expect(getRemoteFetchRefspecs(exec, REPO, 'fork')).resolves.toEqual([
'+refs/heads/main*:refs/remotes/fork/main*'
])
expect(tagOptByRemote.fork).toBe('--no-tags')
})
it('adds a second branch alongside an already-narrow one instead of replacing it', async () => {
const { exec } = makeConfigExec({ fork: ['+refs/heads/main*:refs/remotes/fork/main*'] })
await ensureRemoteTracksBranchNarrowly(exec, REPO, 'fork', 'feature')
await expect(getRemoteFetchRefspecs(exec, REPO, 'fork')).resolves.toEqual([
'+refs/heads/main*:refs/remotes/fork/main*',
'+refs/heads/feature*:refs/remotes/fork/feature*'
])
})
it('is a no-op for a branch already narrowly tracked (idempotent)', async () => {
const { exec } = makeConfigExec({ fork: ['+refs/heads/main*:refs/remotes/fork/main*'] })
await ensureRemoteTracksBranchNarrowly(exec, REPO, 'fork', 'main')
const addCalls = exec.mock.calls.filter(([args]) => args[1] === '--add')
expect(addCalls).toEqual([])
})
it('replaces a stray literal (non-suffixed) entry for the same branch with the suffixed form', async () => {
const { exec } = makeConfigExec({ fork: ['+refs/heads/main:refs/remotes/fork/main'] })
await ensureRemoteTracksBranchNarrowly(exec, REPO, 'fork', 'main')
await expect(getRemoteFetchRefspecs(exec, REPO, 'fork')).resolves.toEqual([
'+refs/heads/main*:refs/remotes/fork/main*'
])
})
it('leaves a different branch entry untouched when replacing a stray literal', async () => {
const { exec } = makeConfigExec({
fork: [
'+refs/heads/main:refs/remotes/fork/main',
'+refs/heads/other*:refs/remotes/fork/other*'
]
})
await ensureRemoteTracksBranchNarrowly(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*'
])
})
})
describe('removeStaleForkFetchRefspec', () => {
it('drops only the refspec whose source matches the stale branch', async () => {
const { exec } = makeConfigExec({
fork: ['+refs/heads/gone:refs/remotes/fork/gone', '+refs/heads/keep:refs/remotes/fork/keep']
})
await expect(removeStaleForkFetchRefspec(exec, REPO, 'fork', 'gone')).resolves.toBe(true)
await expect(getRemoteFetchRefspecs(exec, REPO, 'fork')).resolves.toEqual([
'+refs/heads/keep:refs/remotes/fork/keep'
])
})
it('returns false and changes nothing when the branch is not tracked', async () => {
const { exec } = makeConfigExec({ fork: ['+refs/heads/keep:refs/remotes/fork/keep'] })
await expect(removeStaleForkFetchRefspec(exec, REPO, 'fork', 'gone')).resolves.toBe(false)
await expect(getRemoteFetchRefspecs(exec, REPO, 'fork')).resolves.toEqual([
'+refs/heads/keep:refs/remotes/fork/keep'
])
})
})
describe('pruneUntrackedForkRemoteRefs', () => {
function makeRefsExec(refs: string[]): { exec: Mock<GitExecFn>; refs: string[] } {
const state = [...refs]
const exec = vi.fn<GitExecFn>(async (args: string[]) => {
if (args[0] === 'for-each-ref') {
const prefix = args[2]!
return {
stdout: state.map((r) => `${prefix}${r}`).join('\n') + (state.length ? '\n' : ''),
stderr: ''
}
}
if (args[0] === 'update-ref' && args[1] === '-d') {
const refname = args[2]!
const idx = state.findIndex((r) => refname.endsWith(`/${r}`))
if (idx !== -1) {
state.splice(idx, 1)
}
return { stdout: '', stderr: '' }
}
return { stdout: '', stderr: '' }
})
return { exec, refs: state }
}
it('deletes tracking refs outside the keep set and leaves the rest', async () => {
const { exec, refs } = makeRefsExec(['main', 'unrelated-1', 'unrelated-2'])
const deleted = await pruneUntrackedForkRemoteRefs(exec, REPO, 'fork', new Set(['main']))
expect(deleted.sort()).toEqual(
['refs/remotes/fork/unrelated-1', 'refs/remotes/fork/unrelated-2'].sort()
)
expect(refs).toEqual(['main'])
})
it('never deletes HEAD even if not in the keep set', async () => {
const { exec, refs } = makeRefsExec(['HEAD', 'main'])
await pruneUntrackedForkRemoteRefs(exec, REPO, 'fork', new Set(['main']))
expect(refs).toEqual(['HEAD', 'main'])
})
it('is a no-op when nothing is stray', async () => {
const { exec, refs } = makeRefsExec(['main'])
const deleted = await pruneUntrackedForkRemoteRefs(exec, REPO, 'fork', new Set(['main']))
expect(deleted).toEqual([])
expect(refs).toEqual(['main'])
})
it("keeps a ref that shares a branch prefix, matching the refspec's own trailing-* match", async () => {
const { exec, refs } = makeRefsExec(['fix', 'fix-extra', 'unrelated'])
const deleted = await pruneUntrackedForkRemoteRefs(exec, REPO, 'fork', new Set(['fix']))
expect(deleted).toEqual(['refs/remotes/fork/unrelated'])
expect(refs.sort()).toEqual(['fix', 'fix-extra'])
})
})