From ca516a4306a204f56d694142962db8a2ed674983 Mon Sep 17 00:00:00 2001
From: Neil <4138956+nwparker@users.noreply.github.com>
Date: Mon, 31 Aug 2026 00:09:07 -0700
Subject: [PATCH] test(git): pin FETCH_HEAD lock order in the admission
 lifetime test (#17641)

`serializes FETCH_HEAD callers before they enter admission` assumed that two
same-repo fetches join the FETCH_HEAD lock lane in call order. They do not.

`runWithGitFetchHeadLock` first `await`s `fetchLockPath`, which walks the
filesystem (`realpath`, `stat` per parent directory, `readFile` of `commondir`,
`realpath` again) before it calls `runWithGitOperationLock`, and the lane is
registered only after that walk resolves. For a non-existent `/repo` that is
five libuv threadpool round-trips per caller. Two callers issued back to back
run their chains concurrently, so lane order is threadpool completion order,
not call order.

When the `interactive` fetch won that race it entered the lane ahead of the
`background` fetch. On the first caller's release it reached admission
immediately and, being interactive, took the free network headroom slot instead
of queueing, while the background fetch stayed parked on the lock. `queued`
therefore settled at 0 and never reached the asserted 1. Measured inversion
rate for the bare lock-path walk was 54/500 on an idle machine; the test itself
failed 5/20 locally, always at the same assertion, matching the two CI failures
on unrelated PRs (#17530, #17630) at the same line.

Fix the premise rather than the symptom: stub only the key derivation, keeping
the real FIFO `runWithGitOperationLock` that the test actually exercises, so the
lane is registered synchronously with the call. Key derivation keeps its own
coverage in `src/shared/git-fetch-head-lock.test.ts`. This also stops the fetch
tests in this file from sharing one global `/.git/FETCH_HEAD` lane with each
other and from touching the real filesystem.

Verified deterministic: 40/40, then 30/30 clean runs, plus 25/25 with twelve CPU
hogs and a concurrent `src/main/git/command-runner/` run saturating the box.
---
 .../git-exec-admission-lifetime.test.ts         | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/src/main/git/command-runner/git-exec-admission-lifetime.test.ts b/src/main/git/command-runner/git-exec-admission-lifetime.test.ts
index bba29a4adaa..e603bb3398d 100644
--- a/src/main/git/command-runner/git-exec-admission-lifetime.test.ts
+++ b/src/main/git/command-runner/git-exec-admission-lifetime.test.ts
@@ -1,6 +1,7 @@
 import { EventEmitter } from 'node:events'
 import type { ChildProcess } from 'node:child_process'
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import type * as GitFetchHeadLockModule from '../../../shared/git-fetch-head-lock'
 
 const {
   execFileMock,
@@ -28,6 +29,22 @@ vi.mock('../../../shared/child-process/process-tree-termination', () => ({
   signalProcessTree: signalProcessTreeMock,
   forceTerminateProcessTree: forceTerminateProcessTreeMock
 }))
+// Why: the real FETCH_HEAD key derivation walks the filesystem (realpath/stat/readFile), so
+// concurrent same-repo callers join the lock lane in libuv threadpool completion order rather
+// than call order. Keep the real FIFO lock and drop only the key walk, which owns its coverage
+// in `src/shared/git-fetch-head-lock.test.ts`.
+vi.mock('../../../shared/git-fetch-head-lock', async (importOriginal) => {
+  const actual = await importOriginal<typeof GitFetchHeadLockModule>()
+  const { runWithGitOperationLock } = await import('../../../shared/git-operation-lock')
+  return {
+    ...actual,
+    runWithGitFetchHeadLock: <T>(
+      worktreePath: string,
+      signal: AbortSignal | undefined,
+      run: () => Promise<T>
+    ): Promise<T> => runWithGitOperationLock(worktreePath, signal, run)
+  }
+})
 
 import { gitExecFileAsync, gitExecFileAsyncBuffer } from './git-exec-file'
 import { execFileCapture } from './exec-file-capture'