refactor: improve git lock handling (#790)

Author: jonathanlab

apps/twig/src/main/services/focus/sync-service.ts

@@ -227,8 +227,6 @@ export class FocusSyncService {
       `Syncing changes: staged=${hasStaged}, unstaged=${hasUnstaged}, untracked=${untrackedList.length} files`,
     );
 
-    await this.cleanStaleLockFile(dstPath);
-
     if (hasStaged) {
       try {
         await this.applyPatch(dstPath, stagedPatch, true);
@@ -254,40 +252,6 @@ export class FocusSyncService {
     }
   }
 
-  private async cleanStaleLockFile(repoPath: string): Promise<void> {
-    const possibleLockPaths = [
-      path.join(repoPath, ".git", "index.lock"),
-      path.join(repoPath, ".git", "worktrees"),
-    ];
-
-    for (const lockPath of possibleLockPaths) {
-      if (lockPath.endsWith("worktrees")) {
-        try {
-          const entries = await fs.readdir(lockPath);
-          for (const entry of entries) {
-            const worktreeLock = path.join(lockPath, entry, "index.lock");
-            await this.removeStaleLock(worktreeLock);
-          }
-        } catch {}
-      } else {
-        await this.removeStaleLock(lockPath);
-      }
-    }
-  }
-
-  private async removeStaleLock(lockPath: string): Promise<void> {
-    try {
-      const stat = await fs.stat(lockPath);
-      const ageMs = Date.now() - stat.mtimeMs;
-      if (ageMs > 5000) {
-        await fs.rm(lockPath);
-        log.info(
-          `Removed stale index.lock (age: ${Math.round(ageMs / 1000)}s)`,
-        );
-      }
-    } catch {}
-  }
-
   private async applyPatch(
     repoPath: string,
     patch: string,

packages/git/src/git-saga.ts

@@ -0,0 +1,37 @@
+import { Saga } from "@posthog/shared";
+import type { GitClient } from "./client.js";
+import { getGitOperationManager } from "./operation-manager.js";
+
+export interface GitSagaInput {
+  baseDir: string;
+  signal?: AbortSignal;
+}
+
+export abstract class GitSaga<
+  TInput extends GitSagaInput,
+  TOutput,
+> extends Saga<TInput, TOutput> {
+  private _git: GitClient | null = null;
+
+  protected get git(): GitClient {
+    if (!this._git) {
+      throw new Error("git client accessed before execute() was called");
+    }
+    return this._git;
+  }
+
+  protected async execute(input: TInput): Promise<TOutput> {
+    const manager = getGitOperationManager();
+
+    return manager.executeWrite(
+      input.baseDir,
+      async (git) => {
+        this._git = git;
+        return this.executeGitOperations(input);
+      },
+      { signal: input.signal },
+    );
+  }
+
+  protected abstract executeGitOperations(input: TInput): Promise<TOutput>;
+}

packages/git/src/lock-detector.ts

@@ -0,0 +1,59 @@
+import { execFile } from "node:child_process";
+import fs from "node:fs/promises";
+import path from "node:path";
+import { promisify } from "node:util";
+
+const execFileAsync = promisify(execFile);
+
+export interface LockInfo {
+  path: string;
+  ageMs: number;
+}
+
+export async function getIndexLockPath(repoPath: string): Promise<string> {
+  try {
+    const { stdout } = await execFileAsync(
+      "git",
+      ["rev-parse", "--git-path", "index.lock"],
+      { cwd: repoPath },
+    );
+    return path.resolve(repoPath, stdout.trim());
+  } catch {
+    return path.join(repoPath, ".git", "index.lock");
+  }
+}
+
+export async function getLockInfo(repoPath: string): Promise<LockInfo | null> {
+  const lockPath = await getIndexLockPath(repoPath);
+  try {
+    const stat = await fs.stat(lockPath);
+    return {
+      path: lockPath,
+      ageMs: Date.now() - stat.mtimeMs,
+    };
+  } catch {
+    return null;
+  }
+}
+
+export async function removeLock(repoPath: string): Promise<void> {
+  const lockPath = await getIndexLockPath(repoPath);
+  await fs.rm(lockPath, { force: true });
+}
+
+export async function isLocked(repoPath: string): Promise<boolean> {
+  return (await getLockInfo(repoPath)) !== null;
+}
+
+export async function waitForUnlock(
+  repoPath: string,
+  timeoutMs = 10000,
+  intervalMs = 100,
+): Promise<boolean> {
+  const start = Date.now();
+  while (Date.now() - start < timeoutMs) {
+    if (!(await isLocked(repoPath))) return true;
+    await new Promise((r) => setTimeout(r, intervalMs));
+  }
+  return false;
+}

packages/git/src/operation-manager.ts

@@ -0,0 +1,115 @@
+import { createGitClient } from "./client.js";
+import { removeLock, waitForUnlock } from "./lock-detector.js";
+import { AsyncReaderWriterLock } from "./rw-lock.js";
+
+interface RepoState {
+  lock: AsyncReaderWriterLock;
+  lastAccess: number;
+}
+
+export interface ExecuteOptions {
+  signal?: AbortSignal;
+  timeoutMs?: number;
+  waitForExternalLock?: boolean;
+}
+
+class GitOperationManagerImpl {
+  private repoStates = new Map<string, RepoState>();
+  private cleanupInterval: ReturnType<typeof setInterval> | null = null;
+  private static readonly CLEANUP_INTERVAL_MS = 60000;
+  private static readonly IDLE_TIMEOUT_MS = 300000;
+
+  constructor() {
+    this.cleanupInterval = setInterval(
+      () => this.cleanupIdleRepos(),
+      GitOperationManagerImpl.CLEANUP_INTERVAL_MS,
+    );
+  }
+
+  private getRepoState(repoPath: string): RepoState {
+    let state = this.repoStates.get(repoPath);
+    if (!state) {
+      state = { lock: new AsyncReaderWriterLock(), lastAccess: Date.now() };
+      this.repoStates.set(repoPath, state);
+    }
+    state.lastAccess = Date.now();
+    return state;
+  }
+
+  private cleanupIdleRepos(): void {
+    const now = Date.now();
+    for (const [repoPath, state] of this.repoStates) {
+      if (now - state.lastAccess > GitOperationManagerImpl.IDLE_TIMEOUT_MS) {
+        this.repoStates.delete(repoPath);
+      }
+    }
+  }
+
+  async executeRead<T>(
+    repoPath: string,
+    operation: (git: ReturnType<typeof createGitClient>) => Promise<T>,
+    options?: ExecuteOptions,
+  ): Promise<T> {
+    const git = createGitClient(repoPath, {
+      abortSignal: options?.signal,
+    }).env({ GIT_OPTIONAL_LOCKS: "0" });
+    return operation(git);
+  }
+
+  async executeWrite<T>(
+    repoPath: string,
+    operation: (git: ReturnType<typeof createGitClient>) => Promise<T>,
+    options?: ExecuteOptions,
+  ): Promise<T> {
+    const state = this.getRepoState(repoPath);
+
+    if (options?.waitForExternalLock !== false) {
+      const unlocked = await waitForUnlock(
+        repoPath,
+        options?.timeoutMs ?? 10000,
+      );
+      if (!unlocked) {
+        throw new Error(`Git repository is locked: ${repoPath}`);
+      }
+    }
+
+    await state.lock.acquireWrite();
+    try {
+      const git = createGitClient(repoPath, { abortSignal: options?.signal });
+      return await operation(git);
+    } catch (error) {
+      if (options?.signal?.aborted) {
+        await removeLock(repoPath).catch(() => {});
+      }
+      throw error;
+    } finally {
+      state.lock.releaseWrite();
+    }
+  }
+
+  destroy(): void {
+    if (this.cleanupInterval) {
+      clearInterval(this.cleanupInterval);
+      this.cleanupInterval = null;
+    }
+    this.repoStates.clear();
+  }
+}
+
+let instance: GitOperationManagerImpl | null = null;
+
+export function getGitOperationManager(): GitOperationManagerImpl {
+  if (!instance) {
+    instance = new GitOperationManagerImpl();
+  }
+  return instance;
+}
+
+export function resetGitOperationManager(): void {
+  if (instance) {
+    instance.destroy();
+    instance = null;
+  }
+}
+
+export type GitOperationManager = GitOperationManagerImpl;