From 90d3d7a0e5aa8bcc510b18304f1446d2f3b7bd55 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 29 Jun 2026 15:25:13 -0700 Subject: [PATCH] Reduce CPU from source-control git polling on Windows (#6736) Co-authored-by: Orca --- .../status-upstream-negative-cache.test.ts | 26 ++ .../git/status-upstream-probe-churn.test.ts | 85 +++++-- src/main/git/status.ts | 6 +- ...git-status-upstream-negative-cache.test.ts | 66 +++++ .../git-status-upstream-negative-cache.ts | 4 +- src/shared/git-config-snapshot-runner.test.ts | 227 ++++++++++++++++++ src/shared/git-config-snapshot-runner.ts | 103 ++++++++ 7 files changed, 501 insertions(+), 16 deletions(-) create mode 100644 src/shared/git-config-snapshot-runner.test.ts create mode 100644 src/shared/git-config-snapshot-runner.ts diff --git a/src/main/git/status-upstream-negative-cache.test.ts b/src/main/git/status-upstream-negative-cache.test.ts index ce6b43986..263842810 100644 --- a/src/main/git/status-upstream-negative-cache.test.ts +++ b/src/main/git/status-upstream-negative-cache.test.ts @@ -30,6 +30,14 @@ vi.mock('fs', () => ({ existsSync: existsSyncMock })) +function isConfigListSnapshotCommand(args: string[]): boolean { + return args[0] === 'config' && args[1] === '--list' && args[2] === '-z' +} + +function emptyGitConfigSnapshot(): { stdout: string } { + return { stdout: 'core.repositoryformatversion\n0\0' } +} + import { clearEffectiveUpstreamNegativeStatusCache, clearEffectiveUpstreamStatusCacheForTests, @@ -62,6 +70,9 @@ describe('local upstream negative cache', () => { if (args[0] === 'rev-parse' && args.includes('HEAD@{u}')) { throw new Error('fatal: no upstream configured for branch feature') } + if (isConfigListSnapshotCommand(args)) { + return emptyGitConfigSnapshot() + } if (args[0] === 'rev-parse' && args.includes('refs/remotes/origin/feature')) { if (originBranchExists) { return { stdout: 'abc123\n' } @@ -106,6 +117,9 @@ describe('local upstream negative cache', () => { if (args[0] === 'rev-parse' && args.includes('HEAD@{u}')) { throw new Error('fatal: no upstream configured for branch feature') } + if (isConfigListSnapshotCommand(args)) { + return emptyGitConfigSnapshot() + } if (args[0] === 'rev-parse' && args.includes('refs/remotes/origin/feature')) { if (originBranchExists) { return { stdout: 'abc123\n' } @@ -166,6 +180,9 @@ describe('local upstream negative cache', () => { if (args[0] === 'rev-parse' && args.includes('HEAD@{u}')) { throw new Error(`fatal: no upstream configured for branch ${currentBranch}`) } + if (isConfigListSnapshotCommand(args)) { + return emptyGitConfigSnapshot() + } if (args[0] === 'rev-parse' && args.some((arg) => arg.startsWith('refs/remotes/origin/'))) { if (originBranchExists) { return { stdout: 'abc123\n' } @@ -227,6 +244,9 @@ describe('local upstream negative cache', () => { if (args[0] === 'rev-parse' && args.includes('HEAD@{u}')) { throw new Error(`fatal: no upstream configured for branch ${currentBranch}`) } + if (isConfigListSnapshotCommand(args)) { + return emptyGitConfigSnapshot() + } if (args[0] === 'rev-parse' && args.some((arg) => arg.startsWith('refs/remotes/origin/'))) { if (originBranchExists) { return { stdout: 'abc123\n' } @@ -282,6 +302,9 @@ describe('local upstream negative cache', () => { if (args[0] === 'rev-parse' && args.includes('HEAD@{u}')) { throw new Error(`fatal: no upstream configured for branch ${currentBranch}`) } + if (isConfigListSnapshotCommand(args)) { + return emptyGitConfigSnapshot() + } if (args[0] === 'rev-parse' && args.some((arg) => arg.startsWith('refs/remotes/origin/'))) { throw new Error('missing remote branch') } @@ -313,6 +336,9 @@ describe('local upstream negative cache', () => { if (args[0] === 'rev-parse' && args.includes('HEAD@{u}')) { throw new Error(`fatal: no upstream configured for branch ${currentBranch}`) } + if (isConfigListSnapshotCommand(args)) { + return emptyGitConfigSnapshot() + } if (args[0] === 'rev-parse' && args.includes(`refs/remotes/origin/${currentBranch}`)) { return { stdout: 'abc123\n' } } diff --git a/src/main/git/status-upstream-probe-churn.test.ts b/src/main/git/status-upstream-probe-churn.test.ts index ddf216b3f..f5e8f06c3 100644 --- a/src/main/git/status-upstream-probe-churn.test.ts +++ b/src/main/git/status-upstream-probe-churn.test.ts @@ -41,6 +41,23 @@ function getGitArgs(call: unknown[]): string[] { return call[0] as string[] } +function isConfigListSnapshotCommand(args: string[]): boolean { + return args[0] === 'config' && args[1] === '--list' && args[2] === '-z' +} + +function emptyGitConfigSnapshot(): { stdout: string } { + return { stdout: 'core.repositoryformatversion\n0\0' } +} + +function featureFixPushTargetSnapshot(): { stdout: string } { + const records = [ + 'branch.feature/fix.pushremote\nfork', + 'branch.feature/fix.remote\nfork', + 'branch.feature/fix.merge\nrefs/heads/feature/fix' + ] + return { stdout: `${records.join('\0')}\0` } +} + describe('getStatus missing-upstream polling churn', () => { beforeEach(() => { clearEffectiveUpstreamStatusCacheForTests() @@ -68,6 +85,9 @@ describe('getStatus missing-upstream polling churn', () => { if (args[0] === 'rev-parse' && args.includes('HEAD@{u}')) { throw new Error("fatal: no upstream configured for branch 'Initi-Project'") } + if (isConfigListSnapshotCommand(args)) { + return emptyGitConfigSnapshot() + } if (args[0] === 'rev-parse' && args.includes('refs/remotes/origin/Initi-Project')) { throw new Error('missing remote branch') } @@ -106,6 +126,9 @@ describe('getStatus missing-upstream polling churn', () => { if (args[0] === 'rev-parse' && args.includes('HEAD@{u}')) { throw new Error("fatal: no upstream configured for branch 'Initi-Project'") } + if (isConfigListSnapshotCommand(args)) { + return emptyGitConfigSnapshot() + } if (args[0] === 'rev-parse' && args.includes('refs/remotes/origin/Initi-Project')) { throw new Error('missing remote branch') } @@ -138,6 +161,9 @@ describe('getStatus missing-upstream polling churn', () => { await Promise.resolve() throw new Error("fatal: no upstream configured for branch 'Initi-Project'") } + if (isConfigListSnapshotCommand(args)) { + return emptyGitConfigSnapshot() + } if (args[0] === 'rev-parse' && args.includes('refs/remotes/origin/Initi-Project')) { await Promise.resolve() throw new Error('missing remote branch') @@ -173,20 +199,8 @@ describe('getStatus missing-upstream polling churn', () => { if (args[0] === 'rev-parse' && args.includes('HEAD@{u}')) { throw new Error("fatal: no upstream configured for branch 'feature/fix'") } - if (args[0] === 'config' && args.includes('branch.feature/fix.pushRemote')) { - return { stdout: 'fork\n' } - } - if (args[0] === 'config' && args.includes('remote.pushDefault')) { - throw new Error('missing pushDefault') - } - if (args[0] === 'config' && args.includes('branch.feature/fix.remote')) { - return { stdout: 'fork\n' } - } - if (args[0] === 'config' && args.includes('branch.feature/fix.merge')) { - return { stdout: 'refs/heads/feature/fix\n' } - } - if (args[0] === 'config' && args.includes('branch.feature/fix.base')) { - throw new Error('missing branch base') + if (isConfigListSnapshotCommand(args)) { + return featureFixPushTargetSnapshot() } if (args[0] === 'rev-parse' && args.some((arg) => arg.startsWith('refs/remotes/'))) { throw new Error('missing remote branch') @@ -222,6 +236,9 @@ describe('getStatus missing-upstream polling churn', () => { if (args[0] === 'rev-parse' && args.includes('HEAD@{u}')) { throw new Error('fatal: no upstream configured') } + if (isConfigListSnapshotCommand(args)) { + return emptyGitConfigSnapshot() + } if (args[0] === 'rev-parse' && args.some((arg) => arg.startsWith('refs/remotes/origin/'))) { throw new Error('missing remote branch') } @@ -241,4 +258,44 @@ describe('getStatus missing-upstream polling churn', () => { 'refs/remotes/origin/Other-Project' ]) }) + + it('coalesces no-upstream config reads into one snapshot subprocess', async () => { + gitExecFileAsyncMock.mockImplementation(async (args: string[]) => { + if (args.includes('status')) { + return { + stdout: '# branch.oid abcdef1234567890\n# branch.head Initi-Project\n' + } + } + if (args[0] === 'symbolic-ref' && args.includes('HEAD')) { + return { stdout: 'Initi-Project\n' } + } + if (args[0] === 'rev-parse' && args.includes('HEAD@{u}')) { + throw new Error("fatal: no upstream configured for branch 'Initi-Project'") + } + if (isConfigListSnapshotCommand(args)) { + return emptyGitConfigSnapshot() + } + if (args[0] === 'rev-parse' && args.includes('refs/remotes/origin/Initi-Project')) { + throw new Error('missing remote branch') + } + throw new Error(`unexpected git command: ${args.join(' ')}`) + }) + + const status = await getStatus('/repo') + + const configListCalls = gitExecFileAsyncMock.mock.calls.filter((call) => + isConfigListSnapshotCommand(getGitArgs(call)) + ) + const configGetCalls = gitExecFileAsyncMock.mock.calls.filter((call) => { + const args = getGitArgs(call) + return args[0] === 'config' && args[1] === '--get' + }) + + expect(configListCalls).toHaveLength(1) + expect(configGetCalls).toHaveLength(0) + if (!status.upstreamStatus) { + throw new Error('expected upstream status') + } + expect(status.upstreamStatus.hasUpstream).toBe(false) + }) }) diff --git a/src/main/git/status.ts b/src/main/git/status.ts index 5073da342..b064e31bb 100644 --- a/src/main/git/status.ts +++ b/src/main/git/status.ts @@ -21,6 +21,7 @@ import { getEffectiveGitUpstreamStatus, splitRemoteBranchName } from '../../shared/git-effective-upstream' +import { createGitConfigSnapshotRunner } from '../../shared/git-config-snapshot-runner' import { isBinaryBuffer } from '../../shared/binary-buffer' import { applyLineStats, @@ -654,11 +655,14 @@ async function probeEffectiveUpstreamStatus( options: GitRuntimeOptions = {} ): Promise<{ status: GitUpstreamStatus; probedSameNameOriginRef: boolean }> { let probedSameNameOriginRef = false + const snapshotRunner = createGitConfigSnapshotRunner((args) => + gitExecFileAsync(args, gitOptionsForWorktree(worktreePath, options)) + ) const status = await getEffectiveGitUpstreamStatus((args) => { if (args[0] === 'rev-parse' && args.includes(`refs/remotes/origin/${branchName}`)) { probedSameNameOriginRef = true } - return gitExecFileAsync(args, gitOptionsForWorktree(worktreePath, options)) + return snapshotRunner(args) }) return { status, probedSameNameOriginRef } } diff --git a/src/relay/git-status-upstream-negative-cache.test.ts b/src/relay/git-status-upstream-negative-cache.test.ts index e232f7912..c69fa11e4 100644 --- a/src/relay/git-status-upstream-negative-cache.test.ts +++ b/src/relay/git-status-upstream-negative-cache.test.ts @@ -7,6 +7,14 @@ import { readOrProbeNoEffectiveUpstreamStatus } from './git-status-upstream-negative-cache' +function isConfigListSnapshotCommand(args: string[]): boolean { + return args[0] === 'config' && args[1] === '--list' && args[2] === '-z' +} + +function emptyGitConfigSnapshot(): { stdout: string } { + return { stdout: 'core.repositoryformatversion\n0\0' } +} + describe('relay upstream negative cache', () => { beforeEach(() => { clearNoEffectiveUpstreamStatusCache() @@ -25,6 +33,9 @@ describe('relay upstream negative cache', () => { if (args[0] === 'rev-parse' && args.includes('HEAD@{u}')) { throw new Error('fatal: no upstream configured for branch feature') } + if (isConfigListSnapshotCommand(args)) { + return emptyGitConfigSnapshot() + } if (args[0] === 'rev-parse' && args.includes('refs/remotes/origin/feature')) { if (originBranchExists) { return { stdout: 'abc123\n' } @@ -65,6 +76,9 @@ describe('relay upstream negative cache', () => { if (args[0] === 'rev-parse' && args.includes('HEAD@{u}')) { throw new Error('fatal: no upstream configured for branch feature') } + if (isConfigListSnapshotCommand(args)) { + return emptyGitConfigSnapshot() + } if (args[0] === 'rev-parse' && args.includes('refs/remotes/origin/feature')) { if (originBranchExists) { return { stdout: 'abc123\n' } @@ -114,6 +128,9 @@ describe('relay upstream negative cache', () => { if (args[0] === 'rev-parse' && args.includes('HEAD@{u}')) { throw new Error('fatal: no upstream configured for branch feature') } + if (isConfigListSnapshotCommand(args)) { + return emptyGitConfigSnapshot() + } if (args[0] === 'rev-parse' && args.includes('refs/remotes/origin/feature')) { if (originBranchExists) { return { stdout: 'abc123\n' } @@ -147,6 +164,9 @@ describe('relay upstream negative cache', () => { if (args[0] === 'rev-parse' && args.includes('HEAD@{u}')) { throw new Error(`fatal: no upstream configured for branch ${branchName}`) } + if (isConfigListSnapshotCommand(args)) { + return emptyGitConfigSnapshot() + } if (args[0] === 'rev-parse' && args.includes(`refs/remotes/origin/${branchName}`)) { return { stdout: 'abc123\n' } } @@ -185,6 +205,9 @@ describe('relay upstream negative cache', () => { if (args[0] === 'rev-parse' && args.includes('HEAD@{u}')) { throw new Error('fatal: no upstream configured for branch feature') } + if (isConfigListSnapshotCommand(args)) { + return emptyGitConfigSnapshot() + } if (args[0] === 'rev-parse' && args.includes('refs/remotes/origin/feature')) { if (originBranchExists) { return { stdout: 'abc123\n' } @@ -216,6 +239,9 @@ describe('relay upstream negative cache', () => { if (args[0] === 'rev-parse' && args.includes('HEAD@{u}')) { throw new Error(`fatal: no upstream configured for branch ${branchName}`) } + if (isConfigListSnapshotCommand(args)) { + return emptyGitConfigSnapshot() + } if (args[0] === 'rev-parse' && args.includes(`refs/remotes/origin/${branchName}`)) { return { stdout: 'abc123\n' } } @@ -255,6 +281,9 @@ describe('relay upstream negative cache', () => { if (args[0] === 'rev-parse' && args.includes('HEAD@{u}')) { throw new Error(`fatal: no upstream configured for branch ${branchName}`) } + if (isConfigListSnapshotCommand(args)) { + return emptyGitConfigSnapshot() + } if (args[0] === 'rev-parse' && args.includes(`refs/remotes/origin/${branchName}`)) { throw new Error('missing remote branch') } @@ -279,6 +308,9 @@ describe('relay upstream negative cache', () => { if (args[0] === 'rev-parse' && args.includes('HEAD@{u}')) { throw new Error(`fatal: no upstream configured for branch ${branchName}`) } + if (isConfigListSnapshotCommand(args)) { + return emptyGitConfigSnapshot() + } if (args[0] === 'rev-parse' && args.includes(`refs/remotes/origin/${branchName}`)) { return { stdout: 'abc123\n' } } @@ -294,4 +326,38 @@ describe('relay upstream negative cache', () => { expect(getNoEffectiveUpstreamStatusCacheCountForTests()).toBe(0) expect(getNoEffectiveUpstreamStatusGenerationCountForTests()).toBe(512) }) + + it('coalesces no-upstream config reads into one snapshot subprocess', async () => { + const runGit = vi.fn(async (args: string[]): Promise<{ stdout: string }> => { + if (args[0] === 'symbolic-ref') { + return { stdout: 'feature\n' } + } + if (args[0] === 'rev-parse' && args.includes('HEAD@{u}')) { + throw new Error('fatal: no upstream configured for branch feature') + } + if (isConfigListSnapshotCommand(args)) { + return emptyGitConfigSnapshot() + } + if (args[0] === 'rev-parse' && args.includes('refs/remotes/origin/feature')) { + throw new Error('missing remote branch') + } + throw new Error(`No upstream fixture for git ${args.join(' ')}`) + }) + + const status = await readOrProbeNoEffectiveUpstreamStatus( + { worktreePath: '/repo', branchName: 'feature' }, + runGit + ) + const configListCalls = runGit.mock.calls.filter((call) => + isConfigListSnapshotCommand(call[0] as string[]) + ) + const configGetCalls = runGit.mock.calls.filter((call) => { + const args = call[0] as string[] + return args[0] === 'config' && args[1] === '--get' + }) + + expect(configListCalls).toHaveLength(1) + expect(configGetCalls).toHaveLength(0) + expect(status.hasUpstream).toBe(false) + }) }) diff --git a/src/relay/git-status-upstream-negative-cache.ts b/src/relay/git-status-upstream-negative-cache.ts index a9dbec317..e3ad9239c 100644 --- a/src/relay/git-status-upstream-negative-cache.ts +++ b/src/relay/git-status-upstream-negative-cache.ts @@ -1,3 +1,4 @@ +import { createGitConfigSnapshotRunner } from '../shared/git-config-snapshot-runner' import { getEffectiveGitUpstreamStatus } from '../shared/git-effective-upstream' import type { GitCommandRunner } from '../shared/git-effective-upstream' import type { GitUpstreamStatus } from '../shared/types' @@ -115,12 +116,13 @@ export async function readOrProbeNoEffectiveUpstreamStatus( } let probedSameNameOriginRef = false + const snapshotRunner = createGitConfigSnapshotRunner(runGit) const writeGeneration = noEffectiveUpstreamWriteGeneration.get(cacheKey) ?? 0 const probe = getEffectiveGitUpstreamStatus((args) => { if (args[0] === 'rev-parse' && args.includes(`refs/remotes/origin/${identity.branchName}`)) { probedSameNameOriginRef = true } - return runGit(args) + return snapshotRunner(args) }).then((status) => { cacheNoEffectiveUpstreamStatus(cacheKey, status, probedSameNameOriginRef, writeGeneration) return status diff --git a/src/shared/git-config-snapshot-runner.test.ts b/src/shared/git-config-snapshot-runner.test.ts new file mode 100644 index 000000000..1ec5523f3 --- /dev/null +++ b/src/shared/git-config-snapshot-runner.test.ts @@ -0,0 +1,227 @@ +import { describe, expect, it, vi } from 'vitest' +import { + createGitConfigSnapshotRunner, + GitConfigSnapshotKeyNotFoundError +} from './git-config-snapshot-runner' + +function listSnapshot(records: string[]): string { + return `${records.join('\0')}\0` +} + +function getGitArgs(call: unknown[]): string[] { + return call[0] as string[] +} + +function countCalls(runGit: ReturnType, expectedArgs: string[]): number { + return runGit.mock.calls.filter((call) => { + const args = getGitArgs(call) + return ( + args.length === expectedArgs.length && args.every((arg, index) => arg === expectedArgs[index]) + ) + }).length +} + +function createDeferred(): { promise: Promise; resolve: (value: T) => void } { + let resolveDeferred: (value: T) => void = () => undefined + const promise = new Promise((resolve) => { + resolveDeferred = resolve + }) + return { promise, resolve: resolveDeferred } +} + +describe('createGitConfigSnapshotRunner', () => { + it('serves many config --get lookups from one config --list -z call', async () => { + const runGit = vi.fn(async () => ({ + stdout: listSnapshot([ + 'branch.main.remote\norigin', + 'branch.main.merge\nrefs/heads/main', + 'remote.pushdefault\nfork' + ]) + })) + const runner = createGitConfigSnapshotRunner(runGit) + + await runner(['config', '--get', 'branch.main.remote']) + await runner(['config', '--get', 'branch.main.merge']) + await runner(['config', '--get', 'remote.pushDefault']) + + expect(countCalls(runGit, ['config', '--list', '-z'])).toBe(1) + expect(runGit.mock.calls.filter((call) => getGitArgs(call)[1] === '--get')).toHaveLength(0) + }) + + it('returns branch and remote values from a representative snapshot', async () => { + const runGit = vi.fn(async () => ({ + stdout: listSnapshot([ + 'branch.feature.remote\norigin', + 'branch.feature.merge\nrefs/heads/feature', + 'remote.origin.url\ngit@example.com:org/repo.git' + ]) + })) + const runner = createGitConfigSnapshotRunner(runGit) + + await expect(runner(['config', '--get', 'branch.feature.remote'])).resolves.toEqual({ + stdout: 'origin' + }) + await expect(runner(['config', '--get', 'branch.feature.merge'])).resolves.toEqual({ + stdout: 'refs/heads/feature' + }) + await expect(runner(['config', '--get', 'remote.origin.url'])).resolves.toEqual({ + stdout: 'git@example.com:org/repo.git' + }) + }) + + it('passes remote commands through unchanged', async () => { + const runGit = vi.fn(async (args: string[]) => ({ stdout: args.join(' ') })) + const runner = createGitConfigSnapshotRunner(runGit) + + await expect(runner(['remote'])).resolves.toEqual({ stdout: 'remote' }) + await expect(runner(['remote', 'get-url', 'origin'])).resolves.toEqual({ + stdout: 'remote get-url origin' + }) + + expect(countCalls(runGit, ['remote'])).toBe(1) + expect(countCalls(runGit, ['remote', 'get-url', 'origin'])).toBe(1) + }) + + it('case-folds the requested section and key', async () => { + const runGit = vi.fn(async () => ({ + stdout: listSnapshot(['remote.pushdefault\nfork']) + })) + const runner = createGitConfigSnapshotRunner(runGit) + + await expect(runner(['config', '--get', 'remote.pushDefault'])).resolves.toEqual({ + stdout: 'fork' + }) + }) + + it('preserves subsections with dots in lookup keys', async () => { + const runGit = vi.fn(async () => ({ + stdout: listSnapshot(['branch.feature.x.merge\nrefs/heads/feature.x']) + })) + const runner = createGitConfigSnapshotRunner(runGit) + + await expect(runner(['config', '--get', 'branch.feature.x.merge'])).resolves.toEqual({ + stdout: 'refs/heads/feature.x' + }) + }) + + it('returns the last multivar value for config --get', async () => { + const runGit = vi.fn(async () => ({ + stdout: listSnapshot([ + 'branch.main.merge\nrefs/heads/old', + 'branch.main.merge\nrefs/heads/new' + ]) + })) + const runner = createGitConfigSnapshotRunner(runGit) + + await expect(runner(['config', '--get', 'branch.main.merge'])).resolves.toEqual({ + stdout: 'refs/heads/new' + }) + }) + + it('parses valueless boolean keys without corrupting adjacent records', async () => { + const runGit = vi.fn(async () => ({ + stdout: listSnapshot([ + 'remote.origin.mirror', + 'remote.origin.url\ngit@example.com:org/repo.git' + ]) + })) + const runner = createGitConfigSnapshotRunner(runGit) + + await expect(runner(['config', '--get', 'remote.origin.mirror'])).resolves.toEqual({ + stdout: '' + }) + await expect(runner(['config', '--get', 'remote.origin.url'])).resolves.toEqual({ + stdout: 'git@example.com:org/repo.git' + }) + }) + + it('rejects an absent key from a loaded snapshot instead of returning empty success', async () => { + // Why: real `git config --get` exits non-zero for a missing key; a loaded + // snapshot must preserve that contract (not turn a miss into '' success), + // and must NOT fall back to a real config --get (that would re-spawn the + // subprocess this runner exists to coalesce away). + const runGit = vi.fn(async () => ({ + stdout: listSnapshot(['branch.main.remote\norigin']) + })) + const runner = createGitConfigSnapshotRunner(runGit) + + await expect(runner(['config', '--get', 'branch.main.merge'])).rejects.toThrow( + GitConfigSnapshotKeyNotFoundError + ) + // The miss is served from the loaded snapshot — no passthrough config --get. + expect(countCalls(runGit, ['config', '--list', '-z'])).toBe(1) + expect( + runGit.mock.calls.filter((call) => { + const args = getGitArgs(call) + return args[0] === 'config' && args[1] === '--get' + }) + ).toHaveLength(0) + }) + + it('single-flights concurrent first config --get lookups', async () => { + const snapshot = createDeferred<{ stdout: string }>() + const runGit = vi.fn(async () => await snapshot.promise) + const runner = createGitConfigSnapshotRunner(runGit) + + const lookups = Promise.all([ + runner(['config', '--get', 'branch.main.remote']), + runner(['config', '--get', 'branch.main.merge']), + runner(['config', '--get', 'remote.pushDefault']) + ]) + await Promise.resolve() + + expect(countCalls(runGit, ['config', '--list', '-z'])).toBe(1) + snapshot.resolve({ + stdout: listSnapshot([ + 'branch.main.remote\norigin', + 'branch.main.merge\nrefs/heads/main', + 'remote.pushdefault\nfork' + ]) + }) + + await expect(lookups).resolves.toEqual([ + { stdout: 'origin' }, + { stdout: 'refs/heads/main' }, + { stdout: 'fork' } + ]) + expect(countCalls(runGit, ['config', '--list', '-z'])).toBe(1) + }) + + it('falls back to real config --get after snapshot fetch failure', async () => { + const runGit = vi.fn(async (args: string[]) => { + if (args[0] === 'config' && args[1] === '--list') { + throw new Error('snapshot failed') + } + if (args[0] === 'config' && args[1] === '--get') { + return { stdout: `real:${args[2]}` } + } + throw new Error(`unexpected git command: ${args.join(' ')}`) + }) + const runner = createGitConfigSnapshotRunner(runGit) + + await expect(runner(['config', '--get', 'branch.main.remote'])).resolves.toEqual({ + stdout: 'real:branch.main.remote' + }) + await expect(runner(['config', '--get', 'branch.main.merge'])).resolves.toEqual({ + stdout: 'real:branch.main.merge' + }) + + expect(countCalls(runGit, ['config', '--list', '-z'])).toBe(1) + expect(runGit.mock.calls.filter((call) => getGitArgs(call)[1] === '--get')).toHaveLength(2) + }) + + it('passes non-config commands through', async () => { + const runGit = vi.fn(async (args: string[]) => ({ stdout: args.join(' ') })) + const runner = createGitConfigSnapshotRunner(runGit) + + await expect(runner(['rev-parse', '--abbrev-ref', 'HEAD@{u}'])).resolves.toEqual({ + stdout: 'rev-parse --abbrev-ref HEAD@{u}' + }) + await expect(runner(['symbolic-ref', '--quiet', '--short', 'HEAD'])).resolves.toEqual({ + stdout: 'symbolic-ref --quiet --short HEAD' + }) + + expect(countCalls(runGit, ['rev-parse', '--abbrev-ref', 'HEAD@{u}'])).toBe(1) + expect(countCalls(runGit, ['symbolic-ref', '--quiet', '--short', 'HEAD'])).toBe(1) + }) +}) diff --git a/src/shared/git-config-snapshot-runner.ts b/src/shared/git-config-snapshot-runner.ts new file mode 100644 index 000000000..0eb5a3694 --- /dev/null +++ b/src/shared/git-config-snapshot-runner.ts @@ -0,0 +1,103 @@ +type GitCommandRunner = (args: string[]) => Promise<{ stdout: string }> + +type GitConfigSnapshot = Map + +// Why: mirror `git config --get`'s exit-1-on-absent-key so an intercepted miss +// rejects (matching real git) instead of resolving an empty success value. +export class GitConfigSnapshotKeyNotFoundError extends Error { + constructor(key: string) { + super(`git config --get found no value for '${key}'`) + this.name = 'GitConfigSnapshotKeyNotFoundError' + } +} + +function isConfigGetCommand(args: string[]): boolean { + return args.length === 3 && args[0] === 'config' && args[1] === '--get' +} + +function canonicalizeGitConfigLookupKey(key: string): string { + const parts = key.split('.') + if (parts.length === 1) { + return key.toLowerCase() + } + const firstPart = parts[0]?.toLowerCase() ?? '' + const lastPart = parts.at(-1)?.toLowerCase() ?? '' + return [firstPart, ...parts.slice(1, -1), lastPart].join('.') +} + +function parseGitConfigListSnapshot(stdout: string): GitConfigSnapshot { + const snapshot: GitConfigSnapshot = new Map() + for (const record of stdout.split('\0')) { + if (!record.trim()) { + continue + } + + // Why: config values may contain newlines, so only the first newline + // separates git's emitted key from its value. + const separatorIndex = record.indexOf('\n') + const key = separatorIndex === -1 ? record : record.slice(0, separatorIndex) + const value = separatorIndex === -1 ? '' : record.slice(separatorIndex + 1) + const values = snapshot.get(key) ?? [] + values.push(value) + snapshot.set(key, values) + } + return snapshot +} + +export function createGitConfigSnapshotRunner(runGit: GitCommandRunner): GitCommandRunner { + let snapshotPromise: Promise | null = null + let snapshot: GitConfigSnapshot | null = null + let interceptionDisabled = false + + const readSnapshot = (): Promise => { + if (snapshot) { + return Promise.resolve(snapshot) + } + if (!snapshotPromise) { + // Why: upstream resolvers read config keys with Promise.all; the first + // caller must publish the in-flight snapshot before any await. + try { + snapshotPromise = runGit(['config', '--list', '-z']) + .then(({ stdout }) => { + snapshot = parseGitConfigListSnapshot(stdout) + return snapshot + }) + .catch(() => { + // Why: a snapshot failure should preserve existing real-git + // behavior for this round instead of failing the config lookup. + interceptionDisabled = true + snapshotPromise = null + return null + }) + } catch { + interceptionDisabled = true + snapshotPromise = null + return Promise.resolve(null) + } + } + return snapshotPromise + } + + return async (args) => { + if (interceptionDisabled || !isConfigGetCommand(args)) { + return runGit(args) + } + + const configSnapshot = await readSnapshot() + if (!configSnapshot) { + return runGit(args) + } + + const key = args[2] ?? '' + const values = configSnapshot.get(canonicalizeGitConfigLookupKey(key)) + if (!values?.length) { + // Why: real `git config --get` exits non-zero for an absent key (it does + // not return empty success), so reject here to stay a faithful drop-in — + // callers branch on the rejection just as they would for real git. + throw new GitConfigSnapshotKeyNotFoundError(key) + } + + // Why: git config --get resolves multivar keys using the last occurrence. + return { stdout: values.at(-1) ?? '' } + } +}