Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 79 additions & 32 deletions apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ import { checkGitLfsAvailable, getShellEnvironment } from "./shell-env";

const execFileAsync = promisify(execFile);

/**
* 50 MiB – generous ceiling for git-status stdout.
* Repos with huge numbers of untracked/changed files can easily exceed the
* Node.js default (1 MiB), which causes an ERR_CHILD_PROCESS_STDIO_MAXBUFFER
* crash (Sentry DESKTOP-1J). 50 MiB handles even very large monorepos.
*/
const GIT_STATUS_MAX_BUFFER = 50 * 1024 * 1024;

/**
* Error thrown by execFile when the command fails.
* `code` can be a number (exit code) or string (spawn error like "ENOENT").
Expand Down Expand Up @@ -59,42 +67,81 @@ export async function getStatusNoLock(repoPath: string): Promise<StatusResult> {
const env = await getGitEnv();

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 -uall to show individual files in untracked directories (not just the directory)
// Note: porcelain=v1 already includes rename info (R/C status codes) without needing -M
const { stdout } = await execFileAsync(
"git",
[
"--no-optional-locks",
"-C",
repoPath,
"status",
"--porcelain=v1",
"-b",
"-z",
"-uall",
],
{ env, timeout: 30_000 },
);

return parsePortelainStatus(stdout);
return await runGitStatus({ repoPath, env, untrackedMode: "-uall" });
} 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}`);
// If maxBuffer is exceeded even with the generous limit, retry with
// -unormal (collapses untracked directories into a single entry).
if (isMaxBufferError(error)) {
console.warn(
"[git/status] maxBuffer exceeded with -uall, retrying with -unormal:",
repoPath,
);
try {
return await runGitStatus({
repoPath,
env,
untrackedMode: "-unormal",
});
} catch (retryError) {
throw toGitStatusError(retryError);
}
}
throw new Error(
`Failed to get git status: ${error instanceof Error ? error.message : String(error)}`,
);

throw toGitStatusError(error);
}
Comment on lines 69 to +91
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Log original git‑status failures before wrapping.

The catch block wraps errors into a new Error without logging the original, which drops diagnostics. Please log with context before throwing so failures aren’t silent in logs.

🔧 Suggested patch
 		if (isMaxBufferError(error)) {
 			console.warn(
 				"[git/status] maxBuffer exceeded with -uall, retrying with -unormal:",
 				repoPath,
 			);
 			try {
 				return await runGitStatus({
 					repoPath,
 					env,
 					untrackedMode: "-unormal",
 				});
 			} catch (retryError) {
+				console.error(
+					"[git/status] git status failed after -unormal retry:",
+					repoPath,
+					retryError,
+				);
 				throw toGitStatusError(retryError);
 			}
 		}
 
+		console.error("[git/status] git status failed:", repoPath, error);
 		throw toGitStatusError(error);
 	}

As per coding guidelines, “Never swallow errors silently; at minimum log errors with context before rethrowing or handling them explicitly” and “Use prefixed console logging with consistent context pattern: [domain/operation] message for entry/exit of significant operations, external API calls, and error conditions”.

🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts` around lines 69 -
91, The catch currently wraps errors via toGitStatusError without logging the
original; update the block around runGitStatus so the original error is logged
with context before any rethrow or retry: when entering the catch log the
initial error using a prefixed message like "[git/status] git status failed"
along with repoPath, env and the error; if isMaxBufferError triggers, also log
the initial max-buffer error before retrying and then log the retryError before
throwing the wrapped error from toGitStatusError. Use the same context pattern
for all logs and reference the existing symbols runGitStatus, isMaxBufferError,
toGitStatusError, repoPath, env and untrackedMode so the added logs appear just
prior to each throw/retry.

}

async function runGitStatus({
repoPath,
env,
untrackedMode,
}: {
repoPath: string;
env: Record<string, string>;
untrackedMode: "-uall" | "-unormal";
}): Promise<StatusResult> {
// 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)
// Note: porcelain=v1 already includes rename info (R/C status codes) without needing -M
const { stdout } = await execFileAsync(
"git",
[
"--no-optional-locks",
"-C",
repoPath,
"status",
"--porcelain=v1",
"-b",
"-z",
untrackedMode,
],
{ env, timeout: 30_000, maxBuffer: GIT_STATUS_MAX_BUFFER },
);

return parsePortelainStatus(stdout);
}

function isMaxBufferError(error: unknown): boolean {
return (
error instanceof Error &&
error.message.includes("maxBuffer length exceeded")
);
}

function toGitStatusError(error: unknown): Error {
if (isExecFileException(error)) {
if (error.code === "ENOENT") {
return new Error("Git is not installed or not found in PATH");
}
const stderr = error.stderr || error.message || "";
if (stderr.includes("not a git repository")) {
return new Error(`Not a git repository`);
}
}
return new Error(
`Failed to get git status: ${error instanceof Error ? error.message : String(error)}`,
);
}

/**
Expand Down