mirror of
https://github.com/stablyai/orca.git
synced 2026-10-01 00:02:10 +00:00
* 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.
379 lines
14 KiB
TypeScript
379 lines
14 KiB
TypeScript
import { describe, expect, it, vi, type Mock } from 'vitest'
|
|
import type { WorktreeMeta } from '../../shared/worktree/meta-types'
|
|
import type { GitPushTarget } from '../../shared/worktree/types'
|
|
import type { GitRemoteExec, WorktreePushTargetStore } from './worktree-push-target-cleanup'
|
|
import {
|
|
_resetForkRemoteRefspecMigrationRateLimitForTests,
|
|
migrateForkRemoteRefspecsWithExec
|
|
} from './worktree-push-target-refspec-migration'
|
|
|
|
type ExecMock = Mock<GitRemoteExec>
|
|
|
|
const REPO_PATH = '/repo-root'
|
|
const REPO_ID = 'repo-1'
|
|
const FORK_REMOTE = 'pr-contributor-orca'
|
|
|
|
function worktreeId(suffix: string): string {
|
|
return `${REPO_ID}::${suffix}`
|
|
}
|
|
|
|
function forkTarget(overrides: Partial<GitPushTarget> = {}): GitPushTarget {
|
|
return {
|
|
remoteName: FORK_REMOTE,
|
|
branchName: 'contributor/fix',
|
|
remoteUrl: 'git@github.com:contributor/orca.git',
|
|
remoteCreated: true,
|
|
...overrides
|
|
}
|
|
}
|
|
|
|
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 = {
|
|
fetchByRemote?: Record<string, string[]>
|
|
urlByRemote?: Record<string, string>
|
|
branchConfig?: string
|
|
trackingRefsByRemote?: Record<string, string[]>
|
|
// Simulates #17842's reconciliation sweep concurrently `remote remove`-ing this
|
|
// remote in between this migration's pre-write and post-write existence checks.
|
|
removeUrlAfterFirstCheck?: Set<string>
|
|
// Remotes `git remote` (bare list) reports on disk -- drives discovery of a `pr-*`
|
|
// remote with zero worktree-metadata trace at all.
|
|
remoteNames?: string[]
|
|
}
|
|
|
|
function makeExec(script: ExecScript = {}): ExecMock {
|
|
const {
|
|
fetchByRemote = {},
|
|
urlByRemote = {},
|
|
branchConfig = '',
|
|
trackingRefsByRemote = {},
|
|
removeUrlAfterFirstCheck = new Set<string>(),
|
|
remoteNames = []
|
|
} = script
|
|
const urlCheckCountByRemote: Record<string, number> = {}
|
|
return vi.fn<GitRemoteExec>(async (args: string[]) => {
|
|
if (args[0] === 'remote' && args.length === 1) {
|
|
return { stdout: remoteNames.length ? `${remoteNames.join('\n')}\n` : '', stderr: '' }
|
|
}
|
|
if (args[0] === 'config' && args[1] === '--get' && args[2]!.endsWith('.url')) {
|
|
const remoteName = args[2]!.slice('remote.'.length, -'.url'.length)
|
|
urlCheckCountByRemote[remoteName] = (urlCheckCountByRemote[remoteName] ?? 0) + 1
|
|
const concurrentlyRemoved =
|
|
removeUrlAfterFirstCheck.has(remoteName) && urlCheckCountByRemote[remoteName]! > 1
|
|
const url = concurrentlyRemoved ? undefined : urlByRemote[remoteName]
|
|
if (!url) {
|
|
throw new Error(`no such remote ${remoteName}`)
|
|
}
|
|
return { stdout: url, stderr: '' }
|
|
}
|
|
if (args[0] === 'config' && args[1] === '--remove-section' && args[2]!.startsWith('remote.')) {
|
|
const remoteName = args[2]!.slice('remote.'.length)
|
|
delete fetchByRemote[remoteName]
|
|
delete urlByRemote[remoteName]
|
|
return { stdout: '', stderr: '' }
|
|
}
|
|
if (args[0] === 'config' && args[1] === '--get-regexp') {
|
|
return { stdout: branchConfig, stderr: '' }
|
|
}
|
|
if (args[0] === 'config' && args[1] === '--get-all' && args[2]!.endsWith('.fetch')) {
|
|
const remoteName = args[2]!.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' && args[2]!.endsWith('.fetch')) {
|
|
const remoteName = args[2]!.slice('remote.'.length, -'.fetch'.length)
|
|
fetchByRemote[remoteName] = []
|
|
return { stdout: '', stderr: '' }
|
|
}
|
|
if (args[0] === 'config' && args[1] === '--add' && args[2]!.endsWith('.fetch')) {
|
|
const remoteName = args[2]!.slice('remote.'.length, -'.fetch'.length)
|
|
fetchByRemote[remoteName] = [...(fetchByRemote[remoteName] ?? []), args[3]!]
|
|
return { stdout: '', stderr: '' }
|
|
}
|
|
if (args[0] === 'config' && args[1]?.endsWith('.tagOpt')) {
|
|
return { stdout: '', stderr: '' }
|
|
}
|
|
if (args[0] === 'for-each-ref') {
|
|
const prefix = args[2]!
|
|
const remoteName = prefix.replace('refs/remotes/', '').replace(/\/$/, '')
|
|
const refs = trackingRefsByRemote[remoteName] ?? []
|
|
return {
|
|
stdout: refs.length ? `${refs.map((r) => `${prefix}${r}`).join('\n')}\n` : '',
|
|
stderr: ''
|
|
}
|
|
}
|
|
if (args[0] === 'update-ref' && args[1] === '-d') {
|
|
const refname = args[2]!
|
|
for (const [remoteName, refs] of Object.entries(trackingRefsByRemote)) {
|
|
const prefix = `refs/remotes/${remoteName}/`
|
|
if (refname.startsWith(prefix)) {
|
|
trackingRefsByRemote[remoteName] = refs.filter((r) => `${prefix}${r}` !== refname)
|
|
}
|
|
}
|
|
return { stdout: '', stderr: '' }
|
|
}
|
|
return { stdout: '', stderr: '' }
|
|
})
|
|
}
|
|
|
|
describe('migrateForkRemoteRefspecsWithExec', () => {
|
|
it('narrows a wide-refspec remote to the known branch and deletes the strays left by the old wide fetch', async () => {
|
|
const trackingRefsByRemote = {
|
|
[FORK_REMOTE]: ['contributor/fix', 'contributor/unrelated-1', 'master']
|
|
}
|
|
const exec = makeExec({
|
|
urlByRemote: { [FORK_REMOTE]: 'git@github.com:contributor/orca.git\n' },
|
|
fetchByRemote: { [FORK_REMOTE]: ['+refs/heads/*:refs/remotes/pr-contributor-orca/*'] },
|
|
trackingRefsByRemote
|
|
})
|
|
|
|
const migrated = await migrateForkRemoteRefspecsWithExec(
|
|
REPO_PATH,
|
|
REPO_ID,
|
|
storeOf({ [worktreeId('/wt/a')]: forkTarget() }),
|
|
exec
|
|
)
|
|
|
|
expect(migrated).toEqual([FORK_REMOTE])
|
|
expect(exec.mock.calls).toContainEqual([
|
|
[
|
|
'config',
|
|
'--add',
|
|
`remote.${FORK_REMOTE}.fetch`,
|
|
'+refs/heads/contributor/fix*:refs/remotes/pr-contributor-orca/contributor/fix*'
|
|
],
|
|
REPO_PATH
|
|
])
|
|
// `fetch --prune` cannot reclaim strays under a narrow refspec (verified against real
|
|
// git); the migration must delete them directly instead.
|
|
expect(exec.mock.calls.some(([args]) => args[0] === 'fetch' && args[1] === '--prune')).toBe(
|
|
false
|
|
)
|
|
expect(trackingRefsByRemote[FORK_REMOTE]).toEqual(['contributor/fix'])
|
|
})
|
|
|
|
it('unions branches from multiple worktrees sharing the same fork remote', async () => {
|
|
const exec = makeExec({
|
|
urlByRemote: { [FORK_REMOTE]: 'git@github.com:contributor/orca.git\n' },
|
|
fetchByRemote: { [FORK_REMOTE]: ['+refs/heads/*:refs/remotes/pr-contributor-orca/*'] }
|
|
})
|
|
|
|
await migrateForkRemoteRefspecsWithExec(
|
|
REPO_PATH,
|
|
REPO_ID,
|
|
storeOf({
|
|
[worktreeId('/wt/a')]: forkTarget({ branchName: 'branch-a' }),
|
|
[worktreeId('/wt/b')]: forkTarget({ branchName: 'branch-b', remoteCreated: false })
|
|
}),
|
|
exec
|
|
)
|
|
|
|
const addedRefspecs = exec.mock.calls
|
|
.filter(([args]) => args[0] === 'config' && args[1] === '--add')
|
|
.map(([args]) => args[3])
|
|
expect(addedRefspecs).toEqual(
|
|
expect.arrayContaining([
|
|
'+refs/heads/branch-a*:refs/remotes/pr-contributor-orca/branch-a*',
|
|
'+refs/heads/branch-b*:refs/remotes/pr-contributor-orca/branch-b*'
|
|
])
|
|
)
|
|
})
|
|
|
|
it('skips a remote with no provenance evidence (no metadata entry has remoteCreated: true)', async () => {
|
|
const exec = makeExec({
|
|
urlByRemote: { [FORK_REMOTE]: 'git@github.com:contributor/orca.git\n' },
|
|
fetchByRemote: { [FORK_REMOTE]: ['+refs/heads/*:refs/remotes/pr-contributor-orca/*'] }
|
|
})
|
|
|
|
const migrated = await migrateForkRemoteRefspecsWithExec(
|
|
REPO_PATH,
|
|
REPO_ID,
|
|
storeOf({ [worktreeId('/wt/a')]: forkTarget({ remoteCreated: false }) }),
|
|
exec
|
|
)
|
|
|
|
expect(migrated).toEqual([])
|
|
expect(exec.mock.calls.some(([args]) => args[1] === '--unset-all')).toBe(false)
|
|
})
|
|
|
|
it('never touches origin or upstream even with malformed metadata', async () => {
|
|
const exec = makeExec({ urlByRemote: { origin: 'git@github.com:stablyai/orca.git\n' } })
|
|
|
|
const migrated = await migrateForkRemoteRefspecsWithExec(
|
|
REPO_PATH,
|
|
REPO_ID,
|
|
storeOf({ [worktreeId('/wt/a')]: forkTarget({ remoteName: 'origin' }) }),
|
|
exec
|
|
)
|
|
|
|
expect(migrated).toEqual([])
|
|
})
|
|
|
|
it('scopes provenance to the same repo (remotes are repo-local)', async () => {
|
|
const exec = makeExec({
|
|
urlByRemote: { [FORK_REMOTE]: 'git@github.com:contributor/orca.git\n' },
|
|
fetchByRemote: { [FORK_REMOTE]: ['+refs/heads/*:refs/remotes/pr-contributor-orca/*'] }
|
|
})
|
|
|
|
const migrated = await migrateForkRemoteRefspecsWithExec(
|
|
REPO_PATH,
|
|
REPO_ID,
|
|
storeOf({ 'repo-2::/wt/other-repo': forkTarget() }),
|
|
exec
|
|
)
|
|
|
|
expect(migrated).toEqual([])
|
|
})
|
|
|
|
it('skips (does not re-fetch) a remote that is already narrow', async () => {
|
|
const exec = makeExec({
|
|
urlByRemote: { [FORK_REMOTE]: 'git@github.com:contributor/orca.git\n' },
|
|
fetchByRemote: {
|
|
[FORK_REMOTE]: [
|
|
'+refs/heads/contributor/fix*:refs/remotes/pr-contributor-orca/contributor/fix*'
|
|
]
|
|
}
|
|
})
|
|
|
|
const migrated = await migrateForkRemoteRefspecsWithExec(
|
|
REPO_PATH,
|
|
REPO_ID,
|
|
storeOf({ [worktreeId('/wt/a')]: forkTarget() }),
|
|
exec
|
|
)
|
|
|
|
expect(migrated).toEqual([])
|
|
expect(exec.mock.calls.some(([args]) => args[0] === 'fetch')).toBe(false)
|
|
})
|
|
|
|
it('abandons and cleans up a remote reclaimed concurrently by #17842 reconciliation mid-migration', async () => {
|
|
const exec = makeExec({
|
|
urlByRemote: { [FORK_REMOTE]: 'git@github.com:contributor/orca.git\n' },
|
|
fetchByRemote: { [FORK_REMOTE]: ['+refs/heads/*:refs/remotes/pr-contributor-orca/*'] },
|
|
removeUrlAfterFirstCheck: new Set([FORK_REMOTE])
|
|
})
|
|
|
|
const migrated = await migrateForkRemoteRefspecsWithExec(
|
|
REPO_PATH,
|
|
REPO_ID,
|
|
storeOf({ [worktreeId('/wt/a')]: forkTarget() }),
|
|
exec
|
|
)
|
|
|
|
// Not reported as migrated -- reconciliation won the race, so this sweep backs off.
|
|
expect(migrated).toEqual([])
|
|
expect(
|
|
exec.mock.calls.some(
|
|
([args]) =>
|
|
args[0] === 'config' &&
|
|
args[1] === '--remove-section' &&
|
|
args[2] === `remote.${FORK_REMOTE}`
|
|
)
|
|
).toBe(true)
|
|
// No fetch --prune-equivalent local ref deletion ran for a remote that's already gone.
|
|
expect(exec.mock.calls.some(([args]) => args[0] === 'update-ref')).toBe(false)
|
|
})
|
|
|
|
it('clears the fetch refspec of a wide pr-* remote with zero worktree-metadata trace at all', async () => {
|
|
const ORPHAN_REMOTE = 'pr-ghost-orca'
|
|
const trackingRefsByRemote = { [ORPHAN_REMOTE]: ['some-branch', 'another-branch'] }
|
|
const exec = makeExec({
|
|
remoteNames: [ORPHAN_REMOTE],
|
|
urlByRemote: { [ORPHAN_REMOTE]: 'git@github.com:ghost/orca.git\n' },
|
|
fetchByRemote: { [ORPHAN_REMOTE]: ['+refs/heads/*:refs/remotes/pr-ghost-orca/*'] },
|
|
trackingRefsByRemote
|
|
})
|
|
|
|
// No worktree metadata references this remote at all (worktree removed outside
|
|
// preserve-on-delete, metadata purged) -- only discoverable via `git remote`.
|
|
const migrated = await migrateForkRemoteRefspecsWithExec(REPO_PATH, REPO_ID, storeOf({}), exec)
|
|
|
|
expect(migrated).toEqual([ORPHAN_REMOTE])
|
|
expect(exec.mock.calls).toContainEqual([
|
|
['config', '--unset-all', `remote.${ORPHAN_REMOTE}.fetch`],
|
|
REPO_PATH
|
|
])
|
|
// No branch to narrow to, so it never adds a replacement refspec.
|
|
expect(exec.mock.calls.some(([args]) => args[0] === 'config' && args[1] === '--add')).toBe(
|
|
false
|
|
)
|
|
// Every stray tracking ref is pruned, same as the narrowing path.
|
|
expect(trackingRefsByRemote[ORPHAN_REMOTE]).toEqual([])
|
|
})
|
|
|
|
it('leaves a zero-provenance pr-* remote alone if its refspec is not the stock wide default', async () => {
|
|
const CUSTOM_REMOTE = 'pr-custom-orca'
|
|
const exec = makeExec({
|
|
remoteNames: [CUSTOM_REMOTE],
|
|
urlByRemote: { [CUSTOM_REMOTE]: 'git@github.com:custom/orca.git\n' },
|
|
fetchByRemote: {
|
|
[CUSTOM_REMOTE]: ['+refs/heads/some-branch:refs/remotes/pr-custom-orca/some-branch']
|
|
}
|
|
})
|
|
|
|
const migrated = await migrateForkRemoteRefspecsWithExec(REPO_PATH, REPO_ID, storeOf({}), exec)
|
|
|
|
expect(migrated).toEqual([])
|
|
expect(exec.mock.calls.some(([args]) => args[1] === '--unset-all')).toBe(false)
|
|
})
|
|
|
|
it('never discovers a non-pr-prefixed remote through the bare listing, even if wide', async () => {
|
|
const exec = makeExec({
|
|
remoteNames: ['some-other-remote'],
|
|
urlByRemote: { 'some-other-remote': 'git@github.com:someone/else.git\n' },
|
|
fetchByRemote: { 'some-other-remote': ['+refs/heads/*:refs/remotes/some-other-remote/*'] }
|
|
})
|
|
|
|
const migrated = await migrateForkRemoteRefspecsWithExec(REPO_PATH, REPO_ID, storeOf({}), exec)
|
|
|
|
expect(migrated).toEqual([])
|
|
expect(exec.mock.calls.some(([args]) => args[1] === '--unset-all')).toBe(false)
|
|
})
|
|
|
|
it('also narrows branches only referenced by surviving branch.*.remote config (no metadata left)', async () => {
|
|
const exec = makeExec({
|
|
urlByRemote: { [FORK_REMOTE]: 'git@github.com:contributor/orca.git\n' },
|
|
fetchByRemote: { [FORK_REMOTE]: ['+refs/heads/*:refs/remotes/pr-contributor-orca/*'] },
|
|
branchConfig: `branch.contributor/preserved.remote ${FORK_REMOTE}`
|
|
})
|
|
|
|
const migrated = await migrateForkRemoteRefspecsWithExec(
|
|
REPO_PATH,
|
|
REPO_ID,
|
|
// Only proof of Orca provenance; the branch itself comes from local config.
|
|
storeOf({ [worktreeId('/wt/gone')]: forkTarget({ branchName: 'contributor/fix' }) }),
|
|
exec
|
|
)
|
|
|
|
expect(migrated).toEqual([FORK_REMOTE])
|
|
const addedRefspecs = exec.mock.calls
|
|
.filter(([args]) => args[0] === 'config' && args[1] === '--add')
|
|
.map(([args]) => args[3])
|
|
expect(addedRefspecs).toEqual(
|
|
expect.arrayContaining([
|
|
'+refs/heads/contributor/preserved*:refs/remotes/pr-contributor-orca/contributor/preserved*'
|
|
])
|
|
)
|
|
})
|
|
})
|
|
|
|
describe('fork remote refspec migration rate limiting', () => {
|
|
it('exposes a test reset so repeated test runs are not affected by prior cooldowns', () => {
|
|
expect(() => _resetForkRemoteRefspecMigrationRateLimitForTests()).not.toThrow()
|
|
})
|
|
})
|