From 9b697a32063420f7b0facc1ef4192baf05da36e1 Mon Sep 17 00:00:00 2001 From: AviPeltz Date: Tue, 13 Jan 2026 13:56:33 -0800 Subject: [PATCH 1/3] fix git status to not lock --- .../src/lib/trpc/routers/changes/status.ts | 4 +- .../lib/trpc/routers/workspaces/utils/git.ts | 124 +++++++++++++++++- 2 files changed, 121 insertions(+), 7 deletions(-) diff --git a/apps/desktop/src/lib/trpc/routers/changes/status.ts b/apps/desktop/src/lib/trpc/routers/changes/status.ts index 1b58be237ee..43334e8ae03 100644 --- a/apps/desktop/src/lib/trpc/routers/changes/status.ts +++ b/apps/desktop/src/lib/trpc/routers/changes/status.ts @@ -2,6 +2,7 @@ import type { ChangedFile, GitChangesStatus } from "shared/changes-types"; import simpleGit from "simple-git"; import { z } from "zod"; import { publicProcedure, router } from "../.."; +import { getStatusNoLock } from "../workspaces/utils/git"; import { assertRegisteredWorktree, secureFs } from "./security"; import { applyNumstatToFiles } from "./utils/apply-numstat"; import { @@ -26,7 +27,8 @@ export const createStatusRouter = () => { const defaultBranch = input.defaultBranch || "main"; // First, get status (needed for subsequent operations) - const status = await git.status(); + // Use --no-optional-locks to avoid holding locks on the repository + const status = await getStatusNoLock(input.worktreePath); const parsed = parseGitStatus(status); // Run independent operations in parallel diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts index 2226de97889..ce023aafb8e 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts @@ -3,7 +3,7 @@ import { randomBytes } from "node:crypto"; import { mkdir, readFile, stat } from "node:fs/promises"; import { join } from "node:path"; import { promisify } from "node:util"; -import simpleGit from "simple-git"; +import simpleGit, { type StatusResult } from "simple-git"; import { adjectives, animals, @@ -51,6 +51,120 @@ async function getGitEnv(): Promise> { return result; } +/** + * Runs `git status` with --no-optional-locks to avoid holding locks on the repository. + * This prevents blocking other git operations that may need to acquire locks. + * Returns a StatusResult-compatible object that can be used with parseGitStatus. + */ +export async function getStatusNoLock(repoPath: string): Promise { + const env = await getGitEnv(); + + // Run git status with --no-optional-locks to avoid holding locks + // Use porcelain=v1 for machine-parseable output, -b for branch info + const { stdout } = await execFileAsync( + "git", + ["--no-optional-locks", "-C", repoPath, "status", "--porcelain=v1", "-b"], + { env, timeout: 30_000 }, + ); + + const lines = stdout.split("\n"); + let current: string | null = null; + let tracking: string | null = null; + + // Parse branch line: ## branch...tracking or ## branch + const branchLine = lines[0]; + if (branchLine?.startsWith("## ")) { + const branchInfo = branchLine.slice(3); + const trackingMatch = branchInfo.match(/^(.+?)\.\.\.(.+?)(?:\s|$)/); + if (trackingMatch) { + current = trackingMatch[1]; + tracking = trackingMatch[2].split(" ")[0] || null; + } else { + // No tracking branch, just get branch name (before any space for detached HEAD info) + current = branchInfo.split(" ")[0] || null; + } + } + + // Parse file status lines (skip the branch line) + const files: StatusResult["files"] = []; + const staged: string[] = []; + const modified: string[] = []; + const deleted: string[] = []; + const created: string[] = []; + const renamed: Array<{ from: string; to: string }> = []; + const conflicted: string[] = []; + const not_added: string[] = []; + + for (let i = 1; i < lines.length; i++) { + const line = lines[i]; + if (!line || line.length < 3) continue; + + const indexStatus = line[0]; + const workingStatus = line[1]; + let path = line.slice(3); + let from: string | undefined; + + // Handle renames: "R old -> new" format + if (indexStatus === "R" || workingStatus === "R") { + const renameMatch = path.match(/^(.+) -> (.+)$/); + if (renameMatch) { + from = renameMatch[1]; + path = renameMatch[2]; + renamed.push({ from, to: path }); + } + } + + files.push({ + path, + from: from ?? path, + index: indexStatus, + working_dir: workingStatus, + }); + + // Populate convenience arrays for checkBranchCheckoutSafety compatibility + if (indexStatus === "?" && workingStatus === "?") { + not_added.push(path); + } else { + // Index status + if (indexStatus === "A") created.push(path); + else if (indexStatus === "M") { + staged.push(path); + modified.push(path); + } else if (indexStatus === "D") { + staged.push(path); + deleted.push(path); + } else if (indexStatus === "R") staged.push(path); + else if (indexStatus === "U") conflicted.push(path); + else if (indexStatus !== " " && indexStatus !== "?") staged.push(path); + + // Working tree status + if (workingStatus === "M") modified.push(path); + else if (workingStatus === "D") deleted.push(path); + else if (workingStatus === "U") conflicted.push(path); + } + } + + return { + not_added, + conflicted, + created, + deleted, + ignored: undefined, + modified, + renamed, + files, + staged, + ahead: 0, + behind: 0, + current, + tracking, + detached: current === "HEAD", + isClean: () => + files.length === 0 || + files.every((f) => f.index === "?" && f.working_dir === "?"), + }; +} + async function repoUsesLfs(repoPath: string): Promise { try { const lfsDir = join(repoPath, ".git", "lfs"); @@ -401,8 +515,7 @@ export async function checkNeedsRebase( export async function hasUncommittedChanges( worktreePath: string, ): Promise { - const git = simpleGit(worktreePath); - const status = await git.status(); + const status = await getStatusNoLock(worktreePath); return !status.isClean(); } @@ -714,10 +827,8 @@ export interface CheckoutSafetyResult { export async function checkBranchCheckoutSafety( repoPath: string, ): Promise { - const git = simpleGit(repoPath); - try { - const status = await git.status(); + const status = await getStatusNoLock(repoPath); const hasUncommittedChanges = status.staged.length > 0 || @@ -752,6 +863,7 @@ export async function checkBranchCheckoutSafety( // Fetch and prune stale remote refs (best-effort, ignore errors if offline) try { + const git = simpleGit(repoPath); await git.fetch(["--prune"]); } catch { // Ignore fetch errors From 9ff2728a7e2def3b1444798f990a88d00cdf7a24 Mon Sep 17 00:00:00 2001 From: AviPeltz Date: Tue, 13 Jan 2026 14:19:03 -0800 Subject: [PATCH 2/3] fix rename issue --- .../lib/trpc/routers/workspaces/utils/git.ts | 148 +++++++++++++----- 1 file changed, 107 insertions(+), 41 deletions(-) diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts index ce023aafb8e..21b1a203921 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts @@ -59,33 +59,58 @@ async function getGitEnv(): Promise> { export async function getStatusNoLock(repoPath: string): Promise { const env = await getGitEnv(); - // Run git status with --no-optional-locks to avoid holding locks - // Use porcelain=v1 for machine-parseable output, -b for branch info - const { stdout } = await execFileAsync( - "git", - ["--no-optional-locks", "-C", repoPath, "status", "--porcelain=v1", "-b"], - { env, timeout: 30_000 }, - ); - - const lines = stdout.split("\n"); - let current: string | null = null; - let tracking: string | null = null; + try { + // Run git status with --no-optional-locks to avoid holding locks + // Use porcelain=v1 for machine-parseable output, -b for branch info + // Use -z for NUL-terminated output (handles filenames with special chars) + // Use -M for rename detection (otherwise renames show as delete + add) + const { stdout } = await execFileAsync( + "git", + [ + "--no-optional-locks", + "-C", + repoPath, + "status", + "--porcelain=v1", + "-b", + "-z", + "-M", + ], + { env, timeout: 30_000 }, + ); - // Parse branch line: ## branch...tracking or ## branch - const branchLine = lines[0]; - if (branchLine?.startsWith("## ")) { - const branchInfo = branchLine.slice(3); - const trackingMatch = branchInfo.match(/^(.+?)\.\.\.(.+?)(?:\s|$)/); - if (trackingMatch) { - current = trackingMatch[1]; - tracking = trackingMatch[2].split(" ")[0] || null; - } else { - // No tracking branch, just get branch name (before any space for detached HEAD info) - current = branchInfo.split(" ")[0] || null; + return parsePortelainStatus(stdout); + } catch (error) { + // Provide more descriptive error messages + if (isExecFileException(error)) { + if (error.code === "ENOENT") { + throw new Error("Git is not installed or not found in PATH"); + } + const stderr = error.stderr || error.message || ""; + if (stderr.includes("not a git repository")) { + throw new Error(`Not a git repository: ${repoPath}`); + } } + throw new Error( + `Failed to get git status: ${error instanceof Error ? error.message : String(error)}`, + ); } +} + +/** + * Parses git status --porcelain=v1 -z output into a StatusResult-compatible object. + * The -z format uses NUL characters to separate entries, which safely handles + * filenames containing spaces, newlines, or other special characters. + */ +function parsePortelainStatus(stdout: string): StatusResult { + // Split by NUL character - the -z format separates entries with NUL + const entries = stdout.split("\0").filter(Boolean); - // Parse file status lines (skip the branch line) + let current: string | null = null; + let tracking: string | null = null; + let isDetached = false; + + // Parse file status entries const files: StatusResult["files"] = []; const staged: string[] = []; const modified: string[] = []; @@ -95,23 +120,62 @@ export async function getStatusNoLock(repoPath: string): Promise { const conflicted: string[] = []; const not_added: string[] = []; - for (let i = 1; i < lines.length; i++) { - const line = lines[i]; - if (!line || line.length < 3) continue; + let i = 0; + while (i < entries.length) { + const entry = entries[i]; + if (!entry) { + i++; + continue; + } - const indexStatus = line[0]; - const workingStatus = line[1]; - let path = line.slice(3); + // Parse branch line: ## branch...tracking or ## branch + if (entry.startsWith("## ")) { + const branchInfo = entry.slice(3); + + // Check for detached HEAD states + if (branchInfo.startsWith("HEAD (no branch)") || branchInfo === "HEAD") { + isDetached = true; + current = "HEAD"; + } else if ( + // Handle empty repo: "No commits yet on BRANCH" or "Initial commit on BRANCH" + branchInfo.startsWith("No commits yet on ") || + branchInfo.startsWith("Initial commit on ") + ) { + // Extract branch name from the end + const parts = branchInfo.split(" "); + current = parts[parts.length - 1] || null; + } else { + // Check for tracking info: "branch...origin/branch [ahead 1, behind 2]" + const trackingMatch = branchInfo.match(/^(.+?)\.\.\.(.+?)(?:\s|$)/); + if (trackingMatch) { + current = trackingMatch[1]; + tracking = trackingMatch[2].split(" ")[0] || null; + } else { + // No tracking branch, just get branch name (before any space) + current = branchInfo.split(" ")[0] || null; + } + } + i++; + continue; + } + + // Parse file status: "XY path" where X=index, Y=working tree + if (entry.length < 3) { + i++; + continue; + } + + const indexStatus = entry[0]; + const workingStatus = entry[1]; + // entry[2] is a space separator + const path = entry.slice(3); let from: string | undefined; - // Handle renames: "R old -> new" format - if (indexStatus === "R" || workingStatus === "R") { - const renameMatch = path.match(/^(.+) -> (.+)$/); - if (renameMatch) { - from = renameMatch[1]; - path = renameMatch[2]; - renamed.push({ from, to: path }); - } + // For renames/copies, the next entry is the original path + if (indexStatus === "R" || indexStatus === "C") { + i++; + from = entries[i]; + renamed.push({ from: from || path, to: path }); } files.push({ @@ -125,7 +189,7 @@ export async function getStatusNoLock(repoPath: string): Promise { if (indexStatus === "?" && workingStatus === "?") { not_added.push(path); } else { - // Index status + // Index status (staged changes) if (indexStatus === "A") created.push(path); else if (indexStatus === "M") { staged.push(path); @@ -133,15 +197,17 @@ export async function getStatusNoLock(repoPath: string): Promise { } else if (indexStatus === "D") { staged.push(path); deleted.push(path); - } else if (indexStatus === "R") staged.push(path); + } else if (indexStatus === "R" || indexStatus === "C") staged.push(path); else if (indexStatus === "U") conflicted.push(path); else if (indexStatus !== " " && indexStatus !== "?") staged.push(path); - // Working tree status + // Working tree status (unstaged changes) if (workingStatus === "M") modified.push(path); else if (workingStatus === "D") deleted.push(path); else if (workingStatus === "U") conflicted.push(path); } + + i++; } return { @@ -158,7 +224,7 @@ export async function getStatusNoLock(repoPath: string): Promise { behind: 0, current, tracking, - detached: current === "HEAD", + detached: isDetached, isClean: () => files.length === 0 || files.every((f) => f.index === "?" && f.working_dir === "?"), From 025ed8ce06f04fc2a82ab0378e01a707b597befe Mon Sep 17 00:00:00 2001 From: AviPeltz Date: Tue, 13 Jan 2026 14:34:56 -0800 Subject: [PATCH 3/3] use sets to avoid duplicates --- .../lib/trpc/routers/workspaces/utils/git.ts | 50 ++++++++++--------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts index 21b1a203921..6d7a218a157 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts @@ -112,13 +112,14 @@ function parsePortelainStatus(stdout: string): StatusResult { // Parse file status entries const files: StatusResult["files"] = []; - const staged: string[] = []; - const modified: string[] = []; - const deleted: string[] = []; - const created: string[] = []; + // Use Sets to avoid duplicates (e.g., MM status would otherwise add to modified twice) + const stagedSet = new Set(); + const modifiedSet = new Set(); + const deletedSet = new Set(); + const createdSet = new Set(); const renamed: Array<{ from: string; to: string }> = []; - const conflicted: string[] = []; - const not_added: string[] = []; + const conflictedSet = new Set(); + const notAddedSet = new Set(); let i = 0; while (i < entries.length) { @@ -187,39 +188,40 @@ function parsePortelainStatus(stdout: string): StatusResult { // Populate convenience arrays for checkBranchCheckoutSafety compatibility if (indexStatus === "?" && workingStatus === "?") { - not_added.push(path); + notAddedSet.add(path); } else { // Index status (staged changes) - if (indexStatus === "A") created.push(path); + if (indexStatus === "A") createdSet.add(path); else if (indexStatus === "M") { - staged.push(path); - modified.push(path); + stagedSet.add(path); + modifiedSet.add(path); } else if (indexStatus === "D") { - staged.push(path); - deleted.push(path); - } else if (indexStatus === "R" || indexStatus === "C") staged.push(path); - else if (indexStatus === "U") conflicted.push(path); - else if (indexStatus !== " " && indexStatus !== "?") staged.push(path); + stagedSet.add(path); + deletedSet.add(path); + } else if (indexStatus === "R" || indexStatus === "C") + stagedSet.add(path); + else if (indexStatus === "U") conflictedSet.add(path); + else if (indexStatus !== " " && indexStatus !== "?") stagedSet.add(path); // Working tree status (unstaged changes) - if (workingStatus === "M") modified.push(path); - else if (workingStatus === "D") deleted.push(path); - else if (workingStatus === "U") conflicted.push(path); + if (workingStatus === "M") modifiedSet.add(path); + else if (workingStatus === "D") deletedSet.add(path); + else if (workingStatus === "U") conflictedSet.add(path); } i++; } return { - not_added, - conflicted, - created, - deleted, + not_added: [...notAddedSet], + conflicted: [...conflictedSet], + created: [...createdSet], + deleted: [...deletedSet], ignored: undefined, - modified, + modified: [...modifiedSet], renamed, files, - staged, + staged: [...stagedSet], ahead: 0, behind: 0, current,