Skip to content
109 changes: 109 additions & 0 deletions apps/server/src/vcs/GitVcsDriverCore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import * as Effect from "effect/Effect";
import * as FileSystem from "effect/FileSystem";
import * as Fiber from "effect/Fiber";
import * as Layer from "effect/Layer";
import * as Metric from "effect/Metric";
import * as Path from "effect/Path";
import * as PlatformError from "effect/PlatformError";
import * as Queue from "effect/Queue";
import * as Ref from "effect/Ref";
import * as Result from "effect/Result";
import * as Scope from "effect/Scope";
Expand All @@ -20,6 +22,7 @@ import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process";

import { GitCommandError, type ReviewDiffFileContentsInput } from "@t3tools/contracts";
import { ServerConfig } from "../config.ts";
import { gitCommandDuration } from "../observability/Metrics.ts";
import {
makeGitVcsDriverCore,
parseGitCheckoutProgressLine,
Expand Down Expand Up @@ -136,6 +139,112 @@ const initRepoWithCommit = (
return { initialBranch };
});

it.effect("bounds Git bursts across drivers without timing out queued commands", () =>
Effect.gen(function* () {
const gate = yield* Deferred.make<void>();
const starts = yield* Queue.unbounded<number>();
let active = 0;
let peak = 0;
const spawner = ChildProcessSpawner.make(() =>
Effect.acquireRelease(
Effect.gen(function* () {
peak = Math.max(peak, ++active);
yield* Queue.offer(starts, active);
return ChildProcessSpawner.makeHandle({
...makeSuccessfulHandle("ok"),
exitCode: Deferred.await(gate).pipe(Effect.as(ChildProcessSpawner.ExitCode(0))),
});
}),
() => Effect.sync(() => active--),
),
);
const drivers = yield* Effect.all(
Array.from({ length: 16 }, () =>
makeGitVcsDriverCore().pipe(
Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner),
),
),
);
const burst = yield* Effect.forEach(
drivers,
(driver, index) =>
driver.execute({
operation: "test.gitBurst",
cwd: "/repo",
args: ["rev-parse", "HEAD"],
...(index < 4 ? {} : { timeoutMs: index < 8 ? 30_000 : 1_000 }),
}),
{ concurrency: "unbounded" },
).pipe(Effect.forkChild);

yield* TestClock.adjust("2 seconds");
assert.equal(yield* Queue.size(starts), 8);
assert.equal(peak, 8);
yield* Deferred.succeed(gate, undefined);
const results = yield* Fiber.join(burst);
assert.equal(results.length, 16);
assert.isTrue(results.every((result) => result.stdout === "ok" && result.exitCode === 0));
assert.equal(peak, 8);
assert.equal(active, 0);
const duration = yield* Metric.value(
Metric.withAttributes(gitCommandDuration, [["operation", "test.gitBurst"]]),
);
assert.equal(duration.count, 16);
assert.equal(duration.sum, 16_000);
}).pipe(Effect.provide(ServerConfigLayer.pipe(Layer.provideMerge(NodeServices.layer)))),
);

it.effect.each([{ timeoutMs: null }, { timeoutMs: 30_001 }])(
"keeps all Git slots available with a pending command whose timeout is $timeoutMs",
({ timeoutMs }) =>
Effect.gen(function* () {
const slowGate = yield* Deferred.make<void>();
const fastGate = yield* Deferred.make<void>();
const starts = yield* Queue.unbounded<void>();
let active = 0;
const spawner = ChildProcessSpawner.make((command) =>
Effect.acquireRelease(
Effect.gen(function* () {
active++;
yield* Queue.offer(starts, undefined);
const gate =
ChildProcess.isStandardCommand(command) && command.args[0] === "push"
? slowGate
: fastGate;
return ChildProcessSpawner.makeHandle({
...makeSuccessfulHandle("ok"),
exitCode: Deferred.await(gate).pipe(Effect.as(ChildProcessSpawner.ExitCode(0))),
});
}),
() => Effect.sync(() => active--),
),
);
const driver = yield* makeGitVcsDriverCore().pipe(
Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner),
);
const slow = yield* driver
.execute({ operation: "test.slowGit", cwd: "/repo", args: ["push"], timeoutMs })
.pipe(Effect.forkChild);
yield* Queue.take(starts);
const burst = yield* Effect.all(
Array.from({ length: 8 }, () =>
driver.execute({ operation: "test.fastGit", cwd: "/repo", args: ["status"] }),
),
{ concurrency: "unbounded" },
).pipe(Effect.forkChild);

yield* TestClock.adjust("0 seconds");
assert.equal(yield* Queue.size(starts), 8);
assert.equal(active, 9);
yield* Deferred.succeed(fastGate, undefined);
assert.equal((yield* Fiber.join(burst)).length, 8);
assert.equal(active, 1);
yield* Deferred.succeed(slowGate, undefined);
assert.equal((yield* Fiber.join(slow)).stdout, "ok");
assert.equal(active, 0);
}).pipe(Effect.provide(ServerConfigLayer.pipe(Layer.provideMerge(NodeServices.layer)))),
);

for (const location of ["root", "nested", "worktree"] as const) {
it.effect(
`skips clean filters while the ${location} index is locked and resumes after unlock`,
Expand Down
5 changes: 5 additions & 0 deletions apps/server/src/vcs/GitVcsDriverCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
import { ServerConfig } from "../config.ts";

const DEFAULT_TIMEOUT_MS = 30_000;
const gitProcesses = Semaphore.makeUnsafe(8);
// `git worktree add` checks out the full tree, so on large repositories it can
// take well beyond the default 30s (e.g. a 375k-file repo takes ~40s on an idle
// machine). Give it generous headroom while still bounding a genuinely hung git.
Expand Down Expand Up @@ -903,6 +904,10 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
operation: input.operation,
},
}),
(execution) =>
input.timeoutMs === null || (input.timeoutMs ?? DEFAULT_TIMEOUT_MS) > DEFAULT_TIMEOUT_MS
Comment thread
Bil0000 marked this conversation as resolved.
? execution
: gitProcesses.withPermits(1)(execution),
Effect.withSpan(input.operation, {
kind: "client",
attributes: {
Expand Down
Loading