diff --git a/src/main/git/remote.ts b/src/main/git/remote.ts index 85aafcb9c87..c3e389d8478 100644 --- a/src/main/git/remote.ts +++ b/src/main/git/remote.ts @@ -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 { - await runWithGitReadCacheInvalidation(() => - gitPullWithArgs(worktreePath, ['--ff-only'], pushTarget, options) + await runWithGitWorktreeOperationLock(worktreePath, options.signal, () => + runWithGitReadCacheInvalidation(() => + gitPullWithArgs(worktreePath, ['--ff-only'], pushTarget, options) + ) ) } diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index d3d6f57bbaa..b3e5cb96fcb 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -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, pullArgs: string[]) { + private async pullWithArgs( + params: Record, + pullArgs: string[], + signal?: AbortSignal + ) { + const worktreePath = params.worktreePath as string + return runWithGitWorktreeOperationLock(worktreePath, signal, () => + this.runPullWithArgsUnlocked(params, pullArgs) + ) + } + + private async runPullWithArgsUnlocked( + params: Record, + pullArgs: string[] + ): Promise { this.clearGitMutationReadCaches() const worktreePath = params.worktreePath as string const runPull = async (effectiveArgs: string[]): Promise => { @@ -1181,13 +1195,13 @@ export class GitHandler { } } - private async pull(params: Record) { + private async pull(params: Record, 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) { - await this.pullWithArgs(params, ['--ff-only']) + private async fastForward(params: Record, context?: RequestContext) { + await this.pullWithArgs(params, ['--ff-only'], context?.signal) } private async rebaseFromBase(params: Record, context?: RequestContext) { diff --git a/src/shared/git-fetch-head-lock.ts b/src/shared/git-fetch-head-lock.ts index 474c7b2fdb2..4fbb215a3a5 100644 --- a/src/shared/git-fetch-head-lock.ts +++ b/src/shared/git-fetch-head-lock.ts @@ -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 - release: () => void +function abortError(): Error { + const error = new Error('The operation was aborted.') + error.name = 'AbortError' + return error } - -const lanes = new Map() 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, - signal: AbortSignal | undefined -): Promise { - if (!signal) { - await predecessor.catch(() => undefined) - return - } - if (signal.aborted) { - throw abortError() - } - let rejectAbort!: (error: Error) => void - const aborted = new Promise((_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( worktreePath: string, signal: AbortSignal | undefined, @@ -164,23 +134,5 @@ export async function runWithGitFetchHeadLock( explicitGitDir?: string ): Promise { const key = await fetchLockPath(worktreePath, signal, explicitGitDir) - const predecessor = lanes.get(key)?.tail ?? Promise.resolve() - let release!: () => void - const current = new Promise((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) } diff --git a/src/shared/git-operation-lock.ts b/src/shared/git-operation-lock.ts new file mode 100644 index 00000000000..cacc5af9a53 --- /dev/null +++ b/src/shared/git-operation-lock.ts @@ -0,0 +1,62 @@ +type GitOperationLane = { + tail: Promise + release: () => void +} + +const lanes = new Map() + +function abortError(): Error { + const error = new Error('The operation was aborted.') + error.name = 'AbortError' + return error +} + +async function waitForPredecessor( + predecessor: Promise, + signal: AbortSignal | undefined +): Promise { + if (!signal) { + await predecessor.catch(() => undefined) + return + } + if (signal.aborted) { + throw abortError() + } + let rejectAbort!: (error: Error) => void + const aborted = new Promise((_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( + key: string, + signal: AbortSignal | undefined, + run: () => Promise +): Promise { + const predecessor = lanes.get(key)?.tail ?? Promise.resolve() + let release!: () => void + const current = new Promise((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() + } +} diff --git a/src/shared/git-worktree-operation-lock.ts b/src/shared/git-worktree-operation-lock.ts index 4e116284168..55491bd2ac5 100644 --- a/src/shared/git-worktree-operation-lock.ts +++ b/src/shared/git-worktree-operation-lock.ts @@ -1,41 +1,6 @@ import { realpath } from 'node:fs/promises' import { resolve } from 'node:path' - -type OperationLane = { - tail: Promise - release: () => void -} -const lanes = new Map() - -function abortError(): Error { - const error = new Error('The operation was aborted.') - error.name = 'AbortError' - return error -} - -async function waitForPredecessor( - predecessor: Promise, - signal: AbortSignal | undefined -): Promise { - if (!signal) { - await predecessor.catch(() => undefined) - return - } - if (signal.aborted) { - throw abortError() - } - let rejectAbort!: (error: Error) => void - const aborted = new Promise((_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( @@ -44,26 +9,5 @@ export async function runWithGitWorktreeOperationLock( run: () => Promise ): Promise { const key = await realpath(worktreePath).catch(() => resolve(worktreePath)) - const predecessor = lanes.get(key)?.tail ?? Promise.resolve() - let release!: () => void - const current = new Promise((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) }