mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 08:37:49 +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.
250 lines
11 KiB
TypeScript
250 lines
11 KiB
TypeScript
// Real-binary coverage for #17828: the mocked-exec suites in
|
|
// `worktree-push-target-setup.test.ts` / `worktree-push-target-refspec-migration.test.ts`
|
|
// prove the decision logic, but not that real `git remote add -t/--no-tags`, a plain
|
|
// `git fetch <remote>`, and `git config --get-all/--unset-all/--add` behave the way this
|
|
// fix assumes. In particular this proves the core claim of the fix: a minted remote never
|
|
// imports more than the branch(es) Orca asked for, even from a fork with many branches.
|
|
import { execFile } from 'node:child_process'
|
|
import { mkdir, mkdtemp, realpath, rm, writeFile } from 'node:fs/promises'
|
|
import { tmpdir } from 'node:os'
|
|
import { join } from 'node:path'
|
|
import { promisify } from 'node:util'
|
|
import { afterEach, beforeEach, describe, expect, it } 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 { prepareWorktreePushTargetWithExec } from './worktree-push-target-setup'
|
|
import { migrateForkRemoteRefspecsWithExec } from './worktree-push-target-refspec-migration'
|
|
|
|
const execFileAsync = promisify(execFile)
|
|
|
|
const REPO_ID = 'repo-1'
|
|
const FORK_REMOTE = 'pr-contributor-orca'
|
|
const TRACKED_BRANCH = 'contributor/fix'
|
|
const OTHER_FORK_BRANCHES = Array.from({ length: 12 }, (_, i) => `contributor/unrelated-${i}`)
|
|
|
|
let scratchDir = ''
|
|
let repoPath = ''
|
|
let forkPath = ''
|
|
|
|
async function git(args: string[], cwd: string): Promise<string> {
|
|
const { stdout } = await execFileAsync('git', args, { cwd })
|
|
return stdout
|
|
}
|
|
|
|
const execGit: GitRemoteExec = (args, cwd) => execFileAsync('git', args, { cwd })
|
|
|
|
async function setIdentity(cwd: string): Promise<void> {
|
|
await git(['config', 'user.name', 'Orca Test'], cwd)
|
|
await git(['config', 'user.email', 'orca@example.test'], cwd)
|
|
await git(['config', 'commit.gpgSign', 'false'], cwd)
|
|
await git(['config', 'core.hooksPath', '.git/no-hooks'], cwd)
|
|
}
|
|
|
|
async function trackedRefsUnder(remoteName: string): Promise<string[]> {
|
|
const stdout = await git(
|
|
['for-each-ref', '--format=%(refname:short)', `refs/remotes/${remoteName}/`],
|
|
repoPath
|
|
)
|
|
return stdout.split(/\r?\n/).filter(Boolean)
|
|
}
|
|
|
|
function worktreeId(suffix: string): string {
|
|
return `${REPO_ID}::${suffix}`
|
|
}
|
|
|
|
function storeOf(entries: Record<string, GitPushTarget | undefined>): WorktreePushTargetStore {
|
|
const meta: Record<string, WorktreeMeta> = {}
|
|
for (const [id, pushTarget] of Object.entries(entries)) {
|
|
meta[id] = { pushTarget } as unknown as WorktreeMeta
|
|
}
|
|
return { getAllWorktreeMeta: () => meta }
|
|
}
|
|
|
|
beforeEach(async () => {
|
|
// realpath: macOS hands out /var/... temp paths while Git reports /private/var/...
|
|
scratchDir = await realpath(await mkdtemp(join(tmpdir(), 'orca-fork-refspec-')))
|
|
repoPath = join(scratchDir, 'repo')
|
|
forkPath = join(scratchDir, 'fork')
|
|
await mkdir(repoPath, { recursive: true })
|
|
await git(['init', '-q'], repoPath)
|
|
await setIdentity(repoPath)
|
|
await writeFile(join(repoPath, 'seed.txt'), 'seed\n')
|
|
await git(['add', '-A'], repoPath)
|
|
await git(['commit', '-qm', 'seed'], repoPath)
|
|
|
|
// A large fork: the tracked PR branch plus a dozen unrelated branches, simulating the
|
|
// 1000+-branch forks the issue describes (scaled down for test speed).
|
|
await git(['clone', '-q', repoPath, forkPath], scratchDir)
|
|
await setIdentity(forkPath)
|
|
for (const branch of [TRACKED_BRANCH, ...OTHER_FORK_BRANCHES]) {
|
|
await git(['checkout', '-qb', branch], forkPath)
|
|
await writeFile(join(forkPath, `${branch.replace(/\//g, '-')}.txt`), 'x\n')
|
|
await git(['add', '-A'], forkPath)
|
|
await git(['commit', '-qm', branch], forkPath)
|
|
}
|
|
})
|
|
|
|
afterEach(async () => {
|
|
await rm(scratchDir, { recursive: true, force: true })
|
|
})
|
|
|
|
describe('minting a fork remote against the real Git binary (#17828)', () => {
|
|
it('caps the remote at exactly one fetch refspec, and a plain `git fetch` imports only that branch', async () => {
|
|
const target: GitPushTarget = {
|
|
remoteName: FORK_REMOTE,
|
|
branchName: TRACKED_BRANCH,
|
|
remoteUrl: forkPath
|
|
}
|
|
|
|
await prepareWorktreePushTargetWithExec(execGit, repoPath, target, () => false)
|
|
|
|
const refspecs = (await git(['config', '--get-all', `remote.${FORK_REMOTE}.fetch`], repoPath))
|
|
.split(/\r?\n/)
|
|
.filter(Boolean)
|
|
expect(refspecs).toEqual([
|
|
`+refs/heads/${TRACKED_BRANCH}*:refs/remotes/${FORK_REMOTE}/${TRACKED_BRANCH}*`
|
|
])
|
|
await expect(
|
|
git(['config', '--get', `remote.${FORK_REMOTE}.tagOpt`], repoPath)
|
|
).resolves.toContain('--no-tags')
|
|
|
|
// The critical claim: a later plain `git fetch <remote>` (no explicit refspec) --
|
|
// exactly what Orca's own Fetch action and any agent/user command run -- must not
|
|
// import the fork's other dozen branches.
|
|
await git(['fetch', FORK_REMOTE], repoPath)
|
|
await expect(trackedRefsUnder(FORK_REMOTE)).resolves.toEqual([
|
|
`${FORK_REMOTE}/${TRACKED_BRANCH}`
|
|
])
|
|
})
|
|
|
|
it('a bare `git fetch` (branch-scoped upstream, no remote arg) survives the tracked branch being deleted upstream', async () => {
|
|
const target: GitPushTarget = {
|
|
remoteName: FORK_REMOTE,
|
|
branchName: TRACKED_BRANCH,
|
|
remoteUrl: forkPath
|
|
}
|
|
await prepareWorktreePushTargetWithExec(execGit, repoPath, target, () => false)
|
|
// Mirrors `configureCreatedWorktreePushTargetWithExec`: local branch tracks the fork
|
|
// remote, so a *bare* `git fetch` (the shape an agent running raw git actually types)
|
|
// resolves to this remote via `branch.<name>.remote` -- not `origin`.
|
|
await git(['checkout', '-qb', 'local-branch', `${FORK_REMOTE}/${TRACKED_BRANCH}`], repoPath)
|
|
await git(
|
|
['branch', '--set-upstream-to', `${FORK_REMOTE}/${TRACKED_BRANCH}`, 'local-branch'],
|
|
repoPath
|
|
)
|
|
|
|
// Contributor deletes the branch after the PR merges/closes (checkout any other
|
|
// branch first -- the fork's actual default-branch name isn't relevant here).
|
|
await git(['checkout', '-q', OTHER_FORK_BRANCHES[0]!], forkPath)
|
|
await git(['branch', '-D', TRACKED_BRANCH], forkPath)
|
|
|
|
// Before this fix's trailing-`*` refspec, this exact command hard-failed with
|
|
// "couldn't find remote ref" -- degrading a common agent/user action into a fatal error.
|
|
await expect(git(['fetch'], repoPath)).resolves.toBeDefined()
|
|
// And `--prune` (Orca's own Fetch action) correctly reclaims the now-dead ref.
|
|
await git(['fetch', '--prune'], repoPath)
|
|
await expect(trackedRefsUnder(FORK_REMOTE)).resolves.toEqual([])
|
|
})
|
|
|
|
it('widens rather than replaces when a second worktree reuses the remote for another branch', async () => {
|
|
await prepareWorktreePushTargetWithExec(
|
|
execGit,
|
|
repoPath,
|
|
{ remoteName: FORK_REMOTE, branchName: TRACKED_BRANCH, remoteUrl: forkPath },
|
|
() => false
|
|
)
|
|
const secondBranch = OTHER_FORK_BRANCHES[0]!
|
|
|
|
await prepareWorktreePushTargetWithExec(
|
|
execGit,
|
|
repoPath,
|
|
{ remoteName: FORK_REMOTE, branchName: secondBranch, remoteUrl: forkPath },
|
|
() => true
|
|
)
|
|
|
|
await git(['fetch', FORK_REMOTE], repoPath)
|
|
const refs = await trackedRefsUnder(FORK_REMOTE)
|
|
expect(refs.sort()).toEqual(
|
|
[`${FORK_REMOTE}/${TRACKED_BRANCH}`, `${FORK_REMOTE}/${secondBranch}`].sort()
|
|
)
|
|
// Only the two branches Orca actually asked for -- not the other ten.
|
|
expect(refs).toHaveLength(2)
|
|
})
|
|
})
|
|
|
|
describe('migrateForkRemoteRefspecsWithExec against the real Git binary', () => {
|
|
it('narrows a pre-existing wide-refspec remote and prunes the strays it already fetched', async () => {
|
|
// Simulate a remote minted before the #17828 fix: bare `remote add`, full fetch.
|
|
const forkBranchCount = (await git(['branch', '--format=%(refname:short)'], forkPath))
|
|
.split(/\r?\n/)
|
|
.filter(Boolean).length
|
|
await git(['remote', 'add', FORK_REMOTE, forkPath], repoPath)
|
|
await git(['fetch', FORK_REMOTE], repoPath)
|
|
const before = await trackedRefsUnder(FORK_REMOTE)
|
|
// Every fork branch imported (tracked branch + unrelated ones + the fork's default branch).
|
|
// Git >= 2.44's `followRemoteHEAD` auto-creates `refs/remotes/<name>/HEAD` on a fetch
|
|
// matching the full wildcard refspec (verified: absent on 2.44, present on 2.55) --
|
|
// excluded here since it isn't a branch this test (or the migration) cares about.
|
|
// `%(refname:short)` shortens the remote's own HEAD symref to just the remote name
|
|
// (no `/HEAD` suffix) -- verified against real git.
|
|
const headRef = FORK_REMOTE
|
|
expect(before.filter((ref) => ref !== headRef).length).toBe(forkBranchCount)
|
|
|
|
const migrated = await migrateForkRemoteRefspecsWithExec(
|
|
repoPath,
|
|
REPO_ID,
|
|
storeOf({
|
|
[worktreeId('/wt/a')]: {
|
|
remoteName: FORK_REMOTE,
|
|
branchName: TRACKED_BRANCH,
|
|
remoteUrl: forkPath,
|
|
remoteCreated: true
|
|
}
|
|
}),
|
|
execGit
|
|
)
|
|
|
|
expect(migrated).toEqual([FORK_REMOTE])
|
|
const refspecs = (await git(['config', '--get-all', `remote.${FORK_REMOTE}.fetch`], repoPath))
|
|
.split(/\r?\n/)
|
|
.filter(Boolean)
|
|
expect(refspecs).toEqual([
|
|
`+refs/heads/${TRACKED_BRANCH}*:refs/remotes/${FORK_REMOTE}/${TRACKED_BRANCH}*`
|
|
])
|
|
// `pruneUntrackedForkRemoteRefs` deliberately never deletes `HEAD` (see its docstring),
|
|
// so a `HEAD` symref this git version auto-created above legitimately survives pruning --
|
|
// assert on the branch refs only, same exclusion as above.
|
|
const after = await trackedRefsUnder(FORK_REMOTE)
|
|
expect(after.filter((ref) => ref !== headRef)).toEqual([`${FORK_REMOTE}/${TRACKED_BRANCH}`])
|
|
})
|
|
|
|
it('clears the refspec of a wide pr-* remote with zero worktree-metadata trace, and prunes its refs', async () => {
|
|
// Simulates a remote whose worktree was removed outside preserve-on-delete: no live
|
|
// worktree, and no surviving metadata entry either -- the store knows nothing about it.
|
|
await git(['remote', 'add', FORK_REMOTE, forkPath], repoPath)
|
|
await git(['fetch', FORK_REMOTE], repoPath)
|
|
const before = await trackedRefsUnder(FORK_REMOTE)
|
|
expect(before.length).toBeGreaterThan(1)
|
|
|
|
const migrated = await migrateForkRemoteRefspecsWithExec(
|
|
repoPath,
|
|
REPO_ID,
|
|
storeOf({}),
|
|
execGit
|
|
)
|
|
|
|
expect(migrated).toEqual([FORK_REMOTE])
|
|
await expect(
|
|
git(['config', '--get-all', `remote.${FORK_REMOTE}.fetch`], repoPath)
|
|
).rejects.toThrow()
|
|
// The remote itself survives (only #17842's reconciliation removes remotes outright),
|
|
// and stays pushable -- just imports nothing on a subsequent plain fetch.
|
|
await expect(
|
|
git(['config', '--get', `remote.${FORK_REMOTE}.url`], repoPath)
|
|
).resolves.toContain(forkPath)
|
|
await git(['fetch', FORK_REMOTE], repoPath)
|
|
await expect(trackedRefsUnder(FORK_REMOTE)).resolves.toEqual([])
|
|
})
|
|
})
|