Skip to content
Open
Show file tree
Hide file tree
Changes from 6 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
87 changes: 87 additions & 0 deletions apps/server/src/git/GitManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1020,6 +1020,57 @@ it.layer(layerGitManagerTest)("GitManager", (it) => {
}),
);

it.effect("a git read in flight during a checkout does not outlive it", () =>
Effect.gen(function* () {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
const repoDir = yield* makeTempDir("t3code-git-manager-");
yield* initRepo(repoDir);
const remoteDir = yield* createBareRemote();
yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]);
yield* runGit(repoDir, ["checkout", "-b", "feature/racing-upstream"]);
yield* runGit(repoDir, ["push", "-u", "origin", "feature/racing-upstream"]);
yield* runGit(repoDir, ["push", "origin", "HEAD:refs/heads/feature/raced-head"]);
const pullRequest = (number: number, headRefName: string): FakeChangeRequest[] => [
{
number,
title: headRefName,
url: `https://github.com/pingdotgg/codething-mvp/pull/${number}`,
baseRefName: "main",
headRefName,
state: "open",
updatedAt: "2026-04-07T15:00:00Z",
},
];
const gate = {
matches: (args: ReadonlyArray<string>) =>
args[0] === "for-each-ref" && args.at(-1) === "refs/heads/feature/racing-upstream",
reached: yield* Deferred.make<void>(),
release: yield* Deferred.make<void>(),
};
const { manager } = yield* makeManager({
gitSpawnGate: gate,
providerScenario: {
prListByHeadSelector: {
"feature/racing-upstream": pullRequest(237, "feature/racing-upstream"),
"feature/raced-head": pullRequest(238, "feature/raced-head"),
},
},
});
const lookup = manager.branchPullRequest({
cwd: repoDir,
branch: "feature/racing-upstream",
});

const inFlight = yield* Effect.forkChild(lookup);
yield* Deferred.await(gate.reached);
yield* runGit(repoDir, ["branch", "--set-upstream-to=origin/feature/raced-head"]);
yield* manager.invalidateStatus(repoDir, { keepPullRequestLookups: true });
yield* Deferred.succeed(gate.release, undefined);
yield* Fiber.join(inFlight);

expect((yield* lookup)?.number).toBe(238);
}),
);

it.effect("branch PR lookup notifies linked threads when draft status changes", () =>
Effect.gen(function* () {
const repoDir = yield* makeTempDir("t3code-git-manager-");
Expand Down Expand Up @@ -2433,6 +2484,42 @@ it.layer(layerGitManagerTest)("GitManager", (it) => {
}),
);

it.effect("a checkout's invalidation keeps the branch's cached PR lookup", () =>
Effect.gen(function* () {
const repoDir = yield* makeTempDir("t3code-git-manager-");
yield* initRepo(repoDir);
yield* runGit(repoDir, ["checkout", "-b", "feature/pr-cached"]);
const remoteDir = yield* createBareRemote();
yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]);
yield* runGit(repoDir, ["push", "-u", "origin", "feature/pr-cached"]);

const { manager } = yield* makeManager({
providerScenario: {
prListSequence: [
[
{
number: 215,
title: "Cached PR",
url: "https://github.com/pingdotgg/codething-mvp/pull/215",
baseRefName: "main",
headRefName: "feature/pr-cached",
},
],
[],
],
},
});

expect((yield* manager.status({ cwd: repoDir })).pr?.number).toBe(215);

yield* manager.invalidateStatus(repoDir, { keepPullRequestLookups: true });
expect((yield* manager.status({ cwd: repoDir })).pr?.number).toBe(215);

yield* manager.invalidateStatus(repoDir);
expect((yield* manager.status({ cwd: repoDir })).pr).toBeNull();
}),
);

it.effect(
"status does not reuse a stale PR after the branch is retargeted to a different upstream",
() =>
Expand Down
27 changes: 22 additions & 5 deletions apps/server/src/git/GitManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,11 @@ export class GitManager extends Context.Service<
) => Effect.Effect<GitBranchPullRequest | null, GitManagerServiceError>;
readonly invalidateLocalStatus: (cwd: string) => Effect.Effect<void, never>;
readonly invalidateRemoteStatus: (cwd: string) => Effect.Effect<void, never>;
readonly invalidateStatus: (cwd: string) => Effect.Effect<void, never>;
/** keepPullRequestLookups is for changes that cannot affect a PR, such as a checkout. */
readonly invalidateStatus: (
cwd: string,
options?: { readonly keepPullRequestLookups?: boolean },
) => Effect.Effect<void, never>;
readonly resolvePullRequest: (
input: GitPullRequestRefInput,
) => Effect.Effect<GitResolvePullRequestResult, GitManagerServiceError>;
Expand Down Expand Up @@ -1151,6 +1155,15 @@ export const make = Effect.gen(function* () {
prLookupEpochByCwd.set(cacheKey, prLookupEpoch(cacheKey) + 1);
}),
);
// Separate from the PR lookup epoch because a checkout keeps PR lookups but changes git state.
const prLookupGitStateEpochByCwd = new Map<string, number>();
const prLookupGitStateEpoch = (cwd: string) => prLookupGitStateEpochByCwd.get(cwd) ?? 0;
const bumpPrLookupGitStateEpoch = (cwd: string) =>
normalizeStatusCacheKey(cwd).pipe(
Effect.map((cacheKey) => {
prLookupGitStateEpochByCwd.set(cacheKey, prLookupGitStateEpoch(cacheKey) + 1);
}),
);
const prLookupCacheKey = (
cwd: string,
details: {
Expand Down Expand Up @@ -2403,10 +2416,13 @@ export const make = Effect.gen(function* () {
}
// invalidateStatus bumps the epoch. A read that started before a T3 git
// action, or before a newer read finished, must not replace their state.
const epoch = prLookupEpoch(cwd);
const epoch = prLookupGitStateEpoch(cwd);
const value = yield* read;
const latest = prLookupGitStateByKey.get(entryKey);
if (prLookupEpoch(cwd) !== epoch || (latest !== undefined && latest.readAtMs > readAtMs)) {
if (
prLookupGitStateEpoch(cwd) !== epoch ||
(latest !== undefined && latest.readAtMs > readAtMs)
) {
return value;
}
prLookupGitStateByKey.delete(entryKey);
Expand Down Expand Up @@ -2665,10 +2681,11 @@ export const make = Effect.gen(function* () {
yield* invalidateRemoteStatusResultCache(cwd);
});
const invalidateStatus: GitManager["Service"]["invalidateStatus"] = Effect.fn("invalidateStatus")(
function* (cwd) {
function* (cwd, options) {
yield* invalidateLocalStatusResultCache(cwd);
yield* invalidateRemoteStatusResultCache(cwd);
yield* bumpPrLookupEpoch(cwd);
if (!options?.keepPullRequestLookups) yield* bumpPrLookupEpoch(cwd);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
yield* bumpPrLookupGitStateEpoch(cwd);
yield* forgetPrLookupGitState(cwd);
},
);
Expand Down
4 changes: 3 additions & 1 deletion apps/server/src/git/GitWorkflowService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ export class GitWorkflowService extends Context.Service<
) => Effect.Effect<VcsStatusRemoteResult | null, GitManagerServiceError>;
readonly invalidateLocalStatus: (cwd: string) => Effect.Effect<void, never>;
readonly invalidateRemoteStatus: (cwd: string) => Effect.Effect<void, never>;
readonly invalidateStatus: (cwd: string) => Effect.Effect<void, never>;
readonly invalidateStatus: GitManager.GitManager["Service"]["invalidateStatus"];
readonly statusUpstreamFetchCount: GitVcsDriver.GitVcsDriver["Service"]["statusUpstreamFetchCount"];
readonly pullCurrentBranch: (cwd: string) => Effect.Effect<VcsPullResult, GitCommandError>;
readonly runStackedAction: (
input: GitRunStackedActionInput,
Expand Down Expand Up @@ -369,6 +370,7 @@ export const make = Effect.gen(function* () {
invalidateLocalStatus: gitManager.invalidateLocalStatus,
invalidateRemoteStatus: gitManager.invalidateRemoteStatus,
invalidateStatus: gitManager.invalidateStatus,
statusUpstreamFetchCount: git.statusUpstreamFetchCount,
pullCurrentBranch: Effect.fn("GitWorkflowService.pullCurrentBranch")(function (cwd) {
return ensureGitCommand("GitWorkflowService.pullCurrentBranch", cwd).pipe(
Effect.andThen(git.pullCurrentBranch(cwd)),
Expand Down
2 changes: 2 additions & 0 deletions apps/server/src/vcs/GitVcsDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,8 @@ export class GitVcsDriver extends Context.Service<
cwd: string,
options?: GitRemoteStatusOptions,
) => Effect.Effect<GitRemoteStatusDetails, GitCommandError>;
/** Changes whenever a status read fetches this repository, so callers can tell if refs may have moved. */
readonly statusUpstreamFetchCount: (cwd: string) => Effect.Effect<number>;
readonly prepareCommitContext: (
cwd: string,
filePaths?: readonly string[],
Expand Down
64 changes: 64 additions & 0 deletions apps/server/src/vcs/GitVcsDriverCore.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// @effect-diagnostics nodeBuiltinImport:off - realpathSync.native resolves Windows 8.3 short names, which the Effect realPath does not.
import * as NodeChildProcess from "node:child_process";
import * as NodeFS from "node:fs";
import * as NodeOS from "node:os";
import * as NodePath from "node:path";
import * as NodeURL from "node:url";
import * as NodeServices from "@effect/platform-node/NodeServices";
import * as HostProcess from "@t3tools/shared/HostProcess";
Expand Down Expand Up @@ -3386,6 +3388,68 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => {
}),
);

it.effect("counts a status read's upstream fetch only when one actually ran", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
const remote = yield* makeTmpDir("git-vcs-driver-remote-");
const { initialBranch } = yield* initRepoWithCommit(cwd);
yield* git(remote, ["init", "--bare"]);
yield* git(cwd, ["remote", "add", "origin", remote]);
yield* git(cwd, ["push", "-u", "origin", initialBranch]);

const driver = yield* GitVcsDriver.GitVcsDriver;
const initial = yield* driver.statusUpstreamFetchCount(cwd);

yield* driver.statusDetailsRemote(cwd, { refreshUpstream: false });
assert.equal(yield* driver.statusUpstreamFetchCount(cwd), initial);

yield* driver.statusDetailsRemote(cwd);
const afterFetch = yield* driver.statusUpstreamFetchCount(cwd);
assert.equal(afterFetch, initial + 1);

// Still inside the refresh interval, so this read reuses the last fetch.
yield* driver.statusDetailsRemote(cwd);
assert.equal(yield* driver.statusUpstreamFetchCount(cwd), afterFetch);
}),
);

// Needs mkfifo to hold the fetch open at a known point.
it.effect.skipIf(HostProcess.Platform.defaultValue() === "win32")(
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
"does not count a status fetch that is still running after its wait times out",
() =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
const remote = yield* makeTmpDir("git-vcs-driver-remote-");
const control = yield* makeTmpDir("git-vcs-driver-control-");
const { initialBranch } = yield* initRepoWithCommit(cwd);
yield* git(remote, ["init", "--bare"]);
yield* git(cwd, ["remote", "add", "origin", remote]);
yield* git(cwd, ["push", "-u", "origin", initialBranch]);

const started = NodePath.join(control, "started");
const release = NodePath.join(control, "release");
NodeChildProcess.execFileSync("mkfifo", [started, release]);
const uploadPack = NodePath.join(control, "upload-pack.sh");
NodeFS.writeFileSync(
uploadPack,
`#!/bin/sh\necho started > "${started}"\ncat "${release}" > /dev/null\nexec git-upload-pack "$@"\n`,
{ mode: 0o755 },
);
// Git runs this setting through a shell, and the temp path can contain spaces.
yield* git(cwd, ["config", "remote.origin.uploadpack", `'${uploadPack}'`]);

const driver = yield* GitVcsDriver.GitVcsDriver;
const initial = yield* driver.statusUpstreamFetchCount(cwd);
const status = yield* driver.statusDetailsRemote(cwd).pipe(Effect.forkScoped);
yield* Effect.promise(() => NodeFS.promises.readFile(started));
yield* TestClock.adjust("6 seconds");
yield* Fiber.join(status);

assert.equal(yield* driver.statusUpstreamFetchCount(cwd), initial);
yield* Effect.promise(() => NodeFS.promises.writeFile(release, "go"));
}),
);

it.effect("does not start Git auto-maintenance from background upstream fetches", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
Expand Down
15 changes: 15 additions & 0 deletions apps/server/src/vcs/GitVcsDriverCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1326,6 +1326,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
);
});

const statusUpstreamFetches = new Map<string, number>();
const fetchRemoteForStatus = Effect.fn("GitVcsDriver.fetchRemoteForStatus.wait")(function* (
gitCommonDir: string,
remoteName: string,
Expand Down Expand Up @@ -1374,6 +1375,12 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
Effect.onExit(() =>
Effect.sync(() => {
statusFetches.delete(key);
// Counted at exit, not when a waiter stops waiting: a waiter can time out before refs
// move. Failures count too, because a failed fetch can still move some refs.
statusUpstreamFetches.set(
gitCommonDir,
(statusUpstreamFetches.get(gitCommonDir) ?? 0) + 1,
);
}),
),
Effect.forkIn(serviceScope, { startImmediately: false, uninterruptible: false }),
Expand Down Expand Up @@ -1640,6 +1647,13 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
),
});

const statusUpstreamFetchCount: GitVcsDriver.GitVcsDriver["Service"]["statusUpstreamFetchCount"] =
(cwd) =>
resolveGitCommonDir(cwd).pipe(
Effect.map((gitCommonDir) => statusUpstreamFetches.get(gitCommonDir) ?? 0),
Effect.orElseSucceed(() => 0),
);

const refreshStatusUpstreamIfStale = Effect.fn("refreshStatusUpstreamIfStale")(function* (
cwd: string,
) {
Expand Down Expand Up @@ -4936,6 +4950,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
statusDetails,
statusDetailsLocal,
statusDetailsRemote,
statusUpstreamFetchCount,
prepareCommitContext,
commit: Effect.fnUntraced(function* (cwd, subject, body, options) {
return yield* withListRefsInvalidation(cwd, commit(cwd, subject, body, options));
Expand Down
Loading
Loading