diff --git a/src/main/git/exact-ref-probe.ts b/src/main/git/exact-ref-probe.ts index 96bb421b8e8..ce89fac3f69 100644 --- a/src/main/git/exact-ref-probe.ts +++ b/src/main/git/exact-ref-probe.ts @@ -157,7 +157,11 @@ export async function probeAnyExactRefBatched( } catch { return { found: false, unknown: true } } - const lines = stdout.split('\n').filter((line) => line.trim().length > 0) + // Trim per line so a CRLF-translating host's `\r` does not become part of the type. + const lines = stdout + .split('\n') + .map((line) => line.trim()) + .filter((line) => line.length > 0) // One line per input, in order; a short read means the batch never answered for the rest. if (lines.length !== safeRefs.length) { return { found: false, unknown: true } diff --git a/src/main/git/remote.test.ts b/src/main/git/remote.test.ts index 11ac2c21264..feb237eb18a 100644 --- a/src/main/git/remote.test.ts +++ b/src/main/git/remote.test.ts @@ -193,6 +193,17 @@ describe('git remote operations', () => { if (args[0] === 'remote' && args[1] === 'get-url' && args[2] === 'pr-pynickle-orca') { return { stdout: 'https://github.com/pynickle/orca.git\n', stderr: '' } } + if (args[0] === 'remote' && args[1] === '-v') { + return { + stdout: [ + 'origin\thttps://github.com/stablyai/orca.git (fetch)', + 'origin\thttps://github.com/stablyai/orca.git (push)', + 'pr-pynickle-orca\thttps://github.com/pynickle/orca.git (fetch)', + 'pr-pynickle-orca\thttps://github.com/pynickle/orca.git (push)' + ].join('\n'), + stderr: '' + } + } if (args[0] === 'remote') { return { stdout: 'origin\npr-pynickle-orca\n', stderr: '' } } @@ -207,6 +218,54 @@ describe('git remote operations', () => { ) }) + // Regression: normalizing a URL-valued push remote used to run `git remote` and then a + // serial `git remote get-url` per remote -- 59 subprocesses on a 58-remote repo. + it('normalizes a URL-valued push remote from one remote table read at 58 remotes', async () => { + const remotes = [ + { name: 'origin', url: 'https://github.com/stablyai/orca.git' }, + ...Array.from({ length: 56 }, (_, index) => ({ + name: `pr-user${index}-orca`, + url: `https://github.com/user${index}/orca.git` + })), + { name: 'pr-pynickle-orca', url: 'https://github.com/pynickle/orca.git' } + ] + gitExecFileAsyncMock.mockImplementation(async (args: string[]) => { + if (args[0] === 'symbolic-ref') { + return { stdout: 'imp/chinese-translation\n', stderr: '' } + } + if (args[0] === 'config' && args.includes('branch.imp/chinese-translation.remote')) { + return { stdout: 'https://github.com/pynickle/orca.git\n', stderr: '' } + } + if (args[0] === 'config' && args.includes('branch.imp/chinese-translation.merge')) { + return { stdout: 'refs/heads/imp/chinese-translation\n', stderr: '' } + } + if (args[0] === 'config') { + throw new Error(`config key is not set: ${args.join(' ')}`) + } + if (args[0] === 'remote' && args[1] === '-v') { + return { + stdout: remotes + .flatMap(({ name, url }) => [`${name}\t${url} (fetch)`, `${name}\t${url} (push)`]) + .join('\n'), + stderr: '' + } + } + if (args[0] === 'remote') { + throw new Error(`unexpected remote scan: ${args.join(' ')}`) + } + return { stdout: '', stderr: '' } + }) + + await gitPush('/repo', false) + + const remoteReads = gitExecFileAsyncMock.mock.calls.filter(([args]) => args[0] === 'remote') + expect(remoteReads.map(([args]) => args)).toEqual([['remote', '-v']]) + expect(gitExecFileAsyncMock).toHaveBeenLastCalledWith( + ['push', '--set-upstream', 'pr-pynickle-orca', 'HEAD:imp/chinese-translation'], + { cwd: '/repo' } + ) + }) + it('uses an explicit push target even when it differs from the local branch name', async () => { gitExecFileAsyncMock .mockResolvedValueOnce({ stdout: '', stderr: '' }) diff --git a/src/main/git/remote.ts b/src/main/git/remote.ts index 2baf3b77137..20cf8415d04 100644 --- a/src/main/git/remote.ts +++ b/src/main/git/remote.ts @@ -4,6 +4,7 @@ import { } from '../../shared/git-remote-error' import { resolveEffectiveGitUpstream } from '../../shared/git-effective-upstream' import { gitRefTargetsBranchOnRemote } from '../../shared/git-remote-branch-name' +import { findGitRemoteNameByFetchUrl } from '../../shared/git-remote-url-index' import type { GitPushTarget } from '../../shared/worktree/types' import type { GitRuntimeOptions } from './git-runtime-options' import { gitOptionsForWorktree } from './git-runtime-options' @@ -84,6 +85,8 @@ type ConfiguredPushRemote = { branchRemote: string | null } +// One `git remote -v` instead of `git remote` plus a serial `git remote get-url` +// per remote; both print the same insteadOf-expanded fetch URL. async function findRemoteNameForUrl( worktreePath: string, remoteUrl: string, @@ -91,30 +94,13 @@ async function findRemoteNameForUrl( ): Promise { try { const { stdout } = await gitExecFileAsync( - ['remote'], + ['remote', '-v'], gitOptionsForWorktree(worktreePath, options) ) - const remotes = stdout - .split(/\r?\n/) - .map((line) => line.trim()) - .filter(Boolean) - for (const remoteName of remotes) { - try { - const { stdout: urlStdout } = await gitExecFileAsync( - ['remote', 'get-url', remoteName], - gitOptionsForWorktree(worktreePath, options) - ) - if (urlStdout.trim() === remoteUrl) { - return remoteName - } - } catch { - // Ignore a remote that disappeared or has no fetch URL. - } - } + return findGitRemoteNameByFetchUrl(stdout, (candidateUrl) => candidateUrl === remoteUrl) } catch { return null } - return null } async function normalizePushRemote( @@ -141,11 +127,17 @@ async function getConfiguredPushRemote( if (!remote) { return null } + const normalizedRemote = await normalizePushRemote(worktreePath, remote, options) + // The two usually name the same URL; resolving it twice reads the remote table twice. + if (!branchRemote) { + return { remote: normalizedRemote, branchRemote: null } + } return { - remote: await normalizePushRemote(worktreePath, remote, options), - branchRemote: branchRemote - ? await normalizePushRemote(worktreePath, branchRemote, options) - : null + remote: normalizedRemote, + branchRemote: + branchRemote === remote + ? normalizedRemote + : await normalizePushRemote(worktreePath, branchRemote, options) } } diff --git a/src/main/git/repo-branch-conflict-batched-probe.test.ts b/src/main/git/repo-branch-conflict-batched-probe.test.ts new file mode 100644 index 00000000000..e6e672c65a2 --- /dev/null +++ b/src/main/git/repo-branch-conflict-batched-probe.test.ts @@ -0,0 +1,102 @@ +// Why: the batched `cat-file --batch-check` conflict probe decides from stdout, so a +// WSL login-shell fallback that prints the distro banner onto that stream desynchronizes +// the one-line-per-ref contract. Every batch then came back undecided and fell through to +// one `show-ref` subprocess per remote -- the cost the batch exists to remove. These tests +// pin the fence request and the resulting subprocess count at 58 remotes. + +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { gitExecFileAsyncMock } = vi.hoisted(() => ({ gitExecFileAsyncMock: vi.fn() })) + +vi.mock('./runner', () => ({ gitExecFileAsync: gitExecFileAsyncMock })) + +import { getBranchConflictKind } from './repo-branch-conflict' + +const REMOTES = Array.from({ length: 58 }, (_, index) => `r${index}`) +const BRANCH = 'user/feature' +const WSL_BANNER = + 'Welcome to Ubuntu 24.04.1 LTS (GNU/Linux 5.15.167.4-microsoft-standard-WSL2 x86_64)\n' + + 'To run a command as administrator (user "root"), use "sudo ".\n' + +type GitExecOptions = { stdin?: string; captureWslLoginShellOutput?: boolean } + +/** + * Stand-in for a WSL-routed runner: the login shell prepends its banner to stdout unless + * the caller asked for the fenced form, which slices the payload back out. + */ +function installLoginShellRunner(): { argv: string[][] } { + const argv: string[][] = [] + gitExecFileAsyncMock.mockImplementation(async (args: string[], options: GitExecOptions = {}) => { + argv.push(args) + if (args[0] === 'rev-parse') { + throw new Error('local branch is absent') + } + if (args[0] === 'remote') { + return { stdout: `${WSL_BANNER}${REMOTES.join('\n')}\n`, stderr: '' } + } + if (args[0] === 'show-ref') { + throw Object.assign(new Error('missing ref'), { code: 1, stderr: '' }) + } + if (args[0] === 'cat-file') { + const payload = `${(options.stdin ?? '') + .split('\n') + .filter(Boolean) + .map((ref) => `${ref} missing`) + .join('\n')}\n` + return { + stdout: options.captureWslLoginShellOutput ? payload : `${WSL_BANNER}${payload}`, + stderr: '' + } + } + throw new Error(`unexpected git command: ${args.join(' ')}`) + }) + return { argv } +} + +function countSubcommand(argv: readonly string[][], subcommand: string): number { + return argv.filter((args) => args[0] === subcommand).length +} + +describe('getBranchConflictKind batched remote probe', () => { + beforeEach(() => { + gitExecFileAsyncMock.mockReset() + }) + + it('asks the WSL login shell to fence the batch payload it parses', async () => { + installLoginShellRunner() + + await getBranchConflictKind('/repo', BRANCH) + + const batchCall = gitExecFileAsyncMock.mock.calls.find(([args]) => args[0] === 'cat-file') + expect(batchCall?.[1]).toMatchObject({ captureWslLoginShellOutput: true }) + }) + + it('answers from one batched subprocess instead of one show-ref per remote', async () => { + const { argv } = installLoginShellRunner() + + await expect(getBranchConflictKind('/repo', BRANCH)).resolves.toBeNull() + + expect(countSubcommand(argv, 'cat-file')).toBe(1) + expect(countSubcommand(argv, 'show-ref')).toBe(0) + }) + + it('still falls back to per-ref probes when the batch itself fails', async () => { + gitExecFileAsyncMock.mockImplementation(async (args: string[]) => { + if (args[0] === 'rev-parse') { + throw new Error('local branch is absent') + } + if (args[0] === 'remote') { + return { stdout: `${REMOTES.join('\n')}\n`, stderr: '' } + } + if (args[0] === 'cat-file') { + throw new Error('cat-file is unavailable on this host') + } + if (args[0] === 'show-ref') { + return { stdout: '', stderr: '' } + } + throw new Error(`unexpected git command: ${args.join(' ')}`) + }) + + await expect(getBranchConflictKind('/repo', BRANCH)).resolves.toBe('remote') + }) +}) diff --git a/src/main/git/repo-branch-conflict.ts b/src/main/git/repo-branch-conflict.ts index c799d3770be..5c6e03b94b0 100644 --- a/src/main/git/repo-branch-conflict.ts +++ b/src/main/git/repo-branch-conflict.ts @@ -147,10 +147,12 @@ export function getBranchConflictKind( const execOptions = gitExecOptions(path, options) const runLocalGit = ( argv: string[], - commandOptions?: ExactRefProbeExecOptions & { stdin?: string } + commandOptions?: ExactRefProbeExecOptions & { stdin?: string }, + captureWslLoginShellOutput = false ): Promise<{ stdout: string }> => gitExecFileAsync(argv, { ...execOptions, + ...(captureWslLoginShellOutput ? { captureWslLoginShellOutput: true } : {}), ...(commandOptions?.maxBuffer === undefined ? {} : { maxBuffer: commandOptions.maxBuffer }), ...(commandOptions?.timeoutMs === undefined ? {} : { timeout: commandOptions.timeoutMs }), ...(commandOptions?.stdin === undefined ? {} : { stdin: commandOptions.stdin }) @@ -160,7 +162,13 @@ export function getBranchConflictKind( branchName, allowedBaseRef, {}, - (argv, commandOptions) => runLocalGit(argv, commandOptions) + // Why fenced: the batch decides from stdout, and a WSL login-shell fallback writes + // the distro's rc/motd banner to that same stream. The extra lines break the + // one-line-per-ref contract, so every batch came back undecided and fell through to + // one `show-ref` subprocess per remote -- the exact cost the batch exists to remove. + // `show-ref --verify --quiet` prints nothing and is read by exit code, so it needs + // no fence; the capture wrapper preserves the payload's exit status either way. + (argv, commandOptions) => runLocalGit(argv, commandOptions, true) ) } diff --git a/src/main/git/upstream.test.ts b/src/main/git/upstream.test.ts index 701808a8a31..b7644ad5543 100644 --- a/src/main/git/upstream.test.ts +++ b/src/main/git/upstream.test.ts @@ -363,6 +363,16 @@ describe('getUpstreamStatus', () => { if (args[0] === 'remote' && args[1] === 'get-url' && args[2] === 'pr-pynickle-orca') { return Promise.resolve({ stdout: 'https://github.com/pynickle/orca.git\n' }) } + if (args[0] === 'remote' && args[1] === '-v') { + return Promise.resolve({ + stdout: [ + 'origin\thttps://github.com/stablyai/orca.git (fetch)', + 'origin\thttps://github.com/stablyai/orca.git (push)', + 'pr-pynickle-orca\thttps://github.com/pynickle/orca.git (fetch)', + 'pr-pynickle-orca\thttps://github.com/pynickle/orca.git (push)' + ].join('\n') + }) + } if (args[0] === 'remote') { return Promise.resolve({ stdout: 'origin\npr-pynickle-orca\n' }) } diff --git a/src/main/ipc/worktree-push-target-reconciliation.ts b/src/main/ipc/worktree-push-target-reconciliation.ts index e59da7bdfe7..b28ae36b6f4 100644 --- a/src/main/ipc/worktree-push-target-reconciliation.ts +++ b/src/main/ipc/worktree-push-target-reconciliation.ts @@ -13,7 +13,7 @@ import { listWorktrees } from '../git/worktree' import type { SshGitProvider } from '../providers/ssh-git-provider' import type { GitPushTarget } from '../../shared/worktree/types' import { WORKTREE_ID_SEPARATOR, worktreeIdComparisonKey } from '../../shared/worktree/id' -import { iterateProcessOutputLines } from '../../shared/process-output-field-scanner' +import { parseGitRemoteFetchUrls } from '../../shared/git-remote-url-index' import { findWorktreeMetaReferencingRemote, hasBranchConfigUsingRemote, @@ -44,26 +44,9 @@ async function listPrRemoteCandidates( } catch { return [] } - const candidates = new Map() - for (const line of iterateProcessOutputLines(stdout)) { - const parsed = parseRemoteVerboseLine(line) - if (parsed?.direction === 'fetch' && isOrcaGeneratedPrRemoteName(parsed.name)) { - candidates.set(parsed.name, parsed.url) - } - } - return [...candidates.entries()].map(([name, url]) => ({ name, url })) -} - -function parseRemoteVerboseLine( - line: string -): { name: string; url: string; direction: 'fetch' | 'push' } | null { - const tabIndex = line.indexOf('\t') - if (tabIndex === -1) { - return null - } - const name = line.slice(0, tabIndex) - const match = /^(.*) \((fetch|push)\)$/.exec(line.slice(tabIndex + 1).trim()) - return match ? { name, url: match[1], direction: match[2] as 'fetch' | 'push' } : null + return [...parseGitRemoteFetchUrls(stdout)] + .filter(([name]) => isOrcaGeneratedPrRemoteName(name)) + .map(([name, url]) => ({ name, url })) } async function shouldReclaimPrRemote( diff --git a/src/main/ipc/worktree-push-target-remote-scan.test.ts b/src/main/ipc/worktree-push-target-remote-scan.test.ts new file mode 100644 index 00000000000..f2392c5e71a --- /dev/null +++ b/src/main/ipc/worktree-push-target-remote-scan.test.ts @@ -0,0 +1,178 @@ +// Why: `findRemoteForUrl` used to run `git remote` and then one serial +// `git remote get-url` per remote. These tests pin both halves of the fix: the +// subprocess count at 58 remotes, and result-for-result parity with the old scan +// across the remote shapes a real repo produces. + +import { describe, expect, it } from 'vitest' +import { parseGitHubOwnerRepo } from '../github/gh-utils' +import { findRemoteForUrl } from './worktree-push-target-setup' +import type { GitRemoteExec } from './worktree-push-target-cleanup' + +const SSH_FORK = 'git@github.com:contributor/orca.git' +const HTTPS_FORK = 'https://github.com/contributor/orca.git' +const GITLAB_FORK = 'https://gitlab.com/contributor/orca.git' +const UPSTREAM = 'https://github.com/stablyai/orca.git' + +type RemoteRow = { name: string; fetchUrl: string; pushUrl?: string } + +type CountingExec = GitRemoteExec & { spawns: string[][] } + +function makeExec(remotes: readonly RemoteRow[]): CountingExec { + const spawns: string[][] = [] + const exec: GitRemoteExec = async (args: string[]) => { + spawns.push(args) + if (args[0] === 'remote' && args.length === 1) { + return { stdout: `${remotes.map((remote) => remote.name).join('\n')}\n` } + } + if (args[0] === 'remote' && args[1] === '-v') { + return { + stdout: remotes + .flatMap((remote) => [ + `${remote.name}\t${remote.fetchUrl} (fetch)`, + `${remote.name}\t${remote.pushUrl ?? remote.fetchUrl} (push)` + ]) + .join('\n') + } + } + if (args[0] === 'remote' && args[1] === 'get-url') { + const match = remotes.find((remote) => remote.name === args[2]) + if (!match) { + throw new Error(`No such remote ${args[2]}`) + } + return { stdout: `${match.fetchUrl}\n` } + } + throw new Error(`unexpected git command: ${args.join(' ')}`) + } + return Object.assign(exec, { spawns }) +} + +/** The pre-fix scan, kept as the oracle the batched form must reproduce exactly. */ +async function findRemoteForUrlPerRemote( + execGit: GitRemoteExec, + repoPath: string, + remoteUrl: string +): Promise { + 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 +} + +const fiftyEightRemotes: RemoteRow[] = [ + { name: 'origin', fetchUrl: UPSTREAM }, + ...Array.from({ length: 56 }, (_, index) => ({ + name: `pr-user${index}-orca`, + fetchUrl: `https://github.com/user${index}/orca.git` + })), + { name: 'pr-contributor-orca', fetchUrl: SSH_FORK } +] + +const matrix: { name: string; remotes: RemoteRow[]; lookupUrl: string }[] = [ + { name: 'no remotes', remotes: [], lookupUrl: SSH_FORK }, + { + name: 'one matching remote', + remotes: [{ name: 'origin', fetchUrl: SSH_FORK }], + lookupUrl: SSH_FORK + }, + { + name: 'one non-matching remote', + remotes: [{ name: 'origin', fetchUrl: UPSTREAM }], + lookupUrl: SSH_FORK + }, + { name: '58 remotes, match last', remotes: fiftyEightRemotes, lookupUrl: SSH_FORK }, + { + name: '58 remotes, no match', + remotes: fiftyEightRemotes, + lookupUrl: 'https://github.com/nobody/other.git' + }, + { + name: 'duplicate URLs on two remotes', + remotes: [ + { name: 'origin', fetchUrl: UPSTREAM }, + { name: 'fork-a', fetchUrl: SSH_FORK }, + { name: 'fork-b', fetchUrl: SSH_FORK } + ], + lookupUrl: SSH_FORK + }, + { + name: 'fetch and push URLs differ', + remotes: [{ name: 'split', fetchUrl: SSH_FORK, pushUrl: HTTPS_FORK }], + lookupUrl: SSH_FORK + }, + { + name: 'SSH-form lookup against an HTTPS-form remote', + remotes: [ + { name: 'origin', fetchUrl: UPSTREAM }, + { name: 'fork', fetchUrl: HTTPS_FORK } + ], + lookupUrl: SSH_FORK + }, + { + name: 'HTTPS-form lookup against an SSH-form remote', + remotes: [ + { name: 'origin', fetchUrl: UPSTREAM }, + { name: 'fork', fetchUrl: SSH_FORK } + ], + lookupUrl: HTTPS_FORK + }, + { + name: 'non-GitHub provider matches only on the exact URL', + remotes: [{ name: 'gitlab-fork', fetchUrl: GITLAB_FORK }], + lookupUrl: GITLAB_FORK + }, + { + name: 'non-GitHub provider with a different host does not match', + remotes: [{ name: 'gitlab-fork', fetchUrl: GITLAB_FORK }], + lookupUrl: 'https://bitbucket.org/contributor/orca.git' + } +] + +describe('findRemoteForUrl', () => { + it.each(matrix)('matches the per-remote scan for $name', async ({ remotes, lookupUrl }) => { + const expected = await findRemoteForUrlPerRemote(makeExec(remotes), '/repo', lookupUrl) + await expect(findRemoteForUrl(makeExec(remotes), '/repo', lookupUrl)).resolves.toBe(expected) + }) + + it('answers from one subprocess at 58 remotes instead of one per remote', async () => { + const legacyExec = makeExec(fiftyEightRemotes) + await findRemoteForUrlPerRemote(legacyExec, '/repo', 'https://github.com/nobody/other.git') + expect(legacyExec.spawns).toHaveLength(fiftyEightRemotes.length + 1) + + const exec = makeExec(fiftyEightRemotes) + await findRemoteForUrl(exec, '/repo', 'https://github.com/nobody/other.git') + expect(exec.spawns).toEqual([['remote', '-v']]) + }) + + it('returns null when the remote table cannot be read', async () => { + const failing: GitRemoteExec = async () => { + throw new Error('not a git repository') + } + await expect(findRemoteForUrl(failing, '/repo', SSH_FORK)).resolves.toBeNull() + }) +}) diff --git a/src/main/ipc/worktree-push-target-setup.test.ts b/src/main/ipc/worktree-push-target-setup.test.ts index 2d9670c2f9b..ef1e44aa923 100644 --- a/src/main/ipc/worktree-push-target-setup.test.ts +++ b/src/main/ipc/worktree-push-target-setup.test.ts @@ -16,6 +16,13 @@ const REPO = '/repo-root' const FORK_SSH = 'git@github.com:contributor/orca.git' const FORK_HTTPS = 'https://github.com/contributor/orca.git' +/** Real `git remote -v` shape: a fetch row and a push row per remote, tab-separated. */ +export function renderRemoteVerbose(remotes: Record): string { + return Object.entries(remotes) + .flatMap(([name, url]) => [`${name}\t${url} (fetch)`, `${name}\t${url} (push)`]) + .join('\n') +} + // A stateful fake git: `remotes` maps name -> url. `remote add` mutates it so // later lookups see the new remote, matching real git behavior. Defaults // `symbolic-ref --short HEAD` to a real branch name, since a worktree's HEAD @@ -31,6 +38,9 @@ function makeRepoExec( if (args[0] === 'remote' && args.length === 1) { return { stdout: Object.keys(remotes).join('\n'), stderr: '' } } + if (args[0] === 'remote' && args[1] === '-v' && args.length === 2) { + return { stdout: renderRemoteVerbose(remotes), stderr: '' } + } if (args[0] === 'remote' && args[1] === 'get-url') { const url = remotes[args[2]!] if (!url) { diff --git a/src/main/ipc/worktree-push-target-setup.ts b/src/main/ipc/worktree-push-target-setup.ts index 59ef4c8fea5..c4a82d94933 100644 --- a/src/main/ipc/worktree-push-target-setup.ts +++ b/src/main/ipc/worktree-push-target-setup.ts @@ -5,53 +5,33 @@ // repo. The store-aware ownership decision stays with the caller via a predicate. import type { GitPushTarget } from '../../shared/worktree/types' -import { parseGitHubOwnerRepo } from '../github/gh-utils' -import type { GitRemoteExec } from './worktree-push-target-cleanup' +import { findGitRemoteNameByFetchUrl } from '../../shared/git-remote-url-index' +import { sameGitHubRemoteUrl, type GitRemoteExec } from './worktree-push-target-cleanup' import { buildNarrowForkFetchRefspec, ensureRemoteTracksBranchNarrowly } from '../git/fork-remote-refspec' +// 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). export async function findRemoteForUrl( execGit: GitRemoteExec, repoPath: string, remoteUrl: string ): Promise { - 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. - } - } + const { stdout } = await execGit(['remote', '-v'], repoPath) + return findGitRemoteNameByFetchUrl(stdout, (candidateUrl) => + sameGitHubRemoteUrl(candidateUrl, remoteUrl) + ) } catch { return null } - return null } // O(1) probe used before materializing on demand (push/pull/fetch/fast-forward): -// a single `remote get-url ` avoids the O(remotes) `findRemoteForUrl` scan -// once a fork remote already exists under its expected name (#17828). +// a single `remote get-url ` skips the whole-remote-table read once a fork +// remote already exists under its expected name (#17828). export async function remoteAlreadyMatchesUrl( execGit: GitRemoteExec, repoPath: string, @@ -60,18 +40,7 @@ export async function remoteAlreadyMatchesUrl( ): Promise { try { const { stdout } = await execGit(['remote', 'get-url', remoteName], repoPath) - const candidateUrl = stdout.trim() - if (candidateUrl === remoteUrl) { - return true - } - const target = parseGitHubOwnerRepo(remoteUrl) - const candidate = parseGitHubOwnerRepo(candidateUrl) - return Boolean( - target && - candidate && - target.owner.toLowerCase() === candidate.owner.toLowerCase() && - target.repo.toLowerCase() === candidate.repo.toLowerCase() - ) + return sameGitHubRemoteUrl(stdout.trim(), remoteUrl) } catch { return false } diff --git a/src/main/ipc/worktree-remote-push-target-materialization.test.ts b/src/main/ipc/worktree-remote-push-target-materialization.test.ts index 68ed98ab35c..695baa10dec 100644 --- a/src/main/ipc/worktree-remote-push-target-materialization.test.ts +++ b/src/main/ipc/worktree-remote-push-target-materialization.test.ts @@ -553,6 +553,17 @@ describe('materializeWorktreePushTargetRemoteSsh', () => { } throw new Error('No such remote') } + if (args[0] === 'remote' && args[1] === '-v') { + return { + stdout: [ + 'origin\thttps://github.com/stablyai/orca.git (fetch)', + 'origin\thttps://github.com/stablyai/orca.git (push)', + `${SIBLING_REMOTE}\t${FORK_URL} (fetch)`, + `${SIBLING_REMOTE}\t${FORK_URL} (push)` + ].join('\n'), + stderr: '' + } + } if (args[0] === 'remote' && args.length === 1) { return { stdout: `origin\n${SIBLING_REMOTE}\n`, stderr: '' } } diff --git a/src/relay/git-handler-push-target.test.ts b/src/relay/git-handler-push-target.test.ts index c040a645a16..b6fe5e96ad0 100644 --- a/src/relay/git-handler-push-target.test.ts +++ b/src/relay/git-handler-push-target.test.ts @@ -46,6 +46,17 @@ function gitForConfig(config: { } return { stdout: `${config.base ?? ''}\n`, stderr: '' } } + if (args[0] === 'remote' && args[1] === '-v') { + return { + stdout: (config.remotes ?? []) + .flatMap((name) => { + const url = config.remoteUrls?.[name] ?? '' + return [`${name}\t${url} (fetch)`, `${name}\t${url} (push)`] + }) + .join('\n'), + stderr: '' + } + } if (args[0] === 'remote' && args.length === 1) { return { stdout: `${config.remotes?.join('\n') ?? ''}\n`, stderr: '' } } diff --git a/src/relay/git-handler-push-target.ts b/src/relay/git-handler-push-target.ts index 40765491c56..d81111d45d3 100644 --- a/src/relay/git-handler-push-target.ts +++ b/src/relay/git-handler-push-target.ts @@ -1,5 +1,6 @@ import { assertGitPushTargetShape } from '../shared/git-push-target-validation' import { gitRefTargetsBranchOnRemote } from '../shared/git-remote-branch-name' +import { findGitRemoteNameByFetchUrl } from '../shared/git-remote-url-index' import type { GitPushTarget } from '../shared/worktree/types' type RelayGit = (args: string[], cwd: string) => Promise<{ stdout: string; stderr: string }> @@ -67,31 +68,19 @@ type ConfiguredPushRemote = { branchRemote: string | null } +// Host-side twin of `src/main/git/remote.ts`: one `git remote -v` instead of +// `git remote` plus a serial `git remote get-url` per remote. async function findRemoteNameForUrl( git: RelayGit, worktreePath: string, remoteUrl: string ): Promise { try { - const { stdout } = await git(['remote'], worktreePath) - const remotes = stdout - .split(/\r?\n/) - .map((line) => line.trim()) - .filter(Boolean) - for (const remoteName of remotes) { - try { - const { stdout: urlStdout } = await git(['remote', 'get-url', remoteName], worktreePath) - if (urlStdout.trim() === remoteUrl) { - return remoteName - } - } catch { - // Ignore a remote that disappeared or has no fetch URL. - } - } + const { stdout } = await git(['remote', '-v'], worktreePath) + return findGitRemoteNameByFetchUrl(stdout, (candidateUrl) => candidateUrl === remoteUrl) } catch { return null } - return null } async function normalizePushRemote( @@ -120,9 +109,17 @@ async function getConfiguredPushRemote( if (!remote) { return null } + const normalizedRemote = await normalizePushRemote(git, worktreePath, remote) + // The two usually name the same URL; resolving it twice reads the remote table twice. + if (!branchRemote) { + return { remote: normalizedRemote, branchRemote: null } + } return { - remote: await normalizePushRemote(git, worktreePath, remote), - branchRemote: branchRemote ? await normalizePushRemote(git, worktreePath, branchRemote) : null + remote: normalizedRemote, + branchRemote: + branchRemote === remote + ? normalizedRemote + : await normalizePushRemote(git, worktreePath, branchRemote) } } diff --git a/src/shared/git-binary-compatibility.test.ts b/src/shared/git-binary-compatibility.test.ts index a61e64464da..5ad37398d14 100644 --- a/src/shared/git-binary-compatibility.test.ts +++ b/src/shared/git-binary-compatibility.test.ts @@ -15,6 +15,7 @@ import { isUnsupportedWorktreeListZError } from './git-worktree-command-capabilities' import { gitCredentialPromptGuardEnv } from './git-credential-prompt-env' +import { parseGitRemoteFetchUrls } from './git-remote-url-index' import { GIT_HISTORY_COMMIT_FORMAT, parseGitHistoryLog } from './git-history-log-parser' import { githubPullRequestHeadLocalRef, @@ -214,6 +215,41 @@ describeBinaryCompatibility('real Git binary compatibility', () => { await expect(runGit(['merge-base', '--end-of-options', head, unrelated])).rejects.toBeDefined() }) + // Why pin this: Orca answers "which remote has this URL" from one `git remote -v` + // instead of one `git remote get-url` per remote. That is only equivalent if both + // commands report the same URL — the insteadOf-expanded first `remote..url`, + // which a raw config read does not produce — on every supported Git. + it('reports the same fetch URL from remote -v as from remote get-url', async () => { + await runGit(['config', 'url.git@example.invalid:.insteadOf', 'https://example.invalid/']) + await runGit(['remote', 'add', 'compat-single', 'https://example.invalid/a/repo.git']) + await runGit(['remote', 'add', 'compat-multi', 'https://example.invalid/b/repo.git']) + await runGit([ + 'config', + '--add', + 'remote.compat-multi.url', + 'https://example.invalid/b2/repo.git' + ]) + await runGit([ + 'config', + 'remote.compat-multi.pushurl', + 'https://push.example.invalid/b/repo.git' + ]) + try { + const fetchUrls = parseGitRemoteFetchUrls((await runGit(['remote', '-v'])).stdout) + for (const name of ['compat-single', 'compat-multi']) { + const getUrl = (await runGit(['remote', 'get-url', name])).stdout.trim() + expect(fetchUrls.get(name)).toBe(getUrl) + } + expect(fetchUrls.get('compat-single')).toBe('git@example.invalid:a/repo.git') + // A `pushurl` must not displace the fetch URL the scan compares against. + expect(fetchUrls.get('compat-multi')).toBe('git@example.invalid:b/repo.git') + } finally { + await runGit(['remote', 'remove', 'compat-single']) + await runGit(['remote', 'remove', 'compat-multi']) + await runGit(['config', '--unset-all', 'url.git@example.invalid:.insteadOf']) + } + }) + it('recognizes ref and merge-tree compatibility boundaries', async () => { const fetchHeadPath = join(repoPath, '.git', 'FETCH_HEAD') await writeFile(fetchHeadPath, 'sentinel\n') diff --git a/src/shared/git-configured-branch-target.test.ts b/src/shared/git-configured-branch-target.test.ts new file mode 100644 index 00000000000..a599a53bbb8 --- /dev/null +++ b/src/shared/git-configured-branch-target.test.ts @@ -0,0 +1,175 @@ +// Why: resolving a URL-valued `branch..remote` (or `remote.pushDefault`) to a +// remote name used to cost `git remote` plus one serial `git remote get-url` per remote. +// `hasConfiguredBranchPushTarget` resolves up to two of them, so a 58-remote repo paid +// up to 118 subprocesses for one question. These tests pin the count and result parity. + +import { describe, expect, it } from 'vitest' +import { + getConfiguredBranchRemoteUpstream, + hasConfiguredBranchPushTarget +} from './git-configured-branch-target' + +const BRANCH = 'imp/translation' +const FORK_URL = 'https://github.com/contributor/orca.git' +const UPSTREAM_URL = 'https://github.com/stablyai/orca.git' + +type RemoteRow = { name: string; fetchUrl: string; pushUrl?: string } + +type Fixture = { + remotes: readonly RemoteRow[] + config: Readonly> +} + +function makeRunner(fixture: Fixture): { + runGit: (args: string[]) => Promise<{ stdout: string }> + spawns: string[][] +} { + const spawns: string[][] = [] + const runGit = async (args: string[]): Promise<{ stdout: string }> => { + spawns.push(args) + if (args[0] === 'config' && args[1] === '--get') { + const value = fixture.config[args[2]] + if (value === undefined) { + throw Object.assign(new Error('config key is not set'), { code: 1 }) + } + return { stdout: `${value}\n` } + } + if (args[0] === 'remote' && args[1] === '-v') { + return { + stdout: fixture.remotes + .flatMap((remote) => [ + `${remote.name}\t${remote.fetchUrl} (fetch)`, + `${remote.name}\t${remote.pushUrl ?? remote.fetchUrl} (push)` + ]) + .join('\n') + } + } + if (args[0] === 'remote' && args.length === 1) { + return { stdout: `${fixture.remotes.map((remote) => remote.name).join('\n')}\n` } + } + if (args[0] === 'remote' && args[1] === 'get-url') { + const match = fixture.remotes.find((remote) => remote.name === args[2]) + if (!match) { + throw new Error(`No such remote ${args[2]}`) + } + return { stdout: `${match.fetchUrl}\n` } + } + throw new Error(`unexpected git command: ${args.join(' ')}`) + } + return { runGit, spawns } +} + +const fiftyEightRemotes: RemoteRow[] = [ + { name: 'origin', fetchUrl: UPSTREAM_URL }, + ...Array.from({ length: 56 }, (_, index) => ({ + name: `pr-user${index}-orca`, + fetchUrl: `https://github.com/user${index}/orca.git` + })), + { name: 'pr-contributor-orca', fetchUrl: FORK_URL } +] + +describe('hasConfiguredBranchPushTarget', () => { + it('resolves both URL-valued remotes from one remote table read at 58 remotes', async () => { + const { runGit, spawns } = makeRunner({ + remotes: fiftyEightRemotes, + config: { + [`branch.${BRANCH}.pushRemote`]: FORK_URL, + [`branch.${BRANCH}.remote`]: FORK_URL, + [`branch.${BRANCH}.merge`]: `refs/heads/${BRANCH}` + } + }) + + await expect(hasConfiguredBranchPushTarget(runGit, BRANCH)).resolves.toBe(true) + + // Both the push remote and the branch remote name the same URL, so one table read answers. + expect(spawns.filter((args) => args[0] === 'remote')).toEqual([['remote', '-v']]) + expect(spawns.filter((args) => args[1] === 'get-url')).toEqual([]) + }) + + it('keeps the not-set case false when no remote is configured', async () => { + const { runGit } = makeRunner({ remotes: fiftyEightRemotes, config: {} }) + await expect(hasConfiguredBranchPushTarget(runGit, BRANCH)).resolves.toBe(false) + }) + + it('keeps the URL itself as the remote name when nothing matches', async () => { + const { runGit } = makeRunner({ + remotes: [{ name: 'origin', fetchUrl: UPSTREAM_URL }], + config: { + [`branch.${BRANCH}.pushRemote`]: FORK_URL, + [`branch.${BRANCH}.remote`]: FORK_URL, + [`branch.${BRANCH}.merge`]: 'refs/heads/other' + } + }) + // Unchanged no-match fallback: both remotes stay the raw URL, so they still agree + // and the differently named merge branch is still pushable. + await expect(hasConfiguredBranchPushTarget(runGit, BRANCH)).resolves.toBe(true) + }) +}) + +describe('getConfiguredBranchRemoteUpstream', () => { + const remoteTrackingRefExists = async (): Promise => true + + it('picks the first remote holding a duplicated URL', async () => { + const { runGit, spawns } = makeRunner({ + remotes: [ + { name: 'origin', fetchUrl: UPSTREAM_URL }, + { name: 'fork-a', fetchUrl: FORK_URL }, + { name: 'fork-b', fetchUrl: FORK_URL } + ], + config: { + [`branch.${BRANCH}.remote`]: FORK_URL, + [`branch.${BRANCH}.merge`]: `refs/heads/${BRANCH}` + } + }) + + await expect( + getConfiguredBranchRemoteUpstream(runGit, BRANCH, remoteTrackingRefExists) + ).resolves.toEqual({ + upstreamName: `fork-a/${BRANCH}`, + remoteName: 'fork-a', + branchName: BRANCH, + isConfiguredUpstream: false + }) + expect(spawns.filter((args) => args[0] === 'remote')).toEqual([['remote', '-v']]) + }) + + it('ignores a push URL when fetch and push differ', async () => { + const { runGit } = makeRunner({ + remotes: [{ name: 'split', fetchUrl: UPSTREAM_URL, pushUrl: FORK_URL }], + config: { + [`branch.${BRANCH}.remote`]: FORK_URL, + [`branch.${BRANCH}.merge`]: `refs/heads/${BRANCH}` + } + }) + await expect( + getConfiguredBranchRemoteUpstream(runGit, BRANCH, remoteTrackingRefExists) + ).resolves.toBeNull() + }) + + it('returns null with no remotes at all', async () => { + const { runGit } = makeRunner({ + remotes: [], + config: { + [`branch.${BRANCH}.remote`]: FORK_URL, + [`branch.${BRANCH}.merge`]: `refs/heads/${BRANCH}` + } + }) + await expect( + getConfiguredBranchRemoteUpstream(runGit, BRANCH, remoteTrackingRefExists) + ).resolves.toBeNull() + }) + + it('keeps a plain named remote untouched', async () => { + const { runGit, spawns } = makeRunner({ + remotes: fiftyEightRemotes, + config: { + [`branch.${BRANCH}.remote`]: 'origin', + [`branch.${BRANCH}.merge`]: `refs/heads/${BRANCH}` + } + }) + await expect( + getConfiguredBranchRemoteUpstream(runGit, BRANCH, remoteTrackingRefExists) + ).resolves.toMatchObject({ remoteName: 'origin' }) + expect(spawns.filter((args) => args[0] === 'remote')).toEqual([]) + }) +}) diff --git a/src/shared/git-configured-branch-target.ts b/src/shared/git-configured-branch-target.ts index 5c28f023814..9faca91dce7 100644 --- a/src/shared/git-configured-branch-target.ts +++ b/src/shared/git-configured-branch-target.ts @@ -1,4 +1,5 @@ import { gitRefTargetsBranchOnRemote } from './git-remote-branch-name' +import { findGitRemoteNameByFetchUrl } from './git-remote-url-index' type GitCommandRunner = (args: string[]) => Promise<{ stdout: string }> @@ -25,30 +26,18 @@ function isUrlValuedRemote(remote: string): boolean { return /^[A-Za-z][A-Za-z0-9+.-]*:\/\//.test(remote) || /^[^@/:]+@[^:]+:.+/.test(remote) } +// `hasConfiguredBranchPushTarget` resolves up to two URL-valued remotes, so the old +// per-remote `get-url` scan cost up to 2 x (1 + remotes) subprocesses per call. async function findRemoteNameForUrl( runGit: GitCommandRunner, remoteUrl: string ): Promise { try { - const { stdout } = await runGit(['remote']) - const remotes = stdout - .split(/\r?\n/) - .map((line) => line.trim()) - .filter(Boolean) - for (const remoteName of remotes) { - try { - const { stdout: urlStdout } = await runGit(['remote', 'get-url', remoteName]) - if (urlStdout.trim() === remoteUrl) { - return remoteName - } - } catch { - // Ignore a remote that disappeared or has no fetch URL. - } - } + const { stdout } = await runGit(['remote', '-v']) + return findGitRemoteNameByFetchUrl(stdout, (candidateUrl) => candidateUrl === remoteUrl) } catch { return null } - return null } export async function getConfiguredBranchRemoteUpstream( @@ -101,11 +90,14 @@ export async function hasConfiguredBranchPushTarget( const pushRemoteName = isUrlValuedRemote(remote) ? ((await findRemoteNameForUrl(runGit, remote)) ?? remote) : remote - const branchRemoteName = branchRemote - ? isUrlValuedRemote(branchRemote) - ? ((await findRemoteNameForUrl(runGit, branchRemote)) ?? branchRemote) - : branchRemote - : null + // The two usually name the same URL; resolving it twice reads the remote table twice. + const branchRemoteName = !branchRemote + ? null + : branchRemote === remote + ? pushRemoteName + : isUrlValuedRemote(branchRemote) + ? ((await findRemoteNameForUrl(runGit, branchRemote)) ?? branchRemote) + : branchRemote if (gitRefTargetsBranchOnRemote(baseRef, pushRemoteName, branchName)) { return false } diff --git a/src/shared/git-remote-url-index.test.ts b/src/shared/git-remote-url-index.test.ts new file mode 100644 index 00000000000..e798e136ae4 --- /dev/null +++ b/src/shared/git-remote-url-index.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, it } from 'vitest' +import { + findGitRemoteNameByFetchUrl, + parseGitRemoteFetchUrls, + parseGitRemoteVerboseLine +} from './git-remote-url-index' + +const SSH_URL = 'git@github.com:contributor/orca.git' +const HTTPS_URL = 'https://github.com/contributor/orca.git' + +function verbose(rows: readonly (readonly [string, string])[]): string { + return rows.map(([name, url]) => `${name}\t${url}`).join('\n') +} + +describe('parseGitRemoteVerboseLine', () => { + it('reads the name, URL and direction', () => { + expect(parseGitRemoteVerboseLine(`origin\t${HTTPS_URL} (fetch)`)).toEqual({ + name: 'origin', + url: HTTPS_URL, + direction: 'fetch' + }) + }) + + it('keeps a URL that itself contains spaces and parentheses', () => { + const url = '/tmp/my repo (mirror)' + expect(parseGitRemoteVerboseLine(`local\t${url} (push)`)).toEqual({ + name: 'local', + url, + direction: 'push' + }) + }) + + it('rejects the URL-less row git prints for a pushurl-only remote', () => { + expect(parseGitRemoteVerboseLine('pushonly\t')).toBeNull() + expect(parseGitRemoteVerboseLine('not a remote row')).toBeNull() + }) +}) + +describe('parseGitRemoteFetchUrls', () => { + it('returns nothing for a repo with no remotes', () => { + expect([...parseGitRemoteFetchUrls('')]).toEqual([]) + }) + + it('keeps only fetch rows, in git remote order', () => { + const stdout = verbose([ + ['a', `${SSH_URL} (fetch)`], + ['a', `${SSH_URL} (push)`], + ['b', `${HTTPS_URL} (fetch)`], + ['b', 'https://github.com/contributor/other.git (push)'] + ]) + expect([...parseGitRemoteFetchUrls(stdout)]).toEqual([ + ['a', SSH_URL], + ['b', HTTPS_URL] + ]) + }) + + it('parses CRLF output', () => { + const stdout = `a\t${SSH_URL} (fetch)\r\na\t${SSH_URL} (push)\r\n` + expect([...parseGitRemoteFetchUrls(stdout)]).toEqual([['a', SSH_URL]]) + }) + + it('takes the first URL of a multi-URL remote, matching remote get-url', () => { + const stdout = verbose([ + ['multi', `${SSH_URL} (fetch)`], + ['multi', `${SSH_URL} (push)`], + ['multi', `${HTTPS_URL} (push)`] + ]) + expect(parseGitRemoteFetchUrls(stdout).get('multi')).toBe(SSH_URL) + }) + + it('scales to 58 remotes without losing order', () => { + const rows = Array.from({ length: 58 }, (_, index) => [ + `r${index}`, + `https://example.com/o${index}/repo.git` + ]) + const stdout = rows + .flatMap(([name, url]) => [`${name}\t${url} (fetch)`, `${name}\t${url} (push)`]) + .join('\n') + const parsed = [...parseGitRemoteFetchUrls(stdout)] + expect(parsed).toHaveLength(58) + expect(parsed[0]).toEqual(['r0', 'https://example.com/o0/repo.git']) + expect(parsed[57]).toEqual(['r57', 'https://example.com/o57/repo.git']) + }) +}) + +describe('findGitRemoteNameByFetchUrl', () => { + const stdout = verbose([ + ['origin', 'https://github.com/stablyai/orca.git (fetch)'], + ['origin', 'https://github.com/stablyai/orca.git (push)'], + ['first-fork', `${SSH_URL} (fetch)`], + ['first-fork', `${SSH_URL} (push)`], + ['second-fork', `${SSH_URL} (fetch)`], + ['second-fork', `${SSH_URL} (push)`] + ]) + + it('returns the first remote holding a duplicated URL', () => { + expect(findGitRemoteNameByFetchUrl(stdout, (url) => url === SSH_URL)).toBe('first-fork') + }) + + it('returns null when nothing matches', () => { + expect(findGitRemoteNameByFetchUrl(stdout, (url) => url === HTTPS_URL)).toBeNull() + }) + + it('ignores push URLs when fetch and push differ', () => { + const split = verbose([ + ['split', `${SSH_URL} (fetch)`], + ['split', `${HTTPS_URL} (push)`] + ]) + expect(findGitRemoteNameByFetchUrl(split, (url) => url === SSH_URL)).toBe('split') + expect(findGitRemoteNameByFetchUrl(split, (url) => url === HTTPS_URL)).toBeNull() + }) +}) diff --git a/src/shared/git-remote-url-index.ts b/src/shared/git-remote-url-index.ts new file mode 100644 index 00000000000..ec4f2b4a509 --- /dev/null +++ b/src/shared/git-remote-url-index.ts @@ -0,0 +1,64 @@ +// Why: "which remote has this URL?" was answered with one `git remote get-url` +// subprocess per remote, awaited serially -- 58 spawns on a repo with 58 remotes, +// on every push-target resolution. `git remote -v` answers for every remote from +// one child. +// +// `remote -v` is the faithful one-command form, not `config --get-regexp '^remote\.'`: +// both `remote -v` and `remote get-url` print the URL *after* `url..insteadOf` +// expansion and pick the first of several `remote..url` values, while raw config +// reads return the unexpanded value and the last of the multiple values. +// +// `remote -v` also predates `remote get-url` (2.7), so this lowers rather than raises +// the Git floor and needs no capability gate. + +import { iterateProcessOutputLines } from './process-output-field-scanner' + +export type GitRemoteVerboseEntry = { + name: string + url: string + direction: 'fetch' | 'push' +} + +// Greedy prefix so a URL containing spaces or parentheses keeps them. +const REMOTE_VERBOSE_URL_PATTERN = /^(.*) \((fetch|push)\)$/ + +/** Parse one `\t (fetch|push)` row. */ +export function parseGitRemoteVerboseLine(line: string): GitRemoteVerboseEntry | null { + const tabIndex = line.indexOf('\t') + if (tabIndex === -1) { + return null + } + const name = line.slice(0, tabIndex) + const match = REMOTE_VERBOSE_URL_PATTERN.exec(line.slice(tabIndex + 1).trim()) + return match ? { name, url: match[1], direction: match[2] as 'fetch' | 'push' } : null +} + +/** + * Fetch URL per remote in `git remote` order -- the value `git remote get-url ` + * prints. A remote configured with only a `pushurl` has no fetch row and is absent + * here; `get-url` echoed the remote's own name for it, which no caller can match. + */ +export function parseGitRemoteFetchUrls(stdout: string): Map { + const fetchUrls = new Map() + for (const line of iterateProcessOutputLines(stdout)) { + const parsed = parseGitRemoteVerboseLine(line) + // First wins: `get-url` without `--all` prints the first `remote..url`. + if (parsed?.direction === 'fetch' && !fetchUrls.has(parsed.name)) { + fetchUrls.set(parsed.name, parsed.url) + } + } + return fetchUrls +} + +/** First remote whose fetch URL matches, in the order the per-remote scan visited them. */ +export function findGitRemoteNameByFetchUrl( + stdout: string, + matchesUrl: (url: string) => boolean +): string | null { + for (const [name, url] of parseGitRemoteFetchUrls(stdout)) { + if (matchesUrl(url)) { + return name + } + } + return null +}