xiaoxue_admin
/
orca
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Reduce duplicate diff loading work (#6389) 

* Deduplicate in-flight diff reads

* Clear diff dedupe for ref-moving SSH operations

* Clear diff dedupe for worktree ref mutations

* Document diff dedupe mutation invalidation

---------

Co-authored-by: Neil <neil@stably.ai>
This commit is contained in:
Brennan Benson 2026-06-25 18:47:26 -07:00 committed by GitHub
parent a12566b962
commit 829f8d9618
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 1808 additions and 383 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

257
src/main/git/status.test.ts
View File

@ -70,15 +70,41 @@ import {
bulkUnstageFiles,
clearEffectiveUpstreamStatusCacheForTests,
detectConflictOperation,
getBranchDiff,
discardChanges,
getCommitDiff,
getBranchCompare,
getCommitCompare,
getDiff,
getStagedCommitContext,
getStatus,
isWithinWorktree
isWithinWorktree,
stageFile
} from './status'
function deferredBuffer(content: string): {
promise: Promise<{ stdout: Buffer }>
resolve: () => void
} {
let resolve!: (value: { stdout: Buffer }) => void
const promise = new Promise<{ stdout: Buffer }>((innerResolve) => {
resolve = innerResolve
})
return {
promise,
resolve: () => resolve({ stdout: Buffer.from(content) })
}
}
async function waitForMockCalls(mock: ReturnType<typeof vi.fn>, calls: number): Promise<void> {
for (let i = 0; i < 20; i++) {
if (mock.mock.calls.length >= calls) {
return
}
await new Promise<void>((resolve) => setTimeout(resolve, 0))
}
}
describe('discardChanges', () => {
beforeEach(() => {
gitExecFileAsyncMock.mockReset()
@ -445,6 +471,235 @@ describe('getDiff', () => {
mimeType: 'application/pdf'
})
})
it('coalesces concurrent identical staged diff reads while in flight', async () => {
const leftBlob = deferredBuffer('head-content\n')
const rightBlob = deferredBuffer('index-content\n')
const pendingBuffers = [leftBlob, rightBlob]
gitExecFileAsyncBufferMock.mockImplementation(async () => pendingBuffers.shift()!.promise)
const reads = Array.from({ length: 8 }, () => getDiff('/repo', 'src/file.ts', true))
await waitForMockCalls(gitExecFileAsyncBufferMock, 1)
expect(gitExecFileAsyncBufferMock).toHaveBeenCalledTimes(1)
leftBlob.resolve()
await waitForMockCalls(gitExecFileAsyncBufferMock, 2)
rightBlob.resolve()
const results = await Promise.all(reads)
expect(gitExecFileAsyncBufferMock).toHaveBeenCalledTimes(2)
expect(results.every((result) => result.kind === 'text')).toBe(true)
gitExecFileAsyncBufferMock
.mockResolvedValueOnce({ stdout: Buffer.from('fresh-head\n') })
.mockResolvedValueOnce({ stdout: Buffer.from('fresh-index\n') })
await getDiff('/repo', 'src/file.ts', true)
expect(gitExecFileAsyncBufferMock).toHaveBeenCalledTimes(4)
})
it('clears pending diff reads when a mutation runs', async () => {
const firstBlob = deferredBuffer('head-content\n')
const secondBlob = deferredBuffer('fresh-head-content\n')
const pendingBuffers = [firstBlob, secondBlob]
gitExecFileAsyncBufferMock.mockImplementation(async () => pendingBuffers.shift()!.promise)
readFileMock.mockResolvedValue(Buffer.from('working-tree\n'))
gitExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' })
const first = getDiff('/repo', 'src/file.ts', false)
await waitForMockCalls(gitExecFileAsyncBufferMock, 1)
await stageFile('/repo', 'src/file.ts')
const second = getDiff('/repo', 'src/file.ts', false)
await waitForMockCalls(gitExecFileAsyncBufferMock, 2)
firstBlob.resolve()
secondBlob.resolve()
await Promise.all([first, second])
expect(gitExecFileAsyncBufferMock).toHaveBeenCalledTimes(2)
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(
['add', '--', ':(literal)src/file.ts'],
expect.objectContaining({ cwd: '/repo' })
)
})
it('coalesces concurrent identical branch and commit diff reads while in flight', async () => {
const branchLeftBlob = deferredBuffer('branch-left\n')
const branchRightBlob = deferredBuffer('branch-right\n')
const pendingBranchBuffers = [branchLeftBlob, branchRightBlob]
gitExecFileAsyncBufferMock.mockImplementation(async () => pendingBranchBuffers.shift()!.promise)
const branchReads = Array.from({ length: 8 }, () =>
getBranchDiff('/repo', {
mergeBase: 'b'.repeat(40),
headOid: 'c'.repeat(40),
filePath: 'src/file.ts',
oldPath: 'src/old-file.ts'
})
)
await waitForMockCalls(gitExecFileAsyncBufferMock, 1)
branchLeftBlob.resolve()
await waitForMockCalls(gitExecFileAsyncBufferMock, 2)
branchRightBlob.resolve()
await Promise.all(branchReads)
expect(gitExecFileAsyncBufferMock).toHaveBeenCalledTimes(2)
gitExecFileAsyncBufferMock.mockReset()
const commitLeftBlob = deferredBuffer('commit-left\n')
const commitRightBlob = deferredBuffer('commit-right\n')
const pendingCommitBuffers = [commitLeftBlob, commitRightBlob]
gitExecFileAsyncBufferMock.mockImplementation(async () => pendingCommitBuffers.shift()!.promise)
const commitReads = Array.from({ length: 8 }, () =>
getCommitDiff('/repo', {
parentOid: 'd'.repeat(40),
commitOid: 'e'.repeat(40),
filePath: 'src/file.ts',
oldPath: 'src/old-file.ts'
})
)
await waitForMockCalls(gitExecFileAsyncBufferMock, 1)
commitLeftBlob.resolve()
await waitForMockCalls(gitExecFileAsyncBufferMock, 2)
commitRightBlob.resolve()
await Promise.all(commitReads)
expect(gitExecFileAsyncBufferMock).toHaveBeenCalledTimes(2)
})
it('coalesces logically identical branch and commit diff args regardless of property order', async () => {
gitExecFileAsyncBufferMock.mockResolvedValue({ stdout: Buffer.from('blob\n') })
await Promise.all([
getBranchDiff('/repo', {
mergeBase: 'b'.repeat(40),
headOid: 'c'.repeat(40),
filePath: 'src/file.ts',
oldPath: 'src/old-file.ts'
}),
getBranchDiff('/repo', {
oldPath: 'src/old-file.ts',
filePath: 'src/file.ts',
headOid: 'c'.repeat(40),
mergeBase: 'b'.repeat(40)
})
])
expect(gitExecFileAsyncBufferMock).toHaveBeenCalledTimes(2)
gitExecFileAsyncBufferMock.mockClear()
await Promise.all([
getCommitDiff('/repo', {
parentOid: 'd'.repeat(40),
commitOid: 'e'.repeat(40),
filePath: 'src/file.ts',
oldPath: 'src/old-file.ts'
}),
getCommitDiff('/repo', {
oldPath: 'src/old-file.ts',
filePath: 'src/file.ts',
commitOid: 'e'.repeat(40),
parentOid: 'd'.repeat(40)
})
])
expect(gitExecFileAsyncBufferMock).toHaveBeenCalledTimes(2)
})
it('keeps distinct diff inputs on separate in-flight reads', async () => {
gitExecFileAsyncBufferMock.mockResolvedValue({ stdout: Buffer.from('blob\n') })
readFileMock.mockResolvedValue(Buffer.from('working-tree\n'))
await Promise.all([
getDiff('/repo', 'src/file.ts', false, false),
getDiff('/repo', 'src/file.ts', false, true),
getDiff('/repo', 'src/file.ts', true, false),
getDiff('/repo', 'src/file.ts', true, false, { wslDistro: 'ubuntu' }),
getDiff('/repo', 'src/file.ts', true, false, { wslDistro: 'debian' })
])
expect(gitExecFileAsyncBufferMock).toHaveBeenCalledTimes(8)
gitExecFileAsyncBufferMock.mockReset()
gitExecFileAsyncBufferMock.mockResolvedValue({ stdout: Buffer.from('blob\n') })
await Promise.all([
getBranchDiff('/repo', {
mergeBase: 'b'.repeat(40),
headOid: 'c'.repeat(40),
filePath: 'src/file.ts'
}),
getBranchDiff('/repo', {
mergeBase: 'b'.repeat(40),
headOid: 'd'.repeat(40),
filePath: 'src/file.ts'
}),
getBranchDiff('/repo', {
mergeBase: 'b'.repeat(40),
headOid: 'c'.repeat(40),
filePath: 'src/file.ts',
oldPath: 'src/old-a.ts'
}),
getBranchDiff('/repo', {
mergeBase: 'b'.repeat(40),
headOid: 'c'.repeat(40),
filePath: 'src/file.ts',
oldPath: 'src/old-b.ts'
}),
getCommitDiff('/repo', {
parentOid: 'e'.repeat(40),
commitOid: 'f'.repeat(40),
filePath: 'src/file.ts'
}),
getCommitDiff('/repo', {
parentOid: 'a'.repeat(40),
commitOid: 'f'.repeat(40),
filePath: 'src/file.ts'
}),
getCommitDiff('/repo', {
parentOid: 'e'.repeat(40),
commitOid: 'f'.repeat(40),
filePath: 'src/file.ts',
oldPath: 'src/old-a.ts'
}),
getCommitDiff('/repo', {
parentOid: 'e'.repeat(40),
commitOid: 'f'.repeat(40),
filePath: 'src/file.ts',
oldPath: 'src/old-b.ts'
})
])
expect(gitExecFileAsyncBufferMock).toHaveBeenCalledTimes(16)
})
it('coalesces parentless root commit diff reads without reading a left-side blob', async () => {
const rightBlob = deferredBuffer('root-content\n')
gitExecFileAsyncBufferMock.mockImplementation(async () => rightBlob.promise)
const reads = Array.from({ length: 8 }, () =>
getCommitDiff('/repo', {
parentOid: null,
commitOid: 'e'.repeat(40),
filePath: 'src/file.ts'
})
)
await waitForMockCalls(gitExecFileAsyncBufferMock, 1)
rightBlob.resolve()
await Promise.all(reads)
expect(gitExecFileAsyncBufferMock).toHaveBeenCalledTimes(1)
})
})
describe('getStatus', () => {

247
src/main/git/status.ts
View File

@ -45,6 +45,7 @@ import {
import { resolveWorktreeAddBaseRef } from '../../shared/worktree-base-ref'
import { hasWorktreeBaseCommitRef } from './worktree-base-ref-probe'
import { getLargeDiffRenderLimit } from '../../shared/large-diff-render-limit'
import { InFlightPromiseDedupe, stableInFlightKey } from '../../shared/in-flight-promise-dedupe'
import type { GitRuntimeOptions } from './git-runtime-options'
import { gitOptionsForWorktree } from './git-runtime-options'
import { parseGitRevListFirstParentOid } from '../../shared/git-rev-list-output'
@ -63,15 +64,21 @@ type EffectiveUpstreamStatusCacheEntry = {
const effectiveUpstreamStatusCache = new Map<string, EffectiveUpstreamStatusCacheEntry>()
const effectiveUpstreamStatusInFlight = new Map<string, Promise<GitUpstreamStatus>>()
const retiredEffectiveUpstreamStatusInFlight = new Map<string, Promise<GitUpstreamStatus>>()
const gitDiffReadDedupe = new InFlightPromiseDedupe<GitDiffResult>()
const effectiveUpstreamStatusWriteGeneration = new Map<string, number>()
const statusReadsInFlight = new Map<string, Promise<GitStatusResult>>()
function gitRuntimeOptionsKey(options: GitRuntimeOptions): readonly unknown[] {
return [options.wslDistro ?? null]
}
// Why: status tests reuse this reset hook, so every cross-call memoization layer
// must reset together even though the historical name mentions upstream only.
export function clearEffectiveUpstreamStatusCacheForTests(): void {
effectiveUpstreamStatusCache.clear()
effectiveUpstreamStatusInFlight.clear()
retiredEffectiveUpstreamStatusInFlight.clear()
gitDiffReadDedupe.clear()
effectiveUpstreamStatusWriteGeneration.clear()
statusReadsInFlight.clear()
}
@ -101,6 +108,7 @@ export async function getStatus(
worktreePath: string,
options: GetStatusOptions = {}
): Promise<GitStatusResult> {
gitDiffReadDedupe.clear()
// Why: dedupe only concurrent identical reads; after settle, callers must
// execute a fresh status read rather than observing a cached result.
const cacheKey = getStatusReadKey(worktreePath, options)
@ -722,6 +730,26 @@ export async function getDiff(
staged: boolean,
compareAgainstHead = false,
options: GitRuntimeOptions = {}
): Promise<GitDiffResult> {
return gitDiffReadDedupe.run(
stableInFlightKey([
'diff',
worktreePath,
filePath,
staged,
compareAgainstHead,
...gitRuntimeOptionsKey(options)
]),
() => loadDiff(worktreePath, filePath, staged, compareAgainstHead, options)
)
}
async function loadDiff(
worktreePath: string,
filePath: string,
staged: boolean,
compareAgainstHead: boolean,
options: GitRuntimeOptions
): Promise<GitDiffResult> {
let originalContent = ''
let modifiedContent = ''
@ -852,6 +880,30 @@ export async function getBranchDiff(
oldPath?: string
},
options: GitRuntimeOptions = {}
): Promise<GitDiffResult> {
return gitDiffReadDedupe.run(
stableInFlightKey([
'branchDiff',
worktreePath,
args.headOid,
args.mergeBase,
args.filePath,
args.oldPath ?? null,
...gitRuntimeOptionsKey(options)
]),
() => loadBranchDiff(worktreePath, args, options)
)
}
async function loadBranchDiff(
worktreePath: string,
args: {
headOid: string
mergeBase: string
filePath: string
oldPath?: string
},
options: GitRuntimeOptions
): Promise<GitDiffResult> {
try {
const leftPath = args.oldPath ?? args.filePath
@ -941,6 +993,30 @@ export async function getCommitDiff(
oldPath?: string
},
options: GitRuntimeOptions = {}
): Promise<GitDiffResult> {
return gitDiffReadDedupe.run(
stableInFlightKey([
'commitDiff',
worktreePath,
args.commitOid,
args.parentOid ?? null,
args.filePath,
args.oldPath ?? null,
...gitRuntimeOptionsKey(options)
]),
() => loadCommitDiff(worktreePath, args, options)
)
}
async function loadCommitDiff(
worktreePath: string,
args: {
commitOid: string
parentOid?: string | null
filePath: string
oldPath?: string
},
options: GitRuntimeOptions
): Promise<GitDiffResult> {
try {
const leftPath = args.oldPath ?? args.filePath
@ -1316,10 +1392,15 @@ export async function stageFile(
filePath: string,
options: GitRuntimeOptions = {}
): Promise<void> {
await gitExecFileAsync(
['add', '--', literalPathspec(filePath)],
gitOptionsForWorktree(worktreePath, options)
)
gitDiffReadDedupe.clear()
try {
await gitExecFileAsync(
['add', '--', literalPathspec(filePath)],
gitOptionsForWorktree(worktreePath, options)
)
} finally {
gitDiffReadDedupe.clear()
}
}
/**
@ -1330,9 +1411,14 @@ export async function unstageFile(
filePath: string,
options: GitRuntimeOptions = {}
): Promise<void> {
await gitExecFileAsync(['restore', '--staged', '--', literalPathspec(filePath)], {
...gitOptionsForWorktree(worktreePath, options)
})
gitDiffReadDedupe.clear()
try {
await gitExecFileAsync(['restore', '--staged', '--', literalPathspec(filePath)], {
...gitOptionsForWorktree(worktreePath, options)
})
} finally {
gitDiffReadDedupe.clear()
}
}
export async function getStagedCommitContext(
@ -1388,6 +1474,7 @@ export async function commitChanges(
message: string,
options: GitRuntimeOptions = {}
): Promise<{ success: boolean; error?: string }> {
gitDiffReadDedupe.clear()
try {
await gitExecFileAsync(['commit', '-m', message], gitOptionsForWorktree(worktreePath, options))
return { success: true }
@ -1409,6 +1496,8 @@ export async function commitChanges(
readStringField('stdout') ??
(error instanceof Error ? error.message : 'Commit failed')
return { success: false, error: errorMessage }
} finally {
gitDiffReadDedupe.clear()
}
}
@ -1420,35 +1509,40 @@ export async function discardChanges(
filePath: string,
options: GitRuntimeOptions = {}
): Promise<void> {
gitDiffReadDedupe.clear()
const resolvedWorktree = path.resolve(worktreePath)
const resolvedTarget = path.resolve(worktreePath, filePath)
if (!isWithinWorktree(path, resolvedWorktree, resolvedTarget)) {
throw new Error(`Path "${filePath}" resolves outside the worktree`)
}
let tracked = false
try {
await gitExecFileAsync(['ls-files', '--error-unmatch', '--', literalPathspec(filePath)], {
...gitOptionsForWorktree(worktreePath, options)
})
tracked = true
} catch {
// File is not tracked by git
}
if (!isWithinWorktree(path, resolvedWorktree, resolvedTarget)) {
throw new Error(`Path "${filePath}" resolves outside the worktree`)
}
if (tracked) {
await gitExecFileAsync(
['restore', '--worktree', '--source=HEAD', '--', literalPathspec(filePath)],
{
let tracked = false
try {
await gitExecFileAsync(['ls-files', '--error-unmatch', '--', literalPathspec(filePath)], {
...gitOptionsForWorktree(worktreePath, options)
}
)
return
}
})
tracked = true
} catch {
// File is not tracked by git
}
await removeSafeUntrackedDiscardTarget(worktreePath, filePath, (targetPath) =>
cleanUntrackedPaths(worktreePath, [targetPath], options)
)
if (tracked) {
await gitExecFileAsync(
['restore', '--worktree', '--source=HEAD', '--', literalPathspec(filePath)],
{
...gitOptionsForWorktree(worktreePath, options)
}
)
return
}
await removeSafeUntrackedDiscardTarget(worktreePath, filePath, (targetPath) =>
cleanUntrackedPaths(worktreePath, [targetPath], options)
)
} finally {
gitDiffReadDedupe.clear()
}
}
function normalizeGitPathForCompare(filePath: string): string {
@ -1517,39 +1611,46 @@ export async function bulkDiscardChanges(
filePaths: string[],
options: GitRuntimeOptions = {}
): Promise<void> {
gitDiffReadDedupe.clear()
if (filePaths.length === 0) {
return
}
const resolvedWorktree = path.resolve(worktreePath)
for (const filePath of filePaths) {
const resolvedTarget = path.resolve(worktreePath, filePath)
if (!isWithinWorktree(path, resolvedWorktree, resolvedTarget)) {
throw new Error(`Path "${filePath}" resolves outside the worktree`)
}
}
const trackedPathSpecs = await listTrackedPathSpecs(worktreePath, filePaths, options)
const trackedPaths = filePaths.filter((filePath) => isTrackedPathSpec(filePath, trackedPathSpecs))
const untrackedPaths = filePaths.filter(
(filePath) => !isTrackedPathSpec(filePath, trackedPathSpecs)
)
await removeSafeUntrackedDiscardTargets(
worktreePath,
untrackedPaths,
(targetPaths) => cleanUntrackedPaths(worktreePath, targetPaths, options),
async () => {
for (let i = 0; i < trackedPaths.length; i += BULK_CHUNK_SIZE) {
const chunk = trackedPaths.slice(i, i + BULK_CHUNK_SIZE)
await gitExecFileAsync(
['restore', '--worktree', '--source=HEAD', '--', ...chunk.map(literalPathspec)],
{
...gitOptionsForWorktree(worktreePath, options)
}
)
try {
const resolvedWorktree = path.resolve(worktreePath)
for (const filePath of filePaths) {
const resolvedTarget = path.resolve(worktreePath, filePath)
if (!isWithinWorktree(path, resolvedWorktree, resolvedTarget)) {
throw new Error(`Path "${filePath}" resolves outside the worktree`)
}
}
)
const trackedPathSpecs = await listTrackedPathSpecs(worktreePath, filePaths, options)
const trackedPaths = filePaths.filter((filePath) =>
isTrackedPathSpec(filePath, trackedPathSpecs)
)
const untrackedPaths = filePaths.filter(
(filePath) => !isTrackedPathSpec(filePath, trackedPathSpecs)
)
await removeSafeUntrackedDiscardTargets(
worktreePath,
untrackedPaths,
(targetPaths) => cleanUntrackedPaths(worktreePath, targetPaths, options),
async () => {
for (let i = 0; i < trackedPaths.length; i += BULK_CHUNK_SIZE) {
const chunk = trackedPaths.slice(i, i + BULK_CHUNK_SIZE)
await gitExecFileAsync(
['restore', '--worktree', '--source=HEAD', '--', ...chunk.map(literalPathspec)],
{
...gitOptionsForWorktree(worktreePath, options)
}
)
}
}
)
} finally {
gitDiffReadDedupe.clear()
}
}
export function isWithinWorktree(
@ -1574,15 +1675,20 @@ export async function bulkStageFiles(
filePaths: string[],
options: GitRuntimeOptions = {}
): Promise<void> {
gitDiffReadDedupe.clear()
if (filePaths.length === 0) {
return
}
for (let i = 0; i < filePaths.length; i += BULK_CHUNK_SIZE) {
const chunk = filePaths.slice(i, i + BULK_CHUNK_SIZE)
await gitExecFileAsync(
['add', '--', ...chunk.map(literalPathspec)],
gitOptionsForWorktree(worktreePath, options)
)
try {
for (let i = 0; i < filePaths.length; i += BULK_CHUNK_SIZE) {
const chunk = filePaths.slice(i, i + BULK_CHUNK_SIZE)
await gitExecFileAsync(
['add', '--', ...chunk.map(literalPathspec)],
gitOptionsForWorktree(worktreePath, options)
)
}
} finally {
gitDiffReadDedupe.clear()
}
}
@ -1594,13 +1700,18 @@ export async function bulkUnstageFiles(
filePaths: string[],
options: GitRuntimeOptions = {}
): Promise<void> {
gitDiffReadDedupe.clear()
if (filePaths.length === 0) {
return
}
for (let i = 0; i < filePaths.length; i += BULK_CHUNK_SIZE) {
const chunk = filePaths.slice(i, i + BULK_CHUNK_SIZE)
await gitExecFileAsync(['restore', '--staged', '--', ...chunk.map(literalPathspec)], {
...gitOptionsForWorktree(worktreePath, options)
})
try {
for (let i = 0; i < filePaths.length; i += BULK_CHUNK_SIZE) {
const chunk = filePaths.slice(i, i + BULK_CHUNK_SIZE)
await gitExecFileAsync(['restore', '--staged', '--', ...chunk.map(literalPathspec)], {
...gitOptionsForWorktree(worktreePath, options)
})
}
} finally {
gitDiffReadDedupe.clear()
}
}

274
src/main/providers/ssh-git-provider.test.ts
View File

@ -31,6 +31,14 @@ async function waitForRequestCount(mock: ReturnType<typeof vi.fn>, count: number
}
}
function deferredValue<T>(value: T): { promise: Promise<T>; resolve: () => void } {
let resolve!: (value: T) => void
const promise = new Promise<T>((innerResolve) => {
resolve = innerResolve
})
return { promise, resolve: () => resolve(value) }
}
describe('SshGitProvider', () => {
let mux: MockMultiplexer
let provider: SshGitProvider
@ -828,6 +836,272 @@ describe('SshGitProvider', () => {
expect(result).toEqual(diffs)
})
it('coalesces concurrent identical diff RPCs while in flight', async () => {
const diff = {
kind: 'text',
originalContent: 'old',
modifiedContent: 'new',
originalIsBinary: false,
modifiedIsBinary: false
}
const pendingDiff = deferredValue(diff)
mux.request.mockReturnValue(pendingDiff.promise)
const reads = Array.from({ length: 8 }, () =>
provider.getDiff('/home/user/repo', 'src/file.ts', false, true)
)
await waitForRequestCount(mux.request, 1)
expect(mux.request).toHaveBeenCalledTimes(1)
pendingDiff.resolve()
await expect(Promise.all(reads)).resolves.toEqual(Array(8).fill(diff))
mux.request.mockReset()
const branchDiffs = [diff]
const pendingBranchDiff = deferredValue(branchDiffs)
mux.request.mockReturnValue(pendingBranchDiff.promise)
const branchReads = Array.from({ length: 8 }, () =>
provider.getBranchDiff('/home/user/repo', 'main', {
includePatch: true,
filePath: 'src/file.ts'
})
)
await waitForRequestCount(mux.request, 1)
expect(mux.request).toHaveBeenCalledTimes(1)
pendingBranchDiff.resolve()
await expect(Promise.all(branchReads)).resolves.toEqual(Array(8).fill(branchDiffs))
mux.request.mockReset()
const pendingCommitDiff = deferredValue(diff)
mux.request.mockReturnValue(pendingCommitDiff.promise)
const commitReads = Array.from({ length: 8 }, () =>
provider.getCommitDiff('/home/user/repo', {
commitOid: 'c'.repeat(40),
parentOid: 'b'.repeat(40),
filePath: 'src/file.ts'
})
)
await waitForRequestCount(mux.request, 1)
expect(mux.request).toHaveBeenCalledTimes(1)
pendingCommitDiff.resolve()
await expect(Promise.all(commitReads)).resolves.toEqual(Array(8).fill(diff))
})
it('retries diff RPCs after an in-flight rejection settles', async () => {
const failure = new Error('transient relay failure')
mux.request.mockRejectedValueOnce(failure)
const firstBurst = Array.from({ length: 8 }, () =>
provider.getDiff('/home/user/repo', 'src/file.ts', false, true)
)
await expect(Promise.all(firstBurst)).rejects.toThrow('transient relay failure')
expect(mux.request).toHaveBeenCalledTimes(1)
const diff = {
kind: 'text',
originalContent: 'old',
modifiedContent: 'new',
originalIsBinary: false,
modifiedIsBinary: false
}
mux.request.mockResolvedValueOnce(diff)
await expect(provider.getDiff('/home/user/repo', 'src/file.ts', false, true)).resolves.toBe(
diff
)
expect(mux.request).toHaveBeenCalledTimes(2)
})
it('clears pending diff RPCs when status runs', async () => {
const diff = {
kind: 'text',
originalContent: 'old',
modifiedContent: 'new',
originalIsBinary: false,
modifiedIsBinary: false
}
const pendingDiff = deferredValue(diff)
mux.request.mockReturnValueOnce(pendingDiff.promise)
const first = provider.getDiff('/home/user/repo', 'src/file.ts', false, true)
await waitForRequestCount(mux.request, 1)
mux.request.mockResolvedValueOnce({ entries: [], conflictOperation: 'unknown' })
await provider.getStatus('/home/user/repo')
mux.request.mockResolvedValueOnce(diff)
const second = provider.getDiff('/home/user/repo', 'src/file.ts', false, true)
pendingDiff.resolve()
await expect(Promise.all([first, second])).resolves.toEqual([diff, diff])
expect(mux.request).toHaveBeenCalledTimes(3)
})
it('clears pending diff RPCs when a mutation runs', async () => {
const diff = {
kind: 'text',
originalContent: 'old',
modifiedContent: 'new',
originalIsBinary: false,
modifiedIsBinary: false
}
const pendingDiff = deferredValue(diff)
mux.request.mockReturnValueOnce(pendingDiff.promise)
const first = provider.getDiff('/home/user/repo', 'src/file.ts', false, true)
await waitForRequestCount(mux.request, 1)
mux.request.mockResolvedValueOnce(undefined)
await provider.stageFile('/home/user/repo', 'src/file.ts')
mux.request.mockResolvedValueOnce(diff)
const second = provider.getDiff('/home/user/repo', 'src/file.ts', false, true)
pendingDiff.resolve()
await expect(Promise.all([first, second])).resolves.toEqual([diff, diff])
expect(mux.request).toHaveBeenCalledTimes(3)
})
it('clears pending branch diff RPCs when a ref-moving provider operation runs', async () => {
const diff = {
kind: 'text',
originalContent: 'old',
modifiedContent: 'new',
originalIsBinary: false,
modifiedIsBinary: false
}
const pendingDiff = deferredValue([diff])
mux.request.mockReturnValueOnce(pendingDiff.promise)
const first = provider.getBranchDiff('/home/user/repo', 'origin/main', {
includePatch: true,
filePath: 'src/file.ts'
})
await waitForRequestCount(mux.request, 1)
mux.request.mockResolvedValueOnce(undefined)
await provider.fetchRemoteTrackingRef(
'/home/user/repo',
'origin',
'main',
'refs/remotes/origin/main'
)
mux.request.mockResolvedValueOnce([diff])
const second = provider.getBranchDiff('/home/user/repo', 'origin/main', {
includePatch: true,
filePath: 'src/file.ts'
})
pendingDiff.resolve()
await expect(Promise.all([first, second])).resolves.toEqual([[diff], [diff]])
expect(mux.request).toHaveBeenCalledTimes(3)
})
it('coalesces logically identical branch and commit diff RPC args regardless of property order', async () => {
const diff = {
kind: 'text',
originalContent: 'old',
modifiedContent: 'new',
originalIsBinary: false,
modifiedIsBinary: false
}
mux.request.mockResolvedValue([diff])
await Promise.all([
provider.getBranchDiff('/home/user/repo', 'main', {
includePatch: true,
filePath: 'src/file.ts',
oldPath: 'src/old-file.ts'
}),
provider.getBranchDiff('/home/user/repo', 'main', {
oldPath: 'src/old-file.ts',
filePath: 'src/file.ts',
includePatch: true
})
])
expect(mux.request).toHaveBeenCalledTimes(1)
mux.request.mockReset()
mux.request.mockResolvedValue(diff)
await Promise.all([
provider.getCommitDiff('/home/user/repo', {
commitOid: 'c'.repeat(40),
parentOid: 'b'.repeat(40),
filePath: 'src/file.ts',
oldPath: 'src/old-file.ts'
}),
provider.getCommitDiff('/home/user/repo', {
oldPath: 'src/old-file.ts',
filePath: 'src/file.ts',
parentOid: 'b'.repeat(40),
commitOid: 'c'.repeat(40)
})
])
expect(mux.request).toHaveBeenCalledTimes(1)
})
it('keeps distinct diff RPC keys independent', async () => {
const diff = {
kind: 'text',
originalContent: 'old',
modifiedContent: 'new',
originalIsBinary: false,
modifiedIsBinary: false
}
mux.request.mockResolvedValue(diff)
await Promise.all([
provider.getDiff('/home/user/repo', 'src/file.ts', false, false),
provider.getDiff('/home/user/repo', 'src/file.ts', true, false),
provider.getDiff('/home/user/repo', 'src/file.ts', false, true),
provider.getBranchDiff('/home/user/repo', 'main', {
includePatch: true,
filePath: 'src/file.ts'
}),
provider.getBranchDiff('/home/user/repo', 'main', {
includePatch: false,
filePath: 'src/file.ts'
}),
provider.getBranchDiff('/home/user/repo', 'main', {
includePatch: true,
filePath: 'src/file.ts',
oldPath: 'src/old-file.ts'
}),
provider.getBranchDiff('/home/user/repo', 'develop', {
includePatch: true,
filePath: 'src/file.ts'
}),
provider.getCommitDiff('/home/user/repo', {
commitOid: 'c'.repeat(40),
parentOid: 'b'.repeat(40),
filePath: 'src/file.ts'
}),
provider.getCommitDiff('/home/user/repo', {
commitOid: 'c'.repeat(40),
parentOid: 'a'.repeat(40),
filePath: 'src/file.ts'
}),
provider.getCommitDiff('/home/user/repo', {
commitOid: 'c'.repeat(40),
parentOid: 'b'.repeat(40),
filePath: 'src/file.ts',
oldPath: 'src/old-file.ts'
})
])
expect(mux.request).toHaveBeenCalledTimes(10)
})
it('listWorktrees sends git.listWorktrees request', async () => {
const worktrees = [
{

235
src/main/providers/ssh-git-provider.ts
View File

@ -28,6 +28,7 @@ import {
describeMaxBufferOverflowError,
isMaxBufferOverflowError
} from '../git/max-buffer-overflow'
import { InFlightPromiseDedupe, stableInFlightKey } from '../../shared/in-flight-promise-dedupe'
type NonInteractiveExecQueueEntry = {
started: boolean
@ -58,9 +59,22 @@ function filterUntrackedPorcelainStatus(stdout: string | undefined): string | un
}
export class SshGitProvider implements IGitProvider {
private readonly gitDiffReadDedupe = new InFlightPromiseDedupe<GitDiffResult | GitDiffResult[]>()
private connectionId: string
private mux: SshChannelMultiplexer
private nonInteractiveExecQueues = new Map<string, NonInteractiveExecQueueEntry[]>()
private async runWithDiffDedupeClear<T>(run: () => Promise<T>): Promise<T> {
// Why: git mutations can stale both existing and concurrently-started diff reads.
// Clear before and after so later reads never join pre-mutation work.
this.gitDiffReadDedupe.clear()
try {
return await run()
} finally {
this.gitDiffReadDedupe.clear()
}
}
private loggedWorktreeIsCleanFallback = false
constructor(
@ -84,6 +98,7 @@ export class SshGitProvider implements IGitProvider {
worktreePath: string,
options?: GitProviderStatusOptions
): Promise<GitStatusResult> {
this.gitDiffReadDedupe.clear()
const includeIgnoredArgs = options?.includeIgnored ? { includeIgnored: true } : {}
const upstreamCacheBypassArgs = options?.bypassEffectiveUpstreamNegativeCache
? { bypassEffectiveUpstreamNegativeCache: true }
@ -116,10 +131,13 @@ export class SshGitProvider implements IGitProvider {
worktreePath: string,
message: string
): Promise<{ success: boolean; error?: string }> {
return (await this.mux.request('git.commit', {
worktreePath,
message
})) as { success: boolean; error?: string }
return this.runWithDiffDedupeClear(
async () =>
(await this.mux.request('git.commit', {
worktreePath,
message
})) as { success: boolean; error?: string }
)
}
async getStagedCommitContext(worktreePath: string): Promise<CommitMessageDraftContext | null> {
@ -321,36 +339,70 @@ export class SshGitProvider implements IGitProvider {
staged: boolean,
compareAgainstHead?: boolean
): Promise<GitDiffResult> {
return (await this.mux.request('git.diff', {
worktreePath,
filePath,
staged,
compareAgainstHead
})) as GitDiffResult
return this.gitDiffReadDedupe.run(
stableInFlightKey(['diff', worktreePath, filePath, staged, compareAgainstHead]),
async () =>
(await this.mux.request('git.diff', {
worktreePath,
filePath,
staged,
compareAgainstHead
})) as GitDiffResult
) as Promise<GitDiffResult>
}
async stageFile(worktreePath: string, filePath: string): Promise<void> {
await this.mux.request('git.stage', { worktreePath, filePath })
this.gitDiffReadDedupe.clear()
try {
await this.mux.request('git.stage', { worktreePath, filePath })
} finally {
this.gitDiffReadDedupe.clear()
}
}
async unstageFile(worktreePath: string, filePath: string): Promise<void> {
await this.mux.request('git.unstage', { worktreePath, filePath })
this.gitDiffReadDedupe.clear()
try {
await this.mux.request('git.unstage', { worktreePath, filePath })
} finally {
this.gitDiffReadDedupe.clear()
}
}
async bulkStageFiles(worktreePath: string, filePaths: string[]): Promise<void> {
await this.mux.request('git.bulkStage', { worktreePath, filePaths })
this.gitDiffReadDedupe.clear()
try {
await this.mux.request('git.bulkStage', { worktreePath, filePaths })
} finally {
this.gitDiffReadDedupe.clear()
}
}
async bulkUnstageFiles(worktreePath: string, filePaths: string[]): Promise<void> {
await this.mux.request('git.bulkUnstage', { worktreePath, filePaths })
this.gitDiffReadDedupe.clear()
try {
await this.mux.request('git.bulkUnstage', { worktreePath, filePaths })
} finally {
this.gitDiffReadDedupe.clear()
}
}
async discardChanges(worktreePath: string, filePath: string): Promise<void> {
await this.mux.request('git.discard', { worktreePath, filePath })
this.gitDiffReadDedupe.clear()
try {
await this.mux.request('git.discard', { worktreePath, filePath })
} finally {
this.gitDiffReadDedupe.clear()
}
}
async bulkDiscardChanges(worktreePath: string, filePaths: string[]): Promise<void> {
await this.mux.request('git.bulkDiscard', { worktreePath, filePaths })
this.gitDiffReadDedupe.clear()
try {
await this.mux.request('git.bulkDiscard', { worktreePath, filePaths })
} finally {
this.gitDiffReadDedupe.clear()
}
}
async detectConflictOperation(worktreePath: string): Promise<GitConflictOperation> {
@ -360,15 +412,21 @@ export class SshGitProvider implements IGitProvider {
}
async abortMerge(worktreePath: string): Promise<void> {
await this.mux.request('git.abortMerge', { worktreePath })
await this.runWithDiffDedupeClear(async () => {
await this.mux.request('git.abortMerge', { worktreePath })
})
}
async abortRebase(worktreePath: string): Promise<void> {
await this.mux.request('git.abortRebase', { worktreePath })
await this.runWithDiffDedupeClear(async () => {
await this.mux.request('git.abortRebase', { worktreePath })
})
}
async checkoutBranch(worktreePath: string, branch: string): Promise<void> {
await this.mux.request('git.checkout', { worktreePath, branch })
await this.runWithDiffDedupeClear(async () => {
await this.mux.request('git.checkout', { worktreePath, branch })
})
}
async listLocalBranches(
@ -410,41 +468,54 @@ export class SshGitProvider implements IGitProvider {
pushTarget?: GitPushTarget,
options: { forceWithLease?: boolean } = {}
): Promise<void> {
await this.mux.request('git.push', {
worktreePath,
publish,
pushTarget,
...(options.forceWithLease === true ? { forceWithLease: true } : {})
await this.runWithDiffDedupeClear(async () => {
await this.mux.request('git.push', {
worktreePath,
publish,
pushTarget,
...(options.forceWithLease === true ? { forceWithLease: true } : {})
})
})
}
async pullBranch(worktreePath: string, pushTarget?: GitPushTarget): Promise<void> {
await this.mux.request('git.pull', { worktreePath, ...(pushTarget ? { pushTarget } : {}) })
await this.runWithDiffDedupeClear(async () => {
await this.mux.request('git.pull', { worktreePath, ...(pushTarget ? { pushTarget } : {}) })
})
}
async fastForwardBranch(worktreePath: string, pushTarget?: GitPushTarget): Promise<void> {
await this.mux.request('git.fastForward', {
worktreePath,
...(pushTarget ? { pushTarget } : {})
await this.runWithDiffDedupeClear(async () => {
await this.mux.request('git.fastForward', {
worktreePath,
...(pushTarget ? { pushTarget } : {})
})
})
}
async rebaseFromBase(worktreePath: string, baseRef: string): Promise<void> {
await this.mux.request('git.rebaseFromBase', { worktreePath, baseRef })
await this.runWithDiffDedupeClear(async () => {
await this.mux.request('git.rebaseFromBase', { worktreePath, baseRef })
})
}
async fetchRemote(worktreePath: string, pushTarget?: GitPushTarget): Promise<void> {
await this.mux.request('git.fetch', { worktreePath, ...(pushTarget ? { pushTarget } : {}) })
await this.runWithDiffDedupeClear(async () => {
await this.mux.request('git.fetch', { worktreePath, ...(pushTarget ? { pushTarget } : {}) })
})
}
async syncForkDefaultBranch(
worktreePath: string,
expectedUpstream: GitForkSyncExpectedUpstream
): Promise<GitForkSyncResult> {
return (await this.mux.request('git.forkSync', {
worktreePath,
...(expectedUpstream ? { expectedUpstream } : {})
})) as GitForkSyncResult
return this.runWithDiffDedupeClear(
async () =>
(await this.mux.request('git.forkSync', {
worktreePath,
...(expectedUpstream ? { expectedUpstream } : {})
})) as GitForkSyncResult
)
}
async fetchRemoteTrackingRef(
@ -453,11 +524,13 @@ export class SshGitProvider implements IGitProvider {
branch: string,
ref: string
): Promise<void> {
await this.mux.request('git.fetchRemoteTrackingRef', {
worktreePath,
remote,
branch,
ref
await this.runWithDiffDedupeClear(async () => {
await this.mux.request('git.fetchRemoteTrackingRef', {
worktreePath,
remote,
branch,
ref
})
})
}
@ -466,10 +539,12 @@ export class SshGitProvider implements IGitProvider {
remote: string,
mrIid: number
): Promise<void> {
await this.mux.request('git.fetchGitLabMergeRequestHead', {
worktreePath,
remote,
mrIid
await this.runWithDiffDedupeClear(async () => {
await this.mux.request('git.fetchGitLabMergeRequestHead', {
worktreePath,
remote,
mrIid
})
})
}
@ -478,21 +553,44 @@ export class SshGitProvider implements IGitProvider {
baseRef: string,
options?: { includePatch?: boolean; filePath?: string; oldPath?: string }
): Promise<GitDiffResult[]> {
return (await this.mux.request('git.branchDiff', {
worktreePath,
baseRef,
...options
})) as GitDiffResult[]
const keyOptions = options ?? {}
return this.gitDiffReadDedupe.run(
stableInFlightKey([
'branchDiff',
worktreePath,
baseRef,
keyOptions.includePatch ?? null,
keyOptions.filePath ?? null,
keyOptions.oldPath ?? null
]),
async () =>
(await this.mux.request('git.branchDiff', {
worktreePath,
baseRef,
...options
})) as GitDiffResult[]
) as Promise<GitDiffResult[]>
}
async getCommitDiff(
worktreePath: string,
args: { commitOid: string; parentOid?: string | null; filePath: string; oldPath?: string }
): Promise<GitDiffResult> {
return (await this.mux.request('git.commitDiff', {
worktreePath,
...args
})) as GitDiffResult
return this.gitDiffReadDedupe.run(
stableInFlightKey([
'commitDiff',
worktreePath,
args.commitOid,
args.parentOid ?? null,
args.filePath,
args.oldPath ?? null
]),
async () =>
(await this.mux.request('git.commitDiff', {
worktreePath,
...args
})) as GitDiffResult
) as Promise<GitDiffResult>
}
async listWorktrees(
@ -514,11 +612,13 @@ export class SshGitProvider implements IGitProvider {
targetDir: string,
options?: { base?: string; checkoutExistingBranch?: boolean; noCheckout?: boolean }
): Promise<void> {
await this.mux.request('git.addWorktree', {
repoPath,
branchName,
targetDir,
...options
await this.runWithDiffDedupeClear(async () => {
await this.mux.request('git.addWorktree', {
repoPath,
branchName,
targetDir,
...options
})
})
}
@ -527,11 +627,14 @@ export class SshGitProvider implements IGitProvider {
force?: boolean,
options?: { deleteBranch?: boolean; forceBranchDelete?: boolean }
): Promise<RemoveWorktreeResult> {
return ((await this.mux.request('git.removeWorktree', {
worktreePath,
force,
...options
})) ?? {}) as RemoveWorktreeResult
return this.runWithDiffDedupeClear(
async () =>
((await this.mux.request('git.removeWorktree', {
worktreePath,
force,
...options
})) ?? {}) as RemoveWorktreeResult
)
}
async worktreeIsClean(
@ -583,11 +686,15 @@ export class SshGitProvider implements IGitProvider {
ownerWorktreePath?: string
checkOnly?: boolean
}): Promise<void> {
await this.mux.request('git.refreshLocalBaseRefForWorktreeCreate', args)
await this.runWithDiffDedupeClear(async () => {
await this.mux.request('git.refreshLocalBaseRefForWorktreeCreate', args)
})
}
async renameCurrentBranch(worktreePath: string, newBranch: string): Promise<void> {
await this.mux.request('git.renameCurrentBranch', { worktreePath, newBranch })
await this.runWithDiffDedupeClear(async () => {
await this.mux.request('git.renameCurrentBranch', { worktreePath, newBranch })
})
}
async exec(

407
src/relay/git-handler.test.ts
View File

@ -19,6 +19,37 @@ import {
type RelayDispatcher
} from './git-handler-test-setup'
type GitBufferSpyTarget = {
gitBuffer(args: string[], cwd: string): Promise<Buffer>
}
type GitSpyTarget = {
git(args: string[], cwd: string): Promise<{ stdout: string; stderr: string }>
}
function deferredRelayBuffer(content: string): {
promise: Promise<Buffer>
resolve: () => void
} {
let resolve!: (value: Buffer) => void
const promise = new Promise<Buffer>((innerResolve) => {
resolve = innerResolve
})
return {
promise,
resolve: () => resolve(Buffer.from(content))
}
}
async function waitForSpyCalls(mock: ReturnType<typeof vi.fn>, calls: number): Promise<void> {
for (let i = 0; i < 20; i++) {
if (mock.mock.calls.length >= calls) {
return
}
await new Promise<void>((resolve) => setTimeout(resolve, 0))
}
}
describe('GitHandler', () => {
let dispatcher: MockDispatcher
let handler: GitHandler
@ -830,6 +861,382 @@ describe('GitHandler', () => {
})
describe('branchDiff', () => {
it('coalesces concurrent identical git.diff reads while in flight and reads fresh after settle', async () => {
const leftBlob = deferredRelayBuffer('left\n')
const rightBlob = deferredRelayBuffer('right\n')
const pendingBuffers = [leftBlob, rightBlob]
const gitBufferSpy = vi
.spyOn(handler as unknown as GitBufferSpyTarget, 'gitBuffer')
.mockImplementation(async () => pendingBuffers.shift()!.promise)
const reads = Array.from({ length: 8 }, () =>
dispatcher.callRequest('git.diff', {
worktreePath: tmpDir,
filePath: 'src/file.ts',
staged: true
})
)
await waitForSpyCalls(gitBufferSpy, 1)
leftBlob.resolve()
await waitForSpyCalls(gitBufferSpy, 2)
rightBlob.resolve()
await Promise.all(reads)
expect(gitBufferSpy).toHaveBeenCalledTimes(2)
gitBufferSpy
.mockResolvedValueOnce(Buffer.from('fresh-left\n'))
.mockResolvedValueOnce(Buffer.from('fresh-right\n'))
await dispatcher.callRequest('git.diff', {
worktreePath: tmpDir,
filePath: 'src/file.ts',
staged: true
})
expect(gitBufferSpy).toHaveBeenCalledTimes(4)
})
it('clears pending git.diff reads when status runs', async () => {
const firstBlob = deferredRelayBuffer('left\n')
const secondBlob = deferredRelayBuffer('fresh-left\n')
const pendingBuffers = [firstBlob, secondBlob]
const gitBufferSpy = vi
.spyOn(handler as unknown as GitBufferSpyTarget, 'gitBuffer')
.mockImplementation(async () => pendingBuffers.shift()!.promise)
const gitSpy = vi
.spyOn(handler as unknown as GitSpyTarget, 'git')
.mockResolvedValue({ stdout: '', stderr: '' })
const first = dispatcher.callRequest('git.diff', {
worktreePath: tmpDir,
filePath: 'src/file.ts',
staged: false
})
await waitForSpyCalls(gitBufferSpy, 1)
await dispatcher.callRequest('git.status', { worktreePath: tmpDir })
const second = dispatcher.callRequest('git.diff', {
worktreePath: tmpDir,
filePath: 'src/file.ts',
staged: false
})
await waitForSpyCalls(gitBufferSpy, 2)
firstBlob.resolve()
secondBlob.resolve()
await Promise.all([first, second])
expect(gitBufferSpy).toHaveBeenCalledTimes(2)
expect(gitSpy).toHaveBeenCalled()
})
it('clears pending git.diff reads when a mutation runs', async () => {
const firstBlob = deferredRelayBuffer('left\n')
const secondBlob = deferredRelayBuffer('fresh-left\n')
const pendingBuffers = [firstBlob, secondBlob]
const gitBufferSpy = vi
.spyOn(handler as unknown as GitBufferSpyTarget, 'gitBuffer')
.mockImplementation(async () => pendingBuffers.shift()!.promise)
const gitSpy = vi
.spyOn(handler as unknown as GitSpyTarget, 'git')
.mockResolvedValue({ stdout: '', stderr: '' })
const first = dispatcher.callRequest('git.diff', {
worktreePath: tmpDir,
filePath: 'src/file.ts',
staged: false
})
await waitForSpyCalls(gitBufferSpy, 1)
await dispatcher.callRequest('git.stage', { worktreePath: tmpDir, filePath: 'src/file.ts' })
const second = dispatcher.callRequest('git.diff', {
worktreePath: tmpDir,
filePath: 'src/file.ts',
staged: false
})
await waitForSpyCalls(gitBufferSpy, 2)
firstBlob.resolve()
secondBlob.resolve()
await Promise.all([first, second])
expect(gitBufferSpy).toHaveBeenCalledTimes(2)
expect(gitSpy).toHaveBeenCalledWith(['add', '--', 'src/file.ts'], tmpDir)
})
it('clears pending git.diff reads when a narrow ref fetch runs', async () => {
const firstBlob = deferredRelayBuffer('left\n')
const secondBlob = deferredRelayBuffer('fresh-left\n')
const pendingBuffers = [firstBlob, secondBlob]
const gitBufferSpy = vi
.spyOn(handler as unknown as GitBufferSpyTarget, 'gitBuffer')
.mockImplementation(async () => pendingBuffers.shift()!.promise)
const gitSpy = vi
.spyOn(handler as unknown as GitSpyTarget, 'git')
.mockImplementation(async (args: string[]) => {
if (args[0] === 'remote') {
return { stdout: 'origin\n', stderr: '' }
}
return { stdout: '', stderr: '' }
})
const first = dispatcher.callRequest('git.diff', {
worktreePath: tmpDir,
filePath: 'src/file.ts',
staged: false
})
await waitForSpyCalls(gitBufferSpy, 1)
await dispatcher.callRequest('git.fetchRemoteTrackingRef', {
worktreePath: tmpDir,
remote: 'origin',
branch: 'main',
ref: 'refs/remotes/origin/main'
})
const second = dispatcher.callRequest('git.diff', {
worktreePath: tmpDir,
filePath: 'src/file.ts',
staged: false
})
await waitForSpyCalls(gitBufferSpy, 2)
firstBlob.resolve()
secondBlob.resolve()
await Promise.all([first, second])
expect(gitBufferSpy).toHaveBeenCalledTimes(2)
expect(gitSpy).toHaveBeenCalledWith(
['fetch', '--no-tags', 'origin', '+refs/heads/main:refs/remotes/origin/main'],
tmpDir
)
})
it('coalesces concurrent identical git.branchDiff reads while in flight', async () => {
const gitSpy = vi
.spyOn(handler as unknown as GitSpyTarget, 'git')
.mockImplementation(async (args: string[]) => {
if (args[0] === 'rev-parse' && args.includes('HEAD')) {
return { stdout: `${'c'.repeat(40)}\n`, stderr: '' }
}
if (args[0] === 'rev-parse') {
return { stdout: `${'b'.repeat(40)}\n`, stderr: '' }
}
if (args[0] === 'merge-base') {
return { stdout: `${'a'.repeat(40)}\n`, stderr: '' }
}
if (args.includes('--name-status')) {
return { stdout: 'M\tsrc/file.ts\n', stderr: '' }
}
throw new Error(`unexpected git args: ${args.join(' ')}`)
})
const leftBlob = deferredRelayBuffer('left\n')
const rightBlob = deferredRelayBuffer('right\n')
const pendingBuffers = [leftBlob, rightBlob]
const gitBufferSpy = vi
.spyOn(handler as unknown as GitBufferSpyTarget, 'gitBuffer')
.mockImplementation(async () => pendingBuffers.shift()!.promise)
const reads = Array.from({ length: 8 }, () =>
dispatcher.callRequest('git.branchDiff', {
worktreePath: tmpDir,
baseRef: 'main',
includePatch: true,
filePath: 'src/file.ts'
})
)
await waitForSpyCalls(gitBufferSpy, 1)
leftBlob.resolve()
await waitForSpyCalls(gitBufferSpy, 2)
rightBlob.resolve()
await Promise.all(reads)
expect(gitBufferSpy).toHaveBeenCalledTimes(2)
expect(gitSpy).toHaveBeenCalledTimes(4)
})
it('coalesces concurrent identical git.commitDiff reads while in flight', async () => {
const leftBlob = deferredRelayBuffer('left\n')
const rightBlob = deferredRelayBuffer('right\n')
const pendingBuffers = [leftBlob, rightBlob]
const gitBufferSpy = vi
.spyOn(handler as unknown as GitBufferSpyTarget, 'gitBuffer')
.mockImplementation(async () => pendingBuffers.shift()!.promise)
const reads = Array.from({ length: 8 }, () =>
dispatcher.callRequest('git.commitDiff', {
worktreePath: tmpDir,
commitOid: 'c'.repeat(40),
parentOid: 'b'.repeat(40),
filePath: 'src/file.ts'
})
)
await waitForSpyCalls(gitBufferSpy, 1)
leftBlob.resolve()
await waitForSpyCalls(gitBufferSpy, 2)
rightBlob.resolve()
await Promise.all(reads)
expect(gitBufferSpy).toHaveBeenCalledTimes(2)
})
it('coalesces parentless root git.commitDiff reads without a left-side blob', async () => {
const rightBlob = deferredRelayBuffer('right\n')
const gitBufferSpy = vi
.spyOn(handler as unknown as GitBufferSpyTarget, 'gitBuffer')
.mockImplementation(async () => rightBlob.promise)
const reads = Array.from({ length: 8 }, () =>
dispatcher.callRequest('git.commitDiff', {
worktreePath: tmpDir,
commitOid: 'c'.repeat(40),
parentOid: null,
filePath: 'src/file.ts'
})
)
await waitForSpyCalls(gitBufferSpy, 1)
rightBlob.resolve()
await Promise.all(reads)
expect(gitBufferSpy).toHaveBeenCalledTimes(1)
})
it('keeps distinct relay diff keys independent', async () => {
const gitBufferSpy = vi
.spyOn(handler as unknown as GitBufferSpyTarget, 'gitBuffer')
.mockResolvedValue(Buffer.from('blob\n'))
await Promise.all([
dispatcher.callRequest('git.diff', {
worktreePath: tmpDir,
filePath: 'src/file.ts',
staged: true
}),
dispatcher.callRequest('git.diff', {
worktreePath: tmpDir,
filePath: 'src/file.ts',
staged: false,
compareAgainstHead: true
})
])
expect(gitBufferSpy).toHaveBeenCalledTimes(3)
gitBufferSpy.mockClear()
const gitSpy = vi
.spyOn(handler as unknown as GitSpyTarget, 'git')
.mockImplementation(async (args: string[]) => {
if (args[0] === 'rev-parse' && args.includes('HEAD')) {
return { stdout: `${'c'.repeat(40)}\n`, stderr: '' }
}
if (args[0] === 'rev-parse' && args.includes('develop')) {
return { stdout: `${'d'.repeat(40)}\n`, stderr: '' }
}
if (args[0] === 'rev-parse') {
return { stdout: `${'b'.repeat(40)}\n`, stderr: '' }
}
if (args[0] === 'merge-base' && args.includes('d'.repeat(40))) {
return { stdout: `${'e'.repeat(40)}\n`, stderr: '' }
}
if (args[0] === 'merge-base') {
return { stdout: `${'a'.repeat(40)}\n`, stderr: '' }
}
if (args.includes('--name-status')) {
return { stdout: 'M\tsrc/file.ts\n', stderr: '' }
}
throw new Error(`unexpected git args: ${args.join(' ')}`)
})
await Promise.all([
dispatcher.callRequest('git.branchDiff', {
worktreePath: tmpDir,
baseRef: 'main',
includePatch: true,
filePath: 'src/file.ts'
}),
dispatcher.callRequest('git.branchDiff', {
worktreePath: tmpDir,
baseRef: 'main',
includePatch: false,
filePath: 'src/file.ts'
}),
dispatcher.callRequest('git.branchDiff', {
worktreePath: tmpDir,
baseRef: 'develop',
includePatch: true,
filePath: 'src/file.ts'
})
])
expect(gitSpy).toHaveBeenCalledTimes(12)
expect(gitBufferSpy).toHaveBeenCalledTimes(4)
gitBufferSpy.mockClear()
await Promise.all([
dispatcher.callRequest('git.commitDiff', {
worktreePath: tmpDir,
commitOid: 'c'.repeat(40),
parentOid: 'b'.repeat(40),
filePath: 'src/file.ts'
}),
dispatcher.callRequest('git.commitDiff', {
worktreePath: tmpDir,
commitOid: 'c'.repeat(40),
parentOid: 'a'.repeat(40),
filePath: 'src/file.ts'
}),
dispatcher.callRequest('git.commitDiff', {
worktreePath: tmpDir,
commitOid: 'c'.repeat(40),
parentOid: 'b'.repeat(40),
filePath: 'src/file.ts',
oldPath: 'src/old-a.ts'
}),
dispatcher.callRequest('git.commitDiff', {
worktreePath: tmpDir,
commitOid: 'c'.repeat(40),
parentOid: 'b'.repeat(40),
filePath: 'src/file.ts',
oldPath: 'src/old-b.ts'
})
])
expect(gitBufferSpy).toHaveBeenCalledTimes(8)
})
it('retries relay diff reads after an in-flight rejection settles', async () => {
const invalidRequest = {
worktreePath: tmpDir,
commitOid: 'not-a-full-oid',
parentOid: 'b'.repeat(40),
filePath: 'src/file.ts'
}
const first = dispatcher.callRequest('git.commitDiff', invalidRequest)
const firstBurst = [
first,
...Array.from({ length: 7 }, () => dispatcher.callRequest('git.commitDiff', invalidRequest))
]
await expect(Promise.all(firstBurst)).rejects.toThrow(
'commitOid must be a full git object id'
)
const retry = dispatcher.callRequest('git.commitDiff', invalidRequest)
expect(retry).not.toBe(first)
await expect(retry).rejects.toThrow('commitOid must be a full git object id')
})
// Why: regression for issue #1503 on git.branchDiff. The branchCompare test
// covers loadBranchChanges in git-handler.ts, but branchDiffEntries in
// git-handler-ops.ts is a separate code path that also passes

631
src/relay/git-handler.ts
View File

@ -46,6 +46,7 @@ import {
} from '../shared/git-discard-path-safety'
import { getGitCloneFailureMessage } from '../shared/git-clone-failure-message'
import { syncForkDefaultBranch, validateGitForkSyncExpectedUpstream } from '../shared/git-fork-sync'
import { InFlightPromiseDedupe, stableInFlightKey } from '../shared/in-flight-promise-dedupe'
const execFileAsync = promisify(execFile)
const MAX_GIT_BUFFER = 10 * 1024 * 1024
@ -88,6 +89,7 @@ function execFileWithStdin(
export class GitHandler {
private dispatcher: RelayDispatcher
private readonly gitDiffReadDedupe = new InFlightPromiseDedupe<unknown>()
// Why: RelayContext is accepted for protocol back-compat (see
// docs/relay-fs-allowlist-removal.md) but no longer consulted on git ops.
@ -141,6 +143,17 @@ export class GitHandler {
this.dispatcher.onRequest('git.isGitRepo', (p) => this.isGitRepo(p))
}
private async runWithDiffDedupeClear<T>(run: () => Promise<T>): Promise<T> {
// Why: git mutations can stale both existing and concurrently-started diff reads.
// Clear before and after so later reads never join pre-mutation work.
this.gitDiffReadDedupe.clear()
try {
return await run()
} finally {
this.gitDiffReadDedupe.clear()
}
}
private async git(
args: string[],
cwd: string,
@ -187,6 +200,7 @@ export class GitHandler {
}
private async getStatus(params: Record<string, unknown>) {
this.gitDiffReadDedupe.clear()
return getStatusOp(this.git.bind(this), params)
}
@ -212,64 +226,110 @@ export class GitHandler {
if (rel === '..' || rel.startsWith(`..${path.sep}`) || path.isAbsolute(rel)) {
throw new Error(`Path "${filePath}" resolves outside the worktree`)
}
return computeDiff(
this.gitBuffer.bind(this),
worktreePath,
filePath,
params.staged as boolean,
params.compareAgainstHead as boolean | undefined
return this.gitDiffReadDedupe.run(
stableInFlightKey([
'diff',
worktreePath,
filePath,
params.staged as boolean,
params.compareAgainstHead as boolean | undefined
]),
() =>
computeDiff(
this.gitBuffer.bind(this),
worktreePath,
filePath,
params.staged as boolean,
params.compareAgainstHead as boolean | undefined
)
)
}
private async stage(params: Record<string, unknown>) {
this.gitDiffReadDedupe.clear()
const worktreePath = params.worktreePath as string
const filePath = params.filePath as string
await this.git(['add', '--', filePath], worktreePath)
try {
await this.git(['add', '--', filePath], worktreePath)
} finally {
this.gitDiffReadDedupe.clear()
}
}
private async commit(
params: Record<string, unknown>
): Promise<{ success: boolean; error?: string }> {
this.gitDiffReadDedupe.clear()
const worktreePath = params.worktreePath as string
const message = params.message as string
return commitChangesRelay(this.git.bind(this), worktreePath, message)
try {
return await commitChangesRelay(this.git.bind(this), worktreePath, message)
} finally {
this.gitDiffReadDedupe.clear()
}
}
private async unstage(params: Record<string, unknown>) {
this.gitDiffReadDedupe.clear()
const worktreePath = params.worktreePath as string
const filePath = params.filePath as string
await this.git(['restore', '--staged', '--', filePath], worktreePath)
try {
await this.git(['restore', '--staged', '--', filePath], worktreePath)
} finally {
this.gitDiffReadDedupe.clear()
}
}
private async bulkStage(params: Record<string, unknown>) {
this.gitDiffReadDedupe.clear()
const worktreePath = params.worktreePath as string
const filePaths = params.filePaths as string[]
for (let i = 0; i < filePaths.length; i += BULK_CHUNK_SIZE) {
const chunk = filePaths.slice(i, i + BULK_CHUNK_SIZE)
await this.git(['add', '--', ...chunk], worktreePath)
try {
for (let i = 0; i < filePaths.length; i += BULK_CHUNK_SIZE) {
const chunk = filePaths.slice(i, i + BULK_CHUNK_SIZE)
await this.git(['add', '--', ...chunk], worktreePath)
}
} finally {
this.gitDiffReadDedupe.clear()
}
}
private async bulkUnstage(params: Record<string, unknown>) {
this.gitDiffReadDedupe.clear()
const worktreePath = params.worktreePath as string
const filePaths = params.filePaths as string[]
for (let i = 0; i < filePaths.length; i += BULK_CHUNK_SIZE) {
const chunk = filePaths.slice(i, i + BULK_CHUNK_SIZE)
await this.git(['restore', '--staged', '--', ...chunk], worktreePath)
try {
for (let i = 0; i < filePaths.length; i += BULK_CHUNK_SIZE) {
const chunk = filePaths.slice(i, i + BULK_CHUNK_SIZE)
await this.git(['restore', '--staged', '--', ...chunk], worktreePath)
}
} finally {
this.gitDiffReadDedupe.clear()
}
}
private async abortMerge(params: Record<string, unknown>) {
this.gitDiffReadDedupe.clear()
const worktreePath = params.worktreePath as string
await this.git(['merge', '--abort'], worktreePath)
try {
await this.git(['merge', '--abort'], worktreePath)
} finally {
this.gitDiffReadDedupe.clear()
}
}
private async abortRebase(params: Record<string, unknown>) {
this.gitDiffReadDedupe.clear()
const worktreePath = params.worktreePath as string
await this.git(['rebase', '--abort'], worktreePath)
try {
await this.git(['rebase', '--abort'], worktreePath)
} finally {
this.gitDiffReadDedupe.clear()
}
}
private async checkout(params: Record<string, unknown>) {
this.gitDiffReadDedupe.clear()
const worktreePath = params.worktreePath as string
const branch = params.branch as string
// Defense-in-depth: reject option-like branch tokens (the RPC schema also
@ -279,8 +339,12 @@ export class GitHandler {
if (typeof branch !== 'string' || branch.length === 0 || branch.startsWith('-')) {
throw new Error('invalid_branch_name')
}
await this.git(['checkout', branch, '--'], worktreePath)
return { ok: true as const, branch }
try {
await this.git(['checkout', branch, '--'], worktreePath)
return { ok: true as const, branch }
} finally {
this.gitDiffReadDedupe.clear()
}
}
private async localBranches(params: Record<string, unknown>) {
@ -338,88 +402,96 @@ export class GitHandler {
}
private async discard(params: Record<string, unknown>) {
this.gitDiffReadDedupe.clear()
const worktreePath = params.worktreePath as string
const filePath = params.filePath as string
this.assertInWorktree(worktreePath, filePath)
let tracked = false
try {
await this.git(
['ls-files', '--error-unmatch', '--', this.literalPathspec(filePath)],
worktreePath
)
tracked = true
} catch {
// untracked
}
this.assertInWorktree(worktreePath, filePath)
if (tracked) {
await this.git(
['restore', '--worktree', '--source=HEAD', '--', this.literalPathspec(filePath)],
worktreePath
)
return
}
let tracked = false
try {
await this.git(
['ls-files', '--error-unmatch', '--', this.literalPathspec(filePath)],
worktreePath
)
tracked = true
} catch {
// untracked
}
await removeSafeUntrackedDiscardTarget(worktreePath, filePath, (targetPath) =>
this.cleanUntrackedPaths(worktreePath, [targetPath])
)
if (tracked) {
await this.git(
['restore', '--worktree', '--source=HEAD', '--', this.literalPathspec(filePath)],
worktreePath
)
return
}
await removeSafeUntrackedDiscardTarget(worktreePath, filePath, (targetPath) =>
this.cleanUntrackedPaths(worktreePath, [targetPath])
)
} finally {
this.gitDiffReadDedupe.clear()
}
}
private async bulkDiscard(params: Record<string, unknown>) {
this.gitDiffReadDedupe.clear()
const worktreePath = params.worktreePath as string
const filePaths = params.filePaths as string[]
if (filePaths.length === 0) {
return
}
try {
for (const filePath of filePaths) {
this.assertInWorktree(worktreePath, filePath)
}
for (const filePath of filePaths) {
this.assertInWorktree(worktreePath, filePath)
}
const trackedPathSpecs: string[] = []
for (let i = 0; i < filePaths.length; i += BULK_CHUNK_SIZE) {
const chunk = filePaths.slice(i, i + BULK_CHUNK_SIZE)
const { stdout } = await this.git(
['ls-files', '-z', '--', ...chunk.map((p) => this.literalPathspec(p))],
worktreePath
)
// Why: selecting a tracked directory can make `ls-files -z` return
// enough descendants for push(...split) to exceed the argument limit.
for (const trackedPathSpec of stdout.split('\0')) {
if (trackedPathSpec) {
trackedPathSpecs.push(trackedPathSpec)
}
}
}
const trackedPathSpecs: string[] = []
for (let i = 0; i < filePaths.length; i += BULK_CHUNK_SIZE) {
const chunk = filePaths.slice(i, i + BULK_CHUNK_SIZE)
const { stdout } = await this.git(
['ls-files', '-z', '--', ...chunk.map((p) => this.literalPathspec(p))],
worktreePath
const trackedPaths = filePaths.filter((filePath) =>
this.isTrackedPathSpec(filePath, trackedPathSpecs)
)
// Why: selecting a tracked directory can make `ls-files -z` return
// enough descendants for push(...split) to exceed the argument limit.
for (const trackedPathSpec of stdout.split('\0')) {
if (trackedPathSpec) {
trackedPathSpecs.push(trackedPathSpec)
const untrackedPaths = filePaths.filter(
(filePath) => !this.isTrackedPathSpec(filePath, trackedPathSpecs)
)
await removeSafeUntrackedDiscardTargets(
worktreePath,
untrackedPaths,
(targetPaths) => this.cleanUntrackedPaths(worktreePath, targetPaths),
async () => {
for (let i = 0; i < trackedPaths.length; i += BULK_CHUNK_SIZE) {
const chunk = trackedPaths.slice(i, i + BULK_CHUNK_SIZE)
await this.git(
[
'restore',
'--worktree',
'--source=HEAD',
'--',
...chunk.map((p) => this.literalPathspec(p))
],
worktreePath
)
}
}
}
)
} finally {
this.gitDiffReadDedupe.clear()
}
const trackedPaths = filePaths.filter((filePath) =>
this.isTrackedPathSpec(filePath, trackedPathSpecs)
)
const untrackedPaths = filePaths.filter(
(filePath) => !this.isTrackedPathSpec(filePath, trackedPathSpecs)
)
await removeSafeUntrackedDiscardTargets(
worktreePath,
untrackedPaths,
(targetPaths) => this.cleanUntrackedPaths(worktreePath, targetPaths),
async () => {
for (let i = 0; i < trackedPaths.length; i += BULK_CHUNK_SIZE) {
const chunk = trackedPaths.slice(i, i + BULK_CHUNK_SIZE)
await this.git(
[
'restore',
'--worktree',
'--source=HEAD',
'--',
...chunk.map((p) => this.literalPathspec(p))
],
worktreePath
)
}
}
)
}
private literalPathspec(filePath: string): string {
@ -524,177 +596,204 @@ export class GitHandler {
}
private async fetch(params: Record<string, unknown>) {
this.gitDiffReadDedupe.clear()
const worktreePath = params.worktreePath as string
try {
if (params.pushTarget !== undefined) {
assertGitPushTargetShape(params.pushTarget)
const pushTarget = params.pushTarget as GitPushTarget
await this.git(['check-ref-format', '--branch', pushTarget.branchName], worktreePath)
await this.git(['fetch', '--prune', pushTarget.remoteName], worktreePath)
return
try {
if (params.pushTarget !== undefined) {
assertGitPushTargetShape(params.pushTarget)
const pushTarget = params.pushTarget as GitPushTarget
await this.git(['check-ref-format', '--branch', pushTarget.branchName], worktreePath)
await this.git(['fetch', '--prune', pushTarget.remoteName], worktreePath)
return
}
await this.git(['fetch', '--prune'], worktreePath)
} catch (error) {
// Why: mirror the local gitFetch normalization so SSH users see the same
// actionable messages instead of raw git stderr (which varies across
// versions/locales and may embed credentials).
throw new Error(normalizeGitErrorMessage(error, 'fetch'))
}
await this.git(['fetch', '--prune'], worktreePath)
} catch (error) {
// Why: mirror the local gitFetch normalization so SSH users see the same
// actionable messages instead of raw git stderr (which varies across
// versions/locales and may embed credentials).
throw new Error(normalizeGitErrorMessage(error, 'fetch'))
} finally {
this.gitDiffReadDedupe.clear()
}
}
private async forkSync(params: Record<string, unknown>, context?: RequestContext) {
const worktreePath = params.worktreePath as string
const expectedUpstream = validateGitForkSyncExpectedUpstream(params.expectedUpstream, {
required: true
return this.runWithDiffDedupeClear(async () => {
const worktreePath = params.worktreePath as string
const expectedUpstream = validateGitForkSyncExpectedUpstream(params.expectedUpstream, {
required: true
})
const controller = new AbortController()
const abortFromContext = () => controller.abort()
if (context?.signal?.aborted) {
controller.abort()
} else {
context?.signal?.addEventListener('abort', abortFromContext, { once: true })
}
const timeout = setTimeout(() => controller.abort(), 60_000)
try {
return await syncForkDefaultBranch(
(args) =>
this.git(args, worktreePath, {
nonInteractive: true,
signal: controller.signal
}),
{ expectedUpstream }
)
} catch (error) {
throw new Error(normalizeGitErrorMessage(error, 'push'))
} finally {
clearTimeout(timeout)
context?.signal?.removeEventListener('abort', abortFromContext)
}
})
const controller = new AbortController()
const abortFromContext = () => controller.abort()
if (context?.signal?.aborted) {
controller.abort()
} else {
context?.signal?.addEventListener('abort', abortFromContext, { once: true })
}
const timeout = setTimeout(() => controller.abort(), 60_000)
try {
return await syncForkDefaultBranch(
(args) =>
this.git(args, worktreePath, {
nonInteractive: true,
signal: controller.signal
}),
{ expectedUpstream }
)
} catch (error) {
throw new Error(normalizeGitErrorMessage(error, 'push'))
} finally {
clearTimeout(timeout)
context?.signal?.removeEventListener('abort', abortFromContext)
}
}
private async fetchRemoteTrackingRef(params: Record<string, unknown>) {
this.gitDiffReadDedupe.clear()
const worktreePath = params.worktreePath as string
const remote = params.remote
const branch = params.branch
const ref = params.ref
if (typeof remote !== 'string' || typeof branch !== 'string' || typeof ref !== 'string') {
throw new Error('Invalid remote-tracking fetch request.')
}
if (remote.startsWith('-') || branch.startsWith('-')) {
throw new Error('Remote-tracking fetch inputs must not start with "-".')
}
if (ref !== `refs/remotes/${remote}/${branch}`) {
throw new Error('Remote-tracking ref does not match the requested remote and branch.')
}
try {
const { stdout } = await this.git(['remote'], worktreePath)
const remotes = stdout
.split(/\r?\n/)
.map((line) => line.trim())
.filter(Boolean)
if (!remotes.includes(remote)) {
throw new Error(`Remote "${remote}" is not configured.`)
if (typeof remote !== 'string' || typeof branch !== 'string' || typeof ref !== 'string') {
throw new Error('Invalid remote-tracking fetch request.')
}
await this.git(['check-ref-format', `refs/heads/${branch}`], worktreePath)
await this.git(['check-ref-format', ref], worktreePath)
await this.git(['fetch', '--no-tags', remote, `+refs/heads/${branch}:${ref}`], worktreePath)
} catch (error) {
// Why: create-worktree needs a write-capable fetch, but generic git.exec
// intentionally rejects fetch. This narrow RPC keeps the relay allowlist
// tight while preserving the same safe error normalization as git.fetch.
throw new Error(normalizeGitErrorMessage(error, 'fetch'))
if (remote.startsWith('-') || branch.startsWith('-')) {
throw new Error('Remote-tracking fetch inputs must not start with "-".')
}
if (ref !== `refs/remotes/${remote}/${branch}`) {
throw new Error('Remote-tracking ref does not match the requested remote and branch.')
}
try {
const { stdout } = await this.git(['remote'], worktreePath)
const remotes = stdout
.split(/\r?\n/)
.map((line) => line.trim())
.filter(Boolean)
if (!remotes.includes(remote)) {
throw new Error(`Remote "${remote}" is not configured.`)
}
await this.git(['check-ref-format', `refs/heads/${branch}`], worktreePath)
await this.git(['check-ref-format', ref], worktreePath)
await this.git(['fetch', '--no-tags', remote, `+refs/heads/${branch}:${ref}`], worktreePath)
} catch (error) {
// Why: create-worktree needs a write-capable fetch, but generic git.exec
// intentionally rejects fetch. This narrow RPC keeps the relay allowlist
// tight while preserving the same safe error normalization as git.fetch.
throw new Error(normalizeGitErrorMessage(error, 'fetch'))
}
} finally {
this.gitDiffReadDedupe.clear()
}
}
private async fetchGitLabMergeRequestHead(params: Record<string, unknown>) {
this.gitDiffReadDedupe.clear()
const worktreePath = params.worktreePath as string
const remote = params.remote
const mrIid = params.mrIid
if (typeof remote !== 'string') {
throw new Error('Invalid GitLab merge request fetch request.')
}
if (typeof mrIid !== 'number' || !Number.isSafeInteger(mrIid) || mrIid <= 0) {
throw new Error('Invalid GitLab merge request fetch request.')
}
const mergeRequestIid = mrIid
if (remote.startsWith('-')) {
throw new Error('GitLab merge request fetch remote must not start with "-".')
}
try {
const { stdout } = await this.git(['remote'], worktreePath)
const remotes = stdout
.split(/\r?\n/)
.map((line) => line.trim())
.filter(Boolean)
if (!remotes.includes(remote)) {
throw new Error(`Remote "${remote}" is not configured.`)
if (typeof remote !== 'string') {
throw new Error('Invalid GitLab merge request fetch request.')
}
// Why: GitLab MR heads are not refs/heads/*, so the remote-tracking
// fetch RPC cannot represent fork MRs. Keep this write path MR-only.
await this.git(
['fetch', '--no-tags', remote, `refs/merge-requests/${mergeRequestIid}/head`],
worktreePath
)
} catch (error) {
throw new Error(normalizeGitErrorMessage(error, 'fetch'))
if (typeof mrIid !== 'number' || !Number.isSafeInteger(mrIid) || mrIid <= 0) {
throw new Error('Invalid GitLab merge request fetch request.')
}
const mergeRequestIid = mrIid
if (remote.startsWith('-')) {
throw new Error('GitLab merge request fetch remote must not start with "-".')
}
try {
const { stdout } = await this.git(['remote'], worktreePath)
const remotes = stdout
.split(/\r?\n/)
.map((line) => line.trim())
.filter(Boolean)
if (!remotes.includes(remote)) {
throw new Error(`Remote "${remote}" is not configured.`)
}
// Why: GitLab MR heads are not refs/heads/*, so the remote-tracking
// fetch RPC cannot represent fork MRs. Keep this write path MR-only.
await this.git(
['fetch', '--no-tags', remote, `refs/merge-requests/${mergeRequestIid}/head`],
worktreePath
)
} catch (error) {
throw new Error(normalizeGitErrorMessage(error, 'fetch'))
}
} finally {
this.gitDiffReadDedupe.clear()
}
}
private async push(params: Record<string, unknown>) {
this.gitDiffReadDedupe.clear()
const worktreePath = params.worktreePath as string
// Why: mirror src/main/git/remote.ts. Push to a configured upstream when
// present so SSH worktrees with non-origin targets do not get repointed.
void params.publish
try {
const target = await resolveRelayPushTarget(
this.git.bind(this),
worktreePath,
params.pushTarget
)
const args = [
'push',
...(params.forceWithLease === true ? ['--force-with-lease'] : []),
'--set-upstream',
...(target ? [target.remote, target.refspec] : ['origin', 'HEAD'])
]
await this.git(args, worktreePath)
} catch (error) {
// Why: mirror the local gitPush normalization so SSH users see the same
// "non-fast-forward / pull first" guidance instead of raw git stderr.
throw new Error(normalizeGitErrorMessage(error, 'push'))
try {
const target = await resolveRelayPushTarget(
this.git.bind(this),
worktreePath,
params.pushTarget
)
const args = [
'push',
...(params.forceWithLease === true ? ['--force-with-lease'] : []),
'--set-upstream',
...(target ? [target.remote, target.refspec] : ['origin', 'HEAD'])
]
await this.git(args, worktreePath)
} catch (error) {
// Why: mirror the local gitPush normalization so SSH users see the same
// "non-fast-forward / pull first" guidance instead of raw git stderr.
throw new Error(normalizeGitErrorMessage(error, 'push'))
}
} finally {
this.gitDiffReadDedupe.clear()
}
}
private async pullWithArgs(params: Record<string, unknown>, pullArgs: string[]) {
this.gitDiffReadDedupe.clear()
const worktreePath = params.worktreePath as string
try {
if (params.pushTarget !== undefined) {
assertGitPushTargetShape(params.pushTarget)
const pushTarget = params.pushTarget as GitPushTarget
await this.git(['check-ref-format', '--branch', pushTarget.branchName], worktreePath)
await this.git(
['pull', ...pullArgs, pushTarget.remoteName, pushTarget.branchName],
worktreePath
)
return
try {
if (params.pushTarget !== undefined) {
assertGitPushTargetShape(params.pushTarget)
const pushTarget = params.pushTarget as GitPushTarget
await this.git(['check-ref-format', '--branch', pushTarget.branchName], worktreePath)
await this.git(
['pull', ...pullArgs, pushTarget.remoteName, pushTarget.branchName],
worktreePath
)
return
}
const upstream = await resolveEffectiveGitUpstream((args) => this.git(args, worktreePath))
if (upstream && !upstream.isConfiguredUpstream) {
// Why: legacy Orca branches may still track origin/main while pushes
// target origin/<branch>. Pull the same effective branch the UI reports.
await this.git(
['pull', ...pullArgs, upstream.remoteName, upstream.branchName],
worktreePath
)
return
}
await this.git(['pull', ...pullArgs], worktreePath)
} catch (error) {
// Why: mirror the local gitPull normalization so SSH users see the same
// actionable messages instead of raw git stderr.
throw new Error(normalizeGitErrorMessage(error, 'pull'))
}
const upstream = await resolveEffectiveGitUpstream((args) => this.git(args, worktreePath))
if (upstream && !upstream.isConfiguredUpstream) {
// Why: legacy Orca branches may still track origin/main while pushes
// target origin/<branch>. Pull the same effective branch the UI reports.
await this.git(
['pull', ...pullArgs, upstream.remoteName, upstream.branchName],
worktreePath
)
return
}
await this.git(['pull', ...pullArgs], worktreePath)
} catch (error) {
// Why: mirror the local gitPull normalization so SSH users see the same
// actionable messages instead of raw git stderr.
throw new Error(normalizeGitErrorMessage(error, 'pull'))
} finally {
this.gitDiffReadDedupe.clear()
}
}
@ -709,16 +808,21 @@ export class GitHandler {
}
private async rebaseFromBase(params: Record<string, unknown>) {
this.gitDiffReadDedupe.clear()
const worktreePath = params.worktreePath as string
const baseRef = params.baseRef as string
try {
const source = await resolveGitRemoteRebaseSource(
((args) => this.git(args, worktreePath)) as GitCommandRunner,
baseRef
)
await this.git(['pull', '--rebase', source.remoteName, source.branchName], worktreePath)
} catch (error) {
throw new Error(normalizeGitErrorMessage(error, 'pull'))
try {
const source = await resolveGitRemoteRebaseSource(
((args) => this.git(args, worktreePath)) as GitCommandRunner,
baseRef
)
await this.git(['pull', '--rebase', source.remoteName, source.branchName], worktreePath)
} catch (error) {
throw new Error(normalizeGitErrorMessage(error, 'pull'))
}
} finally {
this.gitDiffReadDedupe.clear()
}
}
@ -728,27 +832,50 @@ export class GitHandler {
if (baseRef.startsWith('-')) {
throw new Error('Base ref must not start with "-"')
}
return branchDiffEntries(
this.git.bind(this),
this.gitBuffer.bind(this),
worktreePath,
baseRef,
{
includePatch: params.includePatch as boolean | undefined,
filePath: params.filePath as string | undefined,
oldPath: params.oldPath as string | undefined
}
const options = {
includePatch: params.includePatch as boolean | undefined,
filePath: params.filePath as string | undefined,
oldPath: params.oldPath as string | undefined
}
return this.gitDiffReadDedupe.run(
stableInFlightKey([
'branchDiff',
worktreePath,
baseRef,
options.includePatch ?? null,
options.filePath ?? null,
options.oldPath ?? null
]),
() =>
branchDiffEntries(
this.git.bind(this),
this.gitBuffer.bind(this),
worktreePath,
baseRef,
options
)
)
}
private async commitDiff(params: Record<string, unknown>) {
const worktreePath = params.worktreePath as string
return commitDiffEntry(this.gitBuffer.bind(this), worktreePath, {
const args = {
commitOid: params.commitOid as string,
parentOid: params.parentOid as string | null | undefined,
filePath: params.filePath as string,
oldPath: params.oldPath as string | undefined
})
}
return this.gitDiffReadDedupe.run(
stableInFlightKey([
'commitDiff',
worktreePath,
args.commitOid,
args.parentOid ?? null,
args.filePath,
args.oldPath ?? null
]),
() => commitDiffEntry(this.gitBuffer.bind(this), worktreePath, args)
)
}
private async exec(params: Record<string, unknown>, context?: RequestContext) {
@ -841,22 +968,24 @@ export class GitHandler {
}
private async renameCurrentBranch(params: Record<string, unknown>) {
const worktreePath = params.worktreePath
const newBranch = params.newBranch
if (typeof worktreePath !== 'string' || typeof newBranch !== 'string') {
throw new Error('Invalid branch rename request.')
}
if (newBranch.startsWith('-')) {
throw new Error('Branch name must not start with "-".')
}
try {
// Why: generic git.exec intentionally blocks destructive branch flags.
// This narrow RPC permits only the already-checked current-branch rename.
await this.git(['check-ref-format', '--branch', newBranch], worktreePath)
await this.git(['branch', '-m', newBranch], worktreePath)
} catch (error) {
throw new Error(normalizeGitErrorMessage(error))
}
return this.runWithDiffDedupeClear(async () => {
const worktreePath = params.worktreePath
const newBranch = params.newBranch
if (typeof worktreePath !== 'string' || typeof newBranch !== 'string') {
throw new Error('Invalid branch rename request.')
}
if (newBranch.startsWith('-')) {
throw new Error('Branch name must not start with "-".')
}
try {
// Why: generic git.exec intentionally blocks destructive branch flags.
// This narrow RPC permits only the already-checked current-branch rename.
await this.git(['check-ref-format', '--branch', newBranch], worktreePath)
await this.git(['branch', '-m', newBranch], worktreePath)
} catch (error) {
throw new Error(normalizeGitErrorMessage(error))
}
})
}
private async isGitRepo(params: Record<string, unknown>) {
@ -891,11 +1020,11 @@ export class GitHandler {
}
private async addWorktree(params: Record<string, unknown>) {
return addWorktreeOp(this.git.bind(this), params)
return this.runWithDiffDedupeClear(() => addWorktreeOp(this.git.bind(this), params))
}
private async removeWorktree(params: Record<string, unknown>) {
return removeWorktreeOp(this.git.bind(this), params)
return this.runWithDiffDedupeClear(() => removeWorktreeOp(this.git.bind(this), params))
}
private async worktreeIsClean(params: Record<string, unknown>) {
@ -903,6 +1032,8 @@ export class GitHandler {
}
private async refreshLocalBaseRefForWorktreeCreate(params: Record<string, unknown>) {
return refreshLocalBaseRefForWorktreeCreateOp(this.git.bind(this), params)
return this.runWithDiffDedupeClear(() =>
refreshLocalBaseRefForWorktreeCreateOp(this.git.bind(this), params)
)
}
}

83
src/shared/in-flight-promise-dedupe.test.ts Normal file
View File

@ -0,0 +1,83 @@
import { describe, expect, it, vi } from 'vitest'
import { InFlightPromiseDedupe, stableInFlightKey } from './in-flight-promise-dedupe'
describe('InFlightPromiseDedupe', () => {
it('coalesces only while in flight and retries after rejection', async () => {
const dedupe = new InFlightPromiseDedupe<string>()
const load = vi
.fn<() => Promise<string>>()
.mockRejectedValueOnce(new Error('transient failure'))
.mockResolvedValueOnce('fresh')
const key = stableInFlightKey(['diff', '/repo', 'src/file.ts', true])
const first = dedupe.run(key, load)
const second = dedupe.run(key, load)
expect(first).toBe(second)
await expect(first).rejects.toThrow('transient failure')
expect(load).toHaveBeenCalledTimes(1)
await expect(dedupe.run(key, load)).resolves.toBe('fresh')
expect(load).toHaveBeenCalledTimes(2)
})
it('uses exact keys for distinct input parts', async () => {
const dedupe = new InFlightPromiseDedupe<string>()
const load = vi.fn(async () => 'value')
await Promise.all([
dedupe.run(stableInFlightKey(['diff', '/repo', 'src/file.ts', true]), load),
dedupe.run(stableInFlightKey(['diff', '/repo', 'src/file.ts', false]), load)
])
expect(load).toHaveBeenCalledTimes(2)
})
it('clears entries after synchronous loader failures', async () => {
const dedupe = new InFlightPromiseDedupe<string>()
const load = vi
.fn<() => Promise<string> | string>()
.mockImplementationOnce(() => {
throw new Error('sync failure')
})
.mockResolvedValueOnce('fresh')
const key = stableInFlightKey(['diff', '/repo', 'src/file.ts'])
await expect(dedupe.run(key, () => Promise.resolve(load()))).rejects.toThrow('sync failure')
await expect(dedupe.run(key, () => Promise.resolve(load()))).resolves.toBe('fresh')
expect(load).toHaveBeenCalledTimes(2)
})
it('clear drops pending entries so later calls start fresh work', async () => {
const dedupe = new InFlightPromiseDedupe<string>()
const load = vi.fn<() => Promise<string>>()
load.mockReturnValueOnce(new Promise(() => undefined)).mockResolvedValueOnce('fresh')
const key = stableInFlightKey(['diff', '/repo', 'src/file.ts'])
void dedupe.run(key, load)
dedupe.clear()
await expect(dedupe.run(key, load)).resolves.toBe('fresh')
expect(load).toHaveBeenCalledTimes(2)
})
it('expires hung entries so retries can start fresh work', async () => {
vi.useFakeTimers()
try {
const dedupe = new InFlightPromiseDedupe<string>(5)
const load = vi.fn<() => Promise<string>>()
load.mockReturnValueOnce(new Promise(() => undefined)).mockResolvedValueOnce('fresh')
const key = stableInFlightKey(['diff', '/repo', 'src/file.ts'])
void dedupe.run(key, load)
vi.advanceTimersByTime(5)
await expect(dedupe.run(key, load)).resolves.toBe('fresh')
expect(load).toHaveBeenCalledTimes(2)
} finally {
vi.useRealTimers()
}
})
})

57
src/shared/in-flight-promise-dedupe.ts Normal file
View File

@ -0,0 +1,57 @@
export class InFlightPromiseDedupe<T> {
private readonly entries = new Map<
string,
{ promise: Promise<T>; timeout: ReturnType<typeof setTimeout> | null }
>()
constructor(private readonly maxInFlightMs = 30_000) {}
run(key: string, load: () => Promise<T>): Promise<T> {
const existing = this.entries.get(key)
if (existing) {
return existing.promise
}
// Why: this is in-flight coalescing only; the next read after settle must
// observe fresh git state instead of a cached diff.
const promise = Promise.resolve()
.then(load)
.finally(() => {
const entry = this.entries.get(key)
if (entry?.promise === promise) {
if (entry.timeout) {
clearTimeout(entry.timeout)
}
this.entries.delete(key)
}
})
const entry = {
promise,
// Why: renderer diff rows already time out hung loads; drop matching
// in-flight entries too so retry can issue fresh git work.
timeout:
this.maxInFlightMs > 0
? setTimeout(() => {
if (this.entries.get(key)?.promise === promise) {
this.entries.delete(key)
}
}, this.maxInFlightMs)
: null
}
this.entries.set(key, entry)
return promise
}
clear(): void {
for (const entry of this.entries.values()) {
if (entry.timeout) {
clearTimeout(entry.timeout)
}
}
this.entries.clear()
}
}
export function stableInFlightKey(parts: readonly unknown[]): string {
return JSON.stringify(parts)
}