Serialize pull and fastForward operations per-worktree

- Extract generic git operation lock to reuse locking pattern
- Refactor existing locks to use the generic implementation
- Apply per-worktree serialization to pull and fastForward to prevent races
This commit is contained in:
Jinjing
2026-08-24 11:46:59 -07:00
parent 21cdce5cf7
commit d1fef4585e
5 changed files with 98 additions and 123 deletions
+7 -4
View File
@@ -10,6 +10,7 @@ import { gitOptionsForWorktree } from './git-runtime-options'
import { validateGitPushTarget } from './push-target-validation'
import { gitExecFileAsync } from './runner'
import { runWithGitReadCacheInvalidation } from './status'
import { runWithGitWorktreeOperationLock } from '../../shared/git-worktree-operation-lock'
export { gitPullRebaseFromBase } from './remote-rebase'
@@ -258,8 +259,8 @@ export async function gitPull(
// Why: plain `git pull` uses the user's configured pull strategy (merge by
// default) so diverged branches reconcile instead of erroring out. Conflicts
// surface through the existing conflict-resolution flow.
await runWithGitReadCacheInvalidation(() =>
gitPullWithArgs(worktreePath, [], pushTarget, options)
await runWithGitWorktreeOperationLock(worktreePath, options.signal, () =>
runWithGitReadCacheInvalidation(() => gitPullWithArgs(worktreePath, [], pushTarget, options))
)
}
@@ -268,8 +269,10 @@ export async function gitFastForward(
pushTarget?: GitPushTarget,
options: GitRuntimeOptions = {}
): Promise<void> {
await runWithGitReadCacheInvalidation(() =>
gitPullWithArgs(worktreePath, ['--ff-only'], pushTarget, options)
await runWithGitWorktreeOperationLock(worktreePath, options.signal, () =>
runWithGitReadCacheInvalidation(() =>
gitPullWithArgs(worktreePath, ['--ff-only'], pushTarget, options)
)
)
}
+21 -7
View File
@@ -297,8 +297,8 @@ export class GitHandler {
this.fetchGitLabMergeRequestHead(p)
)
this.dispatcher.onRequest('git.push', (p) => this.push(p))
this.dispatcher.onRequest('git.pull', (p) => this.pull(p))
this.dispatcher.onRequest('git.fastForward', (p) => this.fastForward(p))
this.dispatcher.onRequest('git.pull', (p, context) => this.pull(p, context))
this.dispatcher.onRequest('git.fastForward', (p, context) => this.fastForward(p, context))
this.dispatcher.onRequest('git.rebaseFromBase', (p, context) => this.rebaseFromBase(p, context))
this.dispatcher.onRequest('git.branchDiff', (p, context) => this.branchDiff(p, context))
this.dispatcher.onRequest('git.commitDiff', (p, context) => this.commitDiff(p, context))
@@ -1143,7 +1143,21 @@ export class GitHandler {
}
}
private async pullWithArgs(params: Record<string, unknown>, pullArgs: string[]) {
private async pullWithArgs(
params: Record<string, unknown>,
pullArgs: string[],
signal?: AbortSignal
) {
const worktreePath = params.worktreePath as string
return runWithGitWorktreeOperationLock(worktreePath, signal, () =>
this.runPullWithArgsUnlocked(params, pullArgs)
)
}
private async runPullWithArgsUnlocked(
params: Record<string, unknown>,
pullArgs: string[]
): Promise<void> {
this.clearGitMutationReadCaches()
const worktreePath = params.worktreePath as string
const runPull = async (effectiveArgs: string[]): Promise<void> => {
@@ -1181,13 +1195,13 @@ export class GitHandler {
}
}
private async pull(params: Record<string, unknown>) {
private async pull(params: Record<string, unknown>, context?: RequestContext) {
// Why: plain `git pull` honors user merge/rebase/ff policy.
await this.pullWithArgs(params, [])
await this.pullWithArgs(params, [], context?.signal)
}
private async fastForward(params: Record<string, unknown>) {
await this.pullWithArgs(params, ['--ff-only'])
private async fastForward(params: Record<string, unknown>, context?: RequestContext) {
await this.pullWithArgs(params, ['--ff-only'], context?.signal)
}
private async rebaseFromBase(params: Record<string, unknown>, context?: RequestContext) {
+6 -54
View File
@@ -1,12 +1,12 @@
import * as path from 'node:path'
import { readFile, realpath, stat } from 'node:fs/promises'
import { runWithGitOperationLock } from './git-operation-lock'
type GitFetchHeadLockLane = {
tail: Promise<void>
release: () => void
function abortError(): Error {
const error = new Error('The operation was aborted.')
error.name = 'AbortError'
return error
}
const lanes = new Map<string, GitFetchHeadLockLane>()
const GLOBAL_OPTIONS_WITH_VALUE = new Set([
'-c',
'-C',
@@ -127,36 +127,6 @@ async function fetchLockPath(
return path.join(canonicalGitDir, 'FETCH_HEAD')
}
function abortError(): Error {
const error = new Error('The operation was aborted.')
error.name = 'AbortError'
return error
}
async function waitForPredecessor(
predecessor: Promise<void>,
signal: AbortSignal | undefined
): Promise<void> {
if (!signal) {
await predecessor.catch(() => undefined)
return
}
if (signal.aborted) {
throw abortError()
}
let rejectAbort!: (error: Error) => void
const aborted = new Promise<never>((_resolve, reject) => {
rejectAbort = reject
})
const onAbort = () => rejectAbort(abortError())
signal.addEventListener('abort', onAbort, { once: true })
try {
await Promise.race([predecessor.catch(() => undefined), aborted])
} finally {
signal.removeEventListener('abort', onAbort)
}
}
export async function runWithGitFetchHeadLock<T>(
worktreePath: string,
signal: AbortSignal | undefined,
@@ -164,23 +134,5 @@ export async function runWithGitFetchHeadLock<T>(
explicitGitDir?: string
): Promise<T> {
const key = await fetchLockPath(worktreePath, signal, explicitGitDir)
const predecessor = lanes.get(key)?.tail ?? Promise.resolve()
let release!: () => void
const current = new Promise<void>((resolve) => {
release = resolve
})
const lane = { tail: predecessor.catch(() => undefined).then(() => current), release }
lanes.set(key, lane)
void lane.tail.then(() => {
if (lanes.get(key) === lane) {
lanes.delete(key)
}
})
try {
await waitForPredecessor(predecessor, signal)
return await run()
} finally {
lane.release()
}
return runWithGitOperationLock(key, signal, run)
}
+62
View File
@@ -0,0 +1,62 @@
type GitOperationLane = {
tail: Promise<void>
release: () => void
}
const lanes = new Map<string, GitOperationLane>()
function abortError(): Error {
const error = new Error('The operation was aborted.')
error.name = 'AbortError'
return error
}
async function waitForPredecessor(
predecessor: Promise<void>,
signal: AbortSignal | undefined
): Promise<void> {
if (!signal) {
await predecessor.catch(() => undefined)
return
}
if (signal.aborted) {
throw abortError()
}
let rejectAbort!: (error: Error) => void
const aborted = new Promise<never>((_resolve, reject) => {
rejectAbort = reject
})
const onAbort = () => rejectAbort(abortError())
signal.addEventListener('abort', onAbort, { once: true })
try {
await Promise.race([predecessor.catch(() => undefined), aborted])
} finally {
signal.removeEventListener('abort', onAbort)
}
}
export async function runWithGitOperationLock<T>(
key: string,
signal: AbortSignal | undefined,
run: () => Promise<T>
): Promise<T> {
const predecessor = lanes.get(key)?.tail ?? Promise.resolve()
let release!: () => void
const current = new Promise<void>((resolve) => {
release = resolve
})
const lane = { tail: predecessor.catch(() => undefined).then(() => current), release }
lanes.set(key, lane)
void lane.tail.then(() => {
if (lanes.get(key) === lane) {
lanes.delete(key)
}
})
try {
await waitForPredecessor(predecessor, signal)
return await run()
} finally {
lane.release()
}
}
+2 -58
View File
@@ -1,41 +1,6 @@
import { realpath } from 'node:fs/promises'
import { resolve } from 'node:path'
type OperationLane = {
tail: Promise<void>
release: () => void
}
const lanes = new Map<string, OperationLane>()
function abortError(): Error {
const error = new Error('The operation was aborted.')
error.name = 'AbortError'
return error
}
async function waitForPredecessor(
predecessor: Promise<void>,
signal: AbortSignal | undefined
): Promise<void> {
if (!signal) {
await predecessor.catch(() => undefined)
return
}
if (signal.aborted) {
throw abortError()
}
let rejectAbort!: (error: Error) => void
const aborted = new Promise<never>((_resolve, reject) => {
rejectAbort = reject
})
const onAbort = () => rejectAbort(abortError())
signal.addEventListener('abort', onAbort, { once: true })
try {
await Promise.race([predecessor.catch(() => undefined), aborted])
} finally {
signal.removeEventListener('abort', onAbort)
}
}
import { runWithGitOperationLock } from './git-operation-lock'
/** Serialize mutations that leave per-worktree state in progress (for example, rebase). */
export async function runWithGitWorktreeOperationLock<T>(
@@ -44,26 +9,5 @@ export async function runWithGitWorktreeOperationLock<T>(
run: () => Promise<T>
): Promise<T> {
const key = await realpath(worktreePath).catch(() => resolve(worktreePath))
const predecessor = lanes.get(key)?.tail ?? Promise.resolve()
let release!: () => void
const current = new Promise<void>((resolveCurrent) => {
release = resolveCurrent
})
const lane = {
tail: predecessor.catch(() => undefined).then(() => current),
release
}
lanes.set(key, lane)
void lane.tail.then(() => {
if (lanes.get(key) === lane) {
lanes.delete(key)
}
})
try {
await waitForPredecessor(predecessor, signal)
return await run()
} finally {
lane.release()
}
return runWithGitOperationLock(key, signal, run)
}