fix(server): centralize git subprocesses with p-limit throttling (#310)
* Fix uncaught timeout rejection when archiving agents * fix(server): centralize git subprocesses under runGitCommand with p-limit throttling Supersedes #299. Same problem (unbounded git subprocess concurrency causing zombie accumulation), different approach: a single `runGitCommand(args, opts)` function backed by `p-limit` instead of a custom semaphore pool. - Always uses `spawn` (one code path, no exec/execFile split) - Concurrency configurable via PASEO_GIT_CONCURRENCY env var (default 8) - 30s default timeout for read-only ops, 120s for mutations - Centralized error formatting, stdout truncation, process reaping - Runtime tests verifying throttling, timeout kills, deadlock prevention * fix: update lockfile signatures and Nix hash * fix(server): add Windows compatibility to runGitCommand - shell: true on Windows (resolves .cmd/.bat git shims) - windowsHide: true always (prevents console window flashes) - quoteForCmd for args with spaces when routing through cmd.exe - Tests verify shell/windowsHide behavior per platform * refactor: use spawnProcess instead of duplicating Windows handling spawnProcess from spawn.ts already handles shell: true on Windows, arg quoting for cmd.exe, and windowsHide: true. No need to reimplement. * fix: non-null assertions for stdio streams in strict build --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This commit is contained in:
co-authored by
github-actions[bot] <github-actions[bot]@users.noreply.github.com>
parent
e6ff15a1e0
commit
964bf3e13f
@@ -12,6 +12,7 @@ import {
|
||||
writePaseoWorktreeMetadata,
|
||||
writePaseoWorktreeRuntimeMetadata,
|
||||
} from "./worktree-metadata.js";
|
||||
import { runGitCommand } from "./run-git-command.js";
|
||||
import { resolvePaseoHome } from "../server/paseo-home.js";
|
||||
|
||||
interface PaseoConfig {
|
||||
@@ -377,10 +378,13 @@ async function inferRepoRootPathFromWorktreePath(worktreePath: string): Promise<
|
||||
} catch {
|
||||
// Fallback: best-effort resolve toplevel (will be the worktree root in typical cases)
|
||||
try {
|
||||
const { stdout } = await execAsync("git rev-parse --path-format=absolute --show-toplevel", {
|
||||
cwd: worktreePath,
|
||||
env: READ_ONLY_GIT_ENV,
|
||||
});
|
||||
const { stdout } = await runGitCommand(
|
||||
["rev-parse", "--path-format=absolute", "--show-toplevel"],
|
||||
{
|
||||
cwd: worktreePath,
|
||||
env: READ_ONLY_GIT_ENV,
|
||||
},
|
||||
);
|
||||
const topLevel = stdout.trim();
|
||||
if (topLevel) {
|
||||
return normalizePathForOwnership(topLevel);
|
||||
@@ -438,8 +442,9 @@ export async function runWorktreeSetupCommands(options: {
|
||||
if (result.exitCode !== 0) {
|
||||
if (options.cleanupOnFailure) {
|
||||
try {
|
||||
await execAsync(`git worktree remove "${options.worktreePath}" --force`, {
|
||||
await runGitCommand(["worktree", "remove", options.worktreePath, "--force"], {
|
||||
cwd: options.worktreePath,
|
||||
timeout: 120_000,
|
||||
});
|
||||
} catch {
|
||||
rmSync(options.worktreePath, { recursive: true, force: true });
|
||||
@@ -457,7 +462,7 @@ export async function runWorktreeSetupCommands(options: {
|
||||
|
||||
async function resolveBranchNameForWorktreePath(worktreePath: string): Promise<string> {
|
||||
try {
|
||||
const { stdout } = await execAsync("git branch --show-current", {
|
||||
const { stdout } = await runGitCommand(["branch", "--show-current"], {
|
||||
cwd: worktreePath,
|
||||
env: READ_ONLY_GIT_ENV,
|
||||
});
|
||||
@@ -560,10 +565,13 @@ export async function runWorktreeTeardownCommands(options: {
|
||||
* This is where refs, objects, etc. are stored.
|
||||
*/
|
||||
export async function getGitCommonDir(cwd: string): Promise<string> {
|
||||
const { stdout } = await execAsync("git rev-parse --path-format=absolute --git-common-dir", {
|
||||
cwd,
|
||||
env: READ_ONLY_GIT_ENV,
|
||||
});
|
||||
const { stdout } = await runGitCommand(
|
||||
["rev-parse", "--path-format=absolute", "--git-common-dir"],
|
||||
{
|
||||
cwd,
|
||||
env: READ_ONLY_GIT_ENV,
|
||||
},
|
||||
);
|
||||
const commonDir = stdout.trim();
|
||||
if (!commonDir) {
|
||||
throw new Error("Not in a git repository");
|
||||
@@ -793,7 +801,7 @@ export async function listPaseoWorktrees({
|
||||
paseoHome?: string;
|
||||
}): Promise<PaseoWorktreeInfo[]> {
|
||||
const worktreesRoot = await getPaseoWorktreesRoot(cwd, paseoHome);
|
||||
const { stdout } = await execAsync("git worktree list --porcelain", {
|
||||
const { stdout } = await runGitCommand(["worktree", "list", "--porcelain"], {
|
||||
cwd,
|
||||
env: READ_ONLY_GIT_ENV,
|
||||
});
|
||||
@@ -824,10 +832,13 @@ export async function resolvePaseoWorktreeRootForCwd(
|
||||
|
||||
let worktreeRoot: string | null = null;
|
||||
try {
|
||||
const { stdout } = await execAsync("git rev-parse --path-format=absolute --show-toplevel", {
|
||||
cwd,
|
||||
env: READ_ONLY_GIT_ENV,
|
||||
});
|
||||
const { stdout } = await runGitCommand(
|
||||
["rev-parse", "--path-format=absolute", "--show-toplevel"],
|
||||
{
|
||||
cwd,
|
||||
env: READ_ONLY_GIT_ENV,
|
||||
},
|
||||
);
|
||||
const trimmed = stdout.trim();
|
||||
worktreeRoot = trimmed.length > 0 ? trimmed : null;
|
||||
} catch {
|
||||
@@ -890,8 +901,9 @@ export async function deletePaseoWorktree({
|
||||
worktreePath: resolvedWorktree,
|
||||
});
|
||||
|
||||
await execAsync(`git worktree remove "${resolvedWorktree}" --force`, {
|
||||
await runGitCommand(["worktree", "remove", resolvedWorktree, "--force"], {
|
||||
cwd,
|
||||
timeout: 120_000,
|
||||
});
|
||||
|
||||
if (existsSync(resolvedWorktree)) {
|
||||
@@ -927,11 +939,11 @@ export async function createWorktree({
|
||||
// Resolve the base branch - prefer origin/{branch}, then fall back to local
|
||||
let resolvedBaseBranch = normalizedBaseBranch;
|
||||
try {
|
||||
await execAsync(`git rev-parse --verify origin/${normalizedBaseBranch}`, { cwd });
|
||||
await runGitCommand(["rev-parse", "--verify", `origin/${normalizedBaseBranch}`], { cwd });
|
||||
resolvedBaseBranch = `origin/${normalizedBaseBranch}`;
|
||||
} catch {
|
||||
try {
|
||||
await execAsync(`git rev-parse --verify ${normalizedBaseBranch}`, { cwd });
|
||||
await runGitCommand(["rev-parse", "--verify", normalizedBaseBranch], { cwd });
|
||||
} catch {
|
||||
throw new Error(`Base branch not found: ${normalizedBaseBranch}`);
|
||||
}
|
||||
@@ -946,7 +958,9 @@ export async function createWorktree({
|
||||
// Check if branch already exists
|
||||
let branchExists = false;
|
||||
try {
|
||||
await execAsync(`git show-ref --verify --quiet refs/heads/${branchName}`, { cwd });
|
||||
await runGitCommand(["show-ref", "--verify", "--quiet", `refs/heads/${branchName}`], {
|
||||
cwd,
|
||||
});
|
||||
branchExists = true;
|
||||
} catch {
|
||||
branchExists = false;
|
||||
@@ -963,7 +977,9 @@ export async function createWorktree({
|
||||
let suffix = 1;
|
||||
while (true) {
|
||||
try {
|
||||
await execAsync(`git show-ref --verify --quiet refs/heads/${newBranchName}`, { cwd });
|
||||
await runGitCommand(["show-ref", "--verify", "--quiet", `refs/heads/${newBranchName}`], {
|
||||
cwd,
|
||||
});
|
||||
// Branch exists, try with suffix
|
||||
newBranchName = `${candidateBranch}-${suffix}`;
|
||||
suffix++;
|
||||
@@ -980,8 +996,10 @@ export async function createWorktree({
|
||||
pathSuffix++;
|
||||
}
|
||||
|
||||
const command = `git worktree add "${finalWorktreePath}" -b "${newBranchName}" "${base}"`;
|
||||
await execAsync(command, { cwd });
|
||||
await runGitCommand(["worktree", "add", finalWorktreePath, "-b", newBranchName, base], {
|
||||
cwd,
|
||||
timeout: 120_000,
|
||||
});
|
||||
worktreePath = normalizePathForOwnership(finalWorktreePath);
|
||||
|
||||
writePaseoWorktreeMetadata(worktreePath, { baseRefName: normalizedBaseBranch });
|
||||
|
||||
Reference in New Issue
Block a user